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

fix: Conditional access mutations could lead to unrecoverable NullReferenceException in compilation #2888

Merged
merged 5 commits into from
Mar 20, 2024

Conversation

dupdob
Copy link
Member

@dupdob dupdob commented Mar 19, 2024

Fix issues related to Conditional Access (and NullSuppression operator)
Add recovery mechanism for when the compilation triggers null reference exception

Fix #2880

@dupdob dupdob force-pushed the fix_conditional_access_again branch from 2a31da4 to 4a2642f Compare March 19, 2024 13:15
@rouke-broersma rouke-broersma changed the title Fix conditional access fix: Conditional access mutations could lead to unrecoverable NullReferenceException in compilation Mar 19, 2024
rouke-broersma
rouke-broersma previously approved these changes Mar 19, 2024
@richardwerkman
Copy link
Member

I've tested this and it seems to work :)

But I'm still seeing some error logging:

[17:23:35 ERR] Some mutations failed to be inserted, they are dropped.
[17:23:35 ERR] Some mutations failed to be inserted, they are dropped.
[17:23:35 ERR] Some mutations failed to be inserted, they are dropped.
[17:24:53 ERR] VsTestRunner: Coverage collector error: Did not find type StrykerH7r178e2ZZDsDvd.MutantControl. This indicates Stryker failed to copy the mutated assembly for test..
[17:24:53 ERR] VsTestRunner: Coverage collector error: Did not find type StrykerH7r178e2ZZDsDvd.MutantControl. This indicates Stryker failed to copy the mutated assembly for test..

When was this added? And should we fix these in this PR? Or can we fix this later

@dupdob
Copy link
Member Author

dupdob commented Mar 20, 2024

Thanks for reporting.

I've tested this and it seems to work :)

But I'm still seeing some error logging:

[17:23:35 ERR] Some mutations failed to be inserted, they are dropped.
[17:23:35 ERR] Some mutations failed to be inserted, they are dropped.
[17:23:35 ERR] Some mutations failed to be inserted, they are dropped.

This is not new, there was several way for mutations to be silently loss along the way. My last refactoring made the process more trustworthy.
That being said, this should not be reported as an error. I fill fix that.

[17:24:53 ERR] VsTestRunner: Coverage collector error: Did not find type StrykerH7r178e2ZZDsDvd.MutantControl. This indicates Stryker failed to copy the mutated assembly for test..
[17:24:53 ERR] VsTestRunner: Coverage collector error: Did not find type StrykerH7r178e2ZZDsDvd.MutantControl. This indicates Stryker failed to copy the mutated assembly for test..

I think I added this in my refactor PR while analyzing some strange issues. At first I assumed there was a problem copying the mutated assembly, but it turned out to be that Stryker was running unrelated test

Will fix in this PR

@dupdob
Copy link
Member Author

dupdob commented Mar 20, 2024

Continuing comment:

[17:24:53 ERR] VsTestRunner: Coverage collector error: Did not find type StrykerH7r178e2ZZDsDvd.MutantControl. This indicates Stryker failed to copy the mutated assembly for test..
[17:24:53 ERR] VsTestRunner: Coverage collector error: Did not find type StrykerH7r178e2ZZDsDvd.MutantControl. This indicates Stryker failed to copy the mutated assembly for test..

So these are triggered by the mutated assembly not being tested by any test. It can happen with solution mode due to the support for transitive testing. In Stryker's case, these are triggered by Stryker.RegexMutators.
This messages could also be triggered by the mutated assembly failing to copy for some reason

==> I downgraded severity down to Debug and stressed the lack of testing as the probable cause

@dupdob
Copy link
Member Author

dupdob commented Mar 20, 2024

@richardwerkman : it should be ok now, could please check it again and confirm on your side?

@dupdob dupdob force-pushed the fix_conditional_access_again branch from 6fca491 to 45cc7ac Compare March 20, 2024 08:28
@dupdob dupdob force-pushed the fix_conditional_access_again branch from 45cc7ac to c976a29 Compare March 20, 2024 08:59
Copy link

sonarcloud bot commented Mar 20, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
51.8% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

@rouke-broersma rouke-broersma merged commit eb8167a into master Mar 20, 2024
8 of 9 checks passed
@rouke-broersma rouke-broersma deleted the fix_conditional_access_again branch March 20, 2024 10:57
@richardwerkman
Copy link
Member

@dupdob thanks for the quick fix! Great work 👍

@dupdob
Copy link
Member Author

dupdob commented Mar 21, 2024

your feedback is welcomed 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stryker failing with NullReferenceException
3 participants