codecov / codecov-exe

.exe report uploader for Codecov https://codecov.io
MIT License
25 stars 21 forks source link

Report uploads fails on certain branches and/or job IDs #36

Closed Sharparam closed 6 years ago

Sharparam commented 6 years ago

We are uploading coverage reports in our AppVeyor build (via Cake.Codecov) which works fine for the develop branch, but fails when it's building feature branches.

Working AppVeyor build on develop branch: https://ci.appveyor.com/project/Corale/colore/build/6.0.0-unstable.231.build.137

Broken AppVeyor build on feature/upgrade-dotnet-sdk branch: https://ci.appveyor.com/project/Corale/colore/build/6.0.0-upgrade-dotnet-sdk.1+230.build.136

The only difference between the two is the branch and version being generated, the two CodeCov URLs are:

Working: https://codecov.io/upload/v4?branch=develop&commit=c452ba71592bf81a23091bc070aae905da01fcd3&build=27iw4u2biyyj72k7&tag=&pr=&name=&flags=&slug=chroma-sdk%2FColore&token=&package=exe-1.0.3&build_url=&yaml=.codecov.yml&job=Corale%2Fcolore%2F6.0.0-unstable.231.build.137&service=appveyor

Broken: https://codecov.io/upload/v4?branch=feature%2Fupgrade-dotnet-sdk&commit=33a280c35f7c9256b15495f517438acf617bcbc5&build=ses9x3442e84l6td&tag=&pr=&name=&flags=&slug=chroma-sdk%2FColore&token=&package=exe-1.0.3&build_url=&yaml=.codecov.yml&job=Corale%2Fcolore%2F6.0.0-upgrade-dotnet-sdk.1%2B230.build.136&service=appveyor

As you can see, the branch name is escaped properly, so this is not related to #32.

The only thing I can think of is that the Codecov service doesn't like the escaped '+' sign in the job ID (%2B)? Slashes (%2F) appear to be fine as they are present in the working build as well.

AdmiringWorm commented 6 years ago

this will be fixed in the upcoming release of the next version

AdmiringWorm commented 6 years ago

@Sharparam are you able to verify that this was indeed fixed in the latest release of codecov exe? (Version 1.0.4).

Sharparam commented 6 years ago

@AdmiringWorm Hm, it seems to be failing still: https://ci.appveyor.com/project/Corale/colore/build/138

Sharparam commented 6 years ago

@AdmiringWorm I added URL escaping to the version number as well and that seems to make it work: https://ci.appveyor.com/project/Corale/colore/build/6.0.0-upgrade-codecov.1+234.build.139 (This build also has verbose logging turned on for Codecov)

AdmiringWorm commented 6 years ago

hmm, I see that the build that succeeds that you linked to actually double encodes the + sign.

How you got that though I have no idea, do you run a custom edition of codecov-exe?

Anyways, something I have to take a closer look into.

Sharparam commented 6 years ago

It seems to be encoding the percentage sign actually:

%252B

The string passed in by me to the Codecov tool (no custom build) is %2B, the tool then seems to encode the '%' to %25, which works for some reason. Would it then work if the '+' is encoded normally as it was earlier? As it is now, Codecov is receiving a version that has '%2B' in it, which probably won't be decoded into a '+' as expected.

I don't see Codecov displaying this version number anywhere to verify what the final version it got was.

AdmiringWorm commented 6 years ago

ah, that would explain it why it was encoded. Could you share the exact string that you're passing to codecov (if possible)? Could help me determine what is going on, it could be that normal encoding of the + char would work if the problem lies elsewhere in the string.

Sharparam commented 6 years ago

For the version string (placed in the APPVEYOR_BUILD_VERSION environment variable) I'm passing 6.0.0-upgrade-codecov.1%2B234.build.139. The branch name is passed as feature%2Fupgrade-codecov.

AdmiringWorm commented 6 years ago

@Sharparam I just made some changes to the library (not pushed to github yet), and uploaded a beta package to a personal myget repository.

Could I ask you to test out that beta package before I make any release? Since you're using cake you could just replace the #tool pre-processor you use for codecov with the following:

#tool nuget:https://www.myget.org/F/wormie-nugets/api/v2?package=Codecov&prerelease
AdmiringWorm commented 6 years ago

It seems like this is actually fixed now.

I'm closing this one again, and preparing to release a new version of codecov-exe.

Sharparam commented 6 years ago

I'm not at home yet so can't test. Should I leave it as it currently is (escaping both branch and version string)?

On Sun, 1 Jul 2018, 14:15 Kim J. Nordmo, notifications@github.com wrote:

It seems like this is actually fixed now.

I'm closing this one again, and preparing to release a new version of codecov-exe.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/codecov/codecov-exe/issues/36#issuecomment-401602949, or mute the thread https://github.com/notifications/unsubscribe-auth/AAhotiCymnaTQBWU28Z1HtZuiL5si2PUks5uCL11gaJpZM4Uqv9F .

AdmiringWorm commented 6 years ago

You could try both. With the new release you shouldn't need to escape it manually.

I'll hold off with the release until later today.

AdmiringWorm commented 6 years ago

I decided to release the new version now (still being indexed on nuget.org though).

Let me know if it still doesn't work.