-
Notifications
You must be signed in to change notification settings - Fork 387
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
[BUG] Coverlet Nightly Crashing when merging JSON results #1583
Comments
Thanks for reporting 🙏 Please use the Packages (ZIP file) from PR build and try the MergeWith operation again. Please share the results. see also https://stackoverflow.com/questions/72268018/jsonconstructor-fails-on-ienumerable-property |
I will try later today if I can, otherwise it will likely need to wait until Monday. Thanks for looking into this. |
@Bertk @MarcoRossignoli @daveMueller Sorry for the delay. I just tested with the packages from the feed linked above and it does not correct the issue:
I think this fix only works for .NET8+ (from this documentation):
I assume this means Coverlet's target framework and not my project's. Coverlet looks to be running on net6.0:
I think another option would be updating the version of S.T.J to the 8.0.x band which should be compatible with .NET6
|
@pinkfloydx33 Could you please check the package from draft build If this does not resolve the issue, I will update |
@Bertk I will test this morning and report back |
@Bertk I can confirm that the packages from the draft build you linked are working as expected |
@Bertk thanks! do you know when this will make it into the nightly builds? |
@pinkfloydx33 The PR is already merged and the new package should be available tomorrow. |
@Bertk great, thanks! |
I should have paid more attention. While it would appear that coverlet no longer crashes when merging JSON, it's not actually merging the results. Only the coverage results from the last test project to run appear in the final results. I don't know when this started happening since the nightly build previously couldn't even run without crashing. Should I open a new issue? It feels like all part of the same thing. This was using nightly (BTW the version you had me download from the Azure Devops pipeline was |
Could you please execute
Tomorrow I will also look for error handling of MergeWith option e.g. path check |
Coverage data is correctly generated for individual projects when not using When using
It's just the |
Please add The path in MergeWith option is not sanitized or normalized. The documentation uses relative path but your example uses |
Relative path is what I'm using locally. The above path was just for sake of the repro. Though in CI we've always used MSBuild properties to dictate the output path, setting I can try adding the AsyncInterfaces to my test project, but I don't see how that would impact coverlet and as a long term solution would probably not be possible (and feel a bit hacky). I'm out of office the rest of the day.. I will give it a try tomorrow morning. The project is using Microsoft.Net.Test.Sdk 17.8.0 |
Looks like there is another serialization problem with Coverlet.Core.Instrumentation.BranchKey.
|
Hey @Bertk I just wanted to let you know that I have tried all of the following (and combinations thereof):
While the coverlet run succeeds with the I did a little further investigation to see if I could figure out when this behavior started. Unfortunately the crashing behavior begins before (or at least in tandem with) the failure to actually merge:
I hope that helps tell you something. 🤞 For now we are going to continue using the stable version. We hope we can start using the nightly again as there's several fixes around records and primary constructors that we are really looking forward to. I appreciate your effort here. Thanks again. |
@Bertk are you already working on this? Sounds interesting. However I will pin this issue as it could break some user pipelines and I think we should probably solve this before we do a new release. |
@daveMueller I agree this has to be fixed before release coverlet and I started with a draft PR #1601. Until now I could not identify the source of the serialization failure. Currently I check the feature of latest System.Text.Json version which supports
This was updated Migrate from Newtonsoft.Json to System.Text.Json By the way, I will be offline until Monday. |
Whatever other serialization issues exist, they don't seem to be occurring 100% of the time (given that coverlet doesn't crash for me anymore). In the cases where a crash does not occur, I'd expect the output of the merge operation to be correct. Or are the other serialization issues trapped/caught and thus don't manifest to the user? I just want to make sure that both sides of this issue are handled (crash and failure to merge on success) |
@pinkfloydx33 I try to reproduce the error and uses a small script. Could you please explain the parameter
By the way, MergeWith does not use |
For the purposes of this issue, Ignore any of the commands I pasted. Just try to merge the JSON results from any two seperate projects (passing the appropriate |
@pinkfloydx33 I did some updates and the Could you please use the nuget package from PR #1601 build and check this with your use case (PR nuget packages). |
Yes I will try later this morning. Sorry about the confusion, as stated it was just a bad copy/paste on my part. |
@Bertk It works, but it also doesn't. Let me explain: If I pass
The only way it works is if I don't pass Running the projects individually with different arguments does produce a properly merged coverage results file. |
@pinkfloydx33 I replaced the exception with a information logging. Please check again PR nuget packages. |
@Bertk I can confirm that everything is now working as expected! |
@Bertk I don't know what made me think it was working. I re-tested with the package mentioned in #1583 (comment) but the coverage data is not being merged. See my comments on PR #1601 for some more details My apologies. I swear I tried running it multiple times to validate that it was working.... I think I was thrown off by an erroneous |
@pinkfloydx33 Thanks you for providing the results. I did one successful test with coverlet projects (see PR #1601) and this looks OK. |
@Bertk I can confirm that the latest nightly is working as expected! No more crashing and json results are properly merged. Thanks for all your work on this. It's much appreciated! We should be able to close this (and unpin it @daveMueller ) |
Describe the bug
When running the coverlet nightly builds the generation of results fails when combining multiple test runs using JSON format. Individual runs generate reports successfully. The exception thrown is:
Depending on the order in which I run the projects, the first line of the message will include multiple Type and/or method names
To Reproduce
Run coverlet nightly build (
6.0.1-preview.31.g0db8de77cb
) against a project and usejson
output format. Then, run coverlet against another project (or even the same one!) with theMergeWith
option.Expected behavior
Coverlet does not crash
Actual behavior
Coverlet crashes with the above exception
Configuration (please complete the following information):
Please provide more information on your .NET configuration:
* Which coverlet package and version was used?
6.0.1-preview.31.g0db8de77cb
(also crashes on6.0.1-preview.26.g71c202b19e
)* Which version of .NET is the code running on?
.NET7
but .NET SDK 8.0.0 is installed and active* What OS and version, and what distro if applicable? Windows 11 x64, Ubuntu 22.04
* What is the architecture (x64, x86, ARM, ARM64)? x64
* Do you know whether it is specific to that configuration? Occurs on at least two configurations mentioned
Additional context
Works fine with package versions
6.0.0
The text was updated successfully, but these errors were encountered: