Analyze code coverage#1106
Conversation
There was a problem hiding this comment.
Not sure why coverage depends on $Env:SHOULD_PACKAGE_NUGET_ARTIFACT?
There was a problem hiding this comment.
You're right. The idea is to analyze on merges. This also happen to be when we generate NuGet packages.
I think the variable is badly named. What would you call it?
There was a problem hiding this comment.
Personally I'd add a separate SHOULD_RUN_COVERALLS variable.
|
The problem is that appveyor removes secure token variables on PR builds. The args list becomes If you look at my other projects, I wrap it in a condition to check for the PR build environment variable |
|
@csMACnz Thanks for your answer. I've pushed this as a separate branch (see ntk/coverage...coveralls). The log still displays the same usage pattern. However, this time it also fails with a Another question. When the Usage is displayed, the possible options aren't. Is that expected? /cc @FeodorFitsner |
|
You mean AppVeyor swallows |
|
@FeodorFitsner It might. By taking a look at source code it looks like at least one of them should be output in this log. It's fairly possible I may be completely wrong about this though. |
|
Your variables don't match so its still the same problem. You declare coveralls_token then use COVERALLS_REPO_TOKEN. Also, the docopts.net library I use is printing the usage statements, and returning the exit code. That is why none of my console.error.writeline calls show up. |
@csMACnz 💎 Thanks a lot for having pointed me at this mistake! @dahlbyk @jamill The I'm going to update the @csMACnz Amazingly neat package and AWESOME support |
|
TODO:
|
I see you've already done this (cb5d495) but 👍 anyway.
This looks correct now as well. Coverage numbers look correct. Low coverage on exceptions makes it kind of hard to identify actual gaps in coverage. I noticed most of our "for mocking purposes" ctors were untested, so just pushed a commit to |
😍 It's just been published! |
|
@dahlbyk As this looks like working, I'm going to rewrite now both this PR and the |
|
@dahlbyk Done. Let's wait for the report to be published to coveralls.io. Once it's available, how about having this one merged? |
|
|
|
@dahlbyk Thanks for the help and the feedback ❤️ I've dropped all the coverage analysis. One should be automatically run during the next scheduled build (for now, daily at 00:00UTC) |
as part of the scheduled buildupon the merge of a PR