Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deterministic build, Coverlet throws a KeyNotFoundException when ContinuousIntegrationBuild = true. #882

Closed
chtoucas opened this issue Jun 21, 2020 · 20 comments · Fixed by #895
Labels
bug Something isn't working

Comments

@chtoucas
Copy link

chtoucas commented Jun 21, 2020

Hello, when I try to run Coverlet (MSBuild or VSTest integration)
with UseSourceLink = true and ContinuousIntegrationBuild = true, it throws a KeyNotFoundException.

Data collector 'XPlat code coverage' message: [coverlet]Coverlet.Collector.Utilities.CoverletDataCollectorException: CoverletCoverageDataCollector: Failed to get coverage result
 ---> System.Collections.Generic.KeyNotFoundException: The given key '' was not present in the dictionary.
   at System.Collections.Generic.Dictionary`2.get_Item(TKey key)
   at Coverlet.Core.Coverage.GetSourceLinkUrl(Dictionary`2 sourceLinkDocuments, String document) in /_/src/coverlet.core/Coverage.cs:line 452
   at Coverlet.Core.Coverage.CalculateCoverage() in /_/src/coverlet.core/Coverage.cs:line 350
   at Coverlet.Core.Coverage.GetCoverageResult() in /_/src/coverlet.core/Coverage.cs:line 141
   at Coverlet.Collector.DataCollection.CoverageWrapper.GetCoverageResult(Coverage coverage) in /_/src/coverlet.collector/DataCollection/CoverageWrapper.cs:line 44
   at Coverlet.Collector.DataCollection.CoverageManager.GetCoverageResult() in /_/src/coverlet.collector/DataCollection/CoverageManager.cs:line 93
   --- End of inner exception stack trace ---
   at Coverlet.Collector.DataCollection.CoverageManager.GetCoverageResult() in /_/src/coverlet.collector/DataCollection/CoverageManager.cs:line 98
   at Coverlet.Collector.DataCollection.CoverletCoverageCollector.OnSessionEnd(Object sender, SessionEndEventArgs e) in /_/src/coverlet.collector/DataCollection/CoverletCoverageCollector.cs:line 160.

Here is a very simple repository that reproduces the error. The exact commands that I use are:

dotnet test /p:CollectCoverage=true /p:UseSourceLink=true /p:ContinuousIntegrationBuild=true

and

dotnet test --collect:"XPlat Code Coverage" --settings coverlet.USL.runsettings /p:ContinuousIntegrationBuild=true

If I use ContinuousIntegrationBuild = false, everything works fine.

Context:

  • .NET Core v3.1.300+
  • coverlet.collector v1.3.0 or coverlet.msbuild v2.9.0 with patch for CoverletGetPathMap
  • Microsoft.SourceLink.GitHub v1.0.0
@MarcoRossignoli MarcoRossignoli added the untriaged To be investigated label Jun 22, 2020
@MarcoRossignoli
Copy link
Collaborator

Can you confirm that if you remove UseSourceLink = true it works?

@chtoucas
Copy link
Author

Actually no. It works when UseSourceLink = true and ContinuousIntegrationBuild = false.
The only case that does not work is when UseSourceLink = true and ContinuousIntegrationBuild = true.

@chtoucas
Copy link
Author

Oh sorry, I think I misread your question. Yes it works fine when I remove UserSourceLink = true and whether I set ContinuousIntegrationBuild to true or false.

@MarcoRossignoli
Copy link
Collaborator

Yep I mean UserSourceLink = false(or removed) and ContinuousIntegrationBuild = true

@chtoucas
Copy link
Author

Just to be crystal clear, the default values for ContinuousIntegrationBuild and UseSourceLink being false, the following commands succeed:

dotnet test /p:CollectCoverage=true
dotnet test /p:CollectCoverage=true /p:ContinuousIntegrationBuild=true
dotnet test /p:CollectCoverage=true /p:UseSourceLink=true

This one fails:

dotnet test /p:CollectCoverage=true /p:UseSourceLink=true /p:ContinuousIntegrationBuild=true

@MarcoRossignoli
Copy link
Collaborator

Ok the issue is source link plus deterministic build. Need to investigate further this scenario, thanks for reporting this.

@chtoucas
Copy link
Author

Upon further investigations, I think the problem is with generate files.

If we force DeterministicSourcePaths to false, things work again.
The build won't be fully deterministic but Coverlet will work fine when using Source Link on a CI server.

dotnet test /p:CollectCoverage=true /p:UseSourceLink=true /p:ContinuousIntegrationBuild=true /p:DeterministicSourcePaths=false

@MarcoRossignoli
Copy link
Collaborator

Thanks a lot for investigation!

@gep13
Copy link

gep13 commented Jul 6, 2020

@MarcoRossignoli as requested on Twitter here:

https://twitter.com/MarcoRossignoli/status/1280043730960093184

I have updated my build to no longer pass in /p:UseSourceLink=true and I can confirm that the build that was failing, is now succeeding, and coverage reports are now successfully being pushed to codecov.io, where they were failing before.

Thanks for the tip. Let me know if there is anything that I can do to help with the testing of this issue.

@MarcoRossignoli
Copy link
Collaborator

Thanks for the test!

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented Jul 10, 2020

Thanks @chtoucas can repro on my local, I'll take a look asap, sorry guys but are very busy months I'm near prod release on my daily job so you know what I mean 🙏
BTW found the issue, fix shouldn't be so hard, confirm that the problem is that deterministic build generates "deterministic" path inside sourcelinks metada, so we need to translate also those.
Dev notes https://github.com/coverlet-coverage/coverlet/blob/master/src/coverlet.core/Coverage.cs#L356

@MarcoRossignoli MarcoRossignoli added bug Something isn't working and removed untriaged To be investigated labels Jul 10, 2020
@MarcoRossignoli
Copy link
Collaborator

Hi guys can someone of you give it a try using our nightly(tomorrow we release every midnight)? https://github.com/coverlet-coverage/coverlet/blob/master/Documentation/ConsumeNightlyBuild.md

@gep13
Copy link

gep13 commented Jul 11, 2020

@MarcoRossignoli we (or rather @AdmiringWorm) have a test ready to go. We will execute this once the latest nightly version is available. Thanks for looking into this, really appreciate it!

@AdmiringWorm
Copy link

@MarcoRossignoli just finished the tests I had set up, and everything seems to work as it should with the latest nightly.

Tried both with and without the workaround mentioned here.
Without the workaround, no error would be shown, but the generated report would be empty (I expected that to happen).
With the workaround, everything would work as expected, with no error, correct coverage report, and so on.

NOTE: These tests were run using the coverlet MSBuild version

@chtoucas
Copy link
Author

Thanks a lot @MarcoRossignoli! I tried with two repositories, one being rather complex, and everything works fine with either coverlet.msbuild or coverlet.collector, and with ContinuousIntegrationBuild and UseSourceLink being set to true or false.

By the way, we still need to patch CoverletGetPathMap when DeterministicSourcePaths or ContinuousIntegrationBuild is set to true. This doc says that it is not necessary with .NET 3.1.300+, but it is, otherwise one would get an empty CC report.

@MarcoRossignoli
Copy link
Collaborator

This doc says that it is not necessary with .NET 3.1.300+, but it is, otherwise one would get an empty CC report.

cc: @clairernovotny

@gep13
Copy link

gep13 commented Jul 12, 2020

@MarcoRossignoli do you have any estimate as to when you will be looking to publish a new version to NuGet.org? Thanks

@MarcoRossignoli
Copy link
Collaborator

We planned to release evey quarter, this time we release before for deterministic feature, we could release a fix or a new version as planned(I've a PR ready with a new feat also --skipautoprops).
I need to talk to @tonerdo (we have some versions change planned) but not clear agreement yet .
#874
https://github.com/coverlet-coverage/coverlet/blob/master/Documentation/ReleasePlan.md#release-calendar

I'll let you know, for now you can use nightly.

@clairernovotny
Copy link
Contributor

There are two issues that 300 fixed:

  1. the need for the workarounds in for the generated files
  2. enabling the base targets to return the SourceRoot items so the coverlet target workarounds aren't needed.

@MarcoRossignoli were you able to update coverlet to use the built-in target to get the SourceRoot items instead of requiring people to add something to their directory targets?

@MarcoRossignoli
Copy link
Collaborator

Will take a look asap.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants