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

Upgrading from 4.0.6 to 4.2.0 analysis no longer works for project #3038

Closed
martincostello opened this issue Sep 18, 2024 · 17 comments · Fixed by #3039
Closed

Upgrading from 4.0.6 to 4.2.0 analysis no longer works for project #3038

martincostello opened this issue Sep 18, 2024 · 17 comments · Fixed by #3039
Labels
🐛 Bug Something isn't working

Comments

@martincostello
Copy link

martincostello commented Sep 18, 2024

Describe the bug

Attempting to upgrade from 4.0.6 to 4.2.0, analysis is failing for one of our test projects, causing an error.

Specifying --dev-mode doesn't appear to add any additional details.

4.0.6

Running mutation tests for '../src/Polly/Polly.csproj'. Test Project: 'Polly.Specs/Polly.Specs.csproj'

   _____ _              _               _   _ ______ _______  
  / ____| |            | |             | \ | |  ____|__   __| 
 | (___ | |_ _ __ _   _| | _____ _ __  |  \| | |__     | |    
  \___ \| __| '__| | | | |/ / _ \ '__| | . ` |  __|    | |    
  ____) | |_| |  | |_| |   <  __/ |    | |\  | |____   | |    
 |_____/ \__|_|   \__, |_|\_\___|_| (_)|_| \_|______|  |_|    
                   __/ |                                      
                  |___/                                       


Version: 4.0.6

A new version of Stryker.NET (4.2.0) is available. Please consider upgrading 
using `dotnet tool update -g dotnet-stryker`

[09:02:00 INF] Analysis starting.
[09:02:06 WRN] Could not find a project analysis for the chosen target framework net8.0. 
The available target frameworks are: net462,net472,net6.0,netstandard2.0. 
The first available framework will be selected, which is net462.
[09:02:06 INF] Found project D:\a\Polly\Polly\src\Polly\Polly.csproj to mutate.
[09:02:06 INF] Analysis complete.
[09:02:06 INF] Building test project D:\a\Polly\Polly\test\Polly.Specs\Polly.Specs.csproj (1/1)
[09:03:18 INF] Number of tests found: 1986 for project D:\a\Polly\Polly\src\Polly\Polly.csproj. Initial test run started.

4.2.0

Running mutation tests for '../src/Polly/Polly.csproj'. Test Project: 'Polly.Specs/Polly.Specs.csproj'

   _____ _              _               _   _ ______ _______  
  / ____| |            | |             | \ | |  ____|__   __| 
 | (___ | |_ _ __ _   _| | _____ _ __  |  \| | |__     | |    
  \___ \| __| '__| | | | |/ / _ \ '__| | . ` |  __|    | |    
  ____) | |_| |  | |_| |   <  __/ |    | |\  | |____   | |    
 |_____/ \__|_|   \__, |_|\_\___|_| (_)|_| \_|______|  |_|    
                   __/ |                                      
                  |___/                                       


Version: 4.2.0

[12:05:57 INF] Analysis starting.
[12:05:57 INF] Analyzing 1 test project(s).
[12:06:11 INF] Analysis complete.
[12:06:11 INF] Time Elapsed 00:00:13.4378512
Stryker.NET failed to mutate your project. For more information see the logs 
below:


No project references found. Please add a project reference to your test project
and retry.

Logs

See the mutation-report artifact here.

Expected behavior

Mutation tests work.

Desktop (please complete the following information):

  • OS: Windows
  • Type of project: framework and core
  • Framework Version: net462,net472,net6.0,netstandard2.0
  • Stryker Version: 4.2.0

Additional context

Failing upgrade: App-vNext/Polly#2293

@martincostello martincostello added the 🐛 Bug Something isn't working label Sep 18, 2024
@rouke-broersma
Copy link
Member

Looks like we used to be unable to properly detect some information, falling back to a default that happens to work. I suspect that we are now able to find a result we can work with, which happens to actually be incorrect. Strange. I base this on these logs that used to appear:

[09:02:06 WRN] Could not find a project analysis for the chosen target framework net8.0. 
The available target frameworks are: net462,net472,net6.0,netstandard2.0. 
The first available framework will be selected, which is net462.

I see you also changed the target frameworks ordering, did you do this after you noticed stryker failed or before?

@dupdob
Copy link
Member

dupdob commented Sep 18, 2024

humm
target framework selection has been redesigned to provide more consistent results, i.e ensuring Stryker picks compatible framework versions across projects in a solution.
fallback logic may have changed, or dropped altogether.
Update: there is no visible issue and their is a dedicated unit test for this situation.

will try to reproduce it via integration testing

@martincostello
Copy link
Author

I see you also changed the target frameworks ordering, did you do this after you noticed stryker failed or before?

It failed before, and I swapped it to see if it would fix it, but alas it didn't.

@dupdob
Copy link
Member

dupdob commented Sep 18, 2024

I do have a problem when specifying a non supported target framewok, but it fails differently, and I have no idea why.

I am surprised that --def-mode does not provide extra information. Both hints toward an non anticipated failure mode.
For the short time, I will fix the problem I reproduce.

@dupdob
Copy link
Member

dupdob commented Sep 18, 2024

I opened a PR where Stryker no longer asks for the configured target framework when analyzing a project.
I assume this will fix this issue, but bear in mind that when I tried to reproduce this issue, I did get extra error messages (such as a warning because project analysis failed).

@martincostello
Copy link
Author

Thanks - if there are genuine issues with our setup I'm fine with us having to fix through them to take the upgrade (e.g. tweaking settings).

@dupdob
Copy link
Member

dupdob commented Sep 18, 2024

well, if could either adapt the target framework option so that it matches one that is an actual target for the project.
Or get rid of it. Because, the fix will revert the previous behavior which is to pick one framework at random 😄

@martincostello
Copy link
Author

I don't fully understand the previous comment. The test project targets net8.0, but the library doesn't because it doesn't need to. We could add net8.0 but there's no functional need for it. The same could be said for a library exclusively targeting netstandard2.0 but tested through a net8.0 project. There's no always a scenario where the code being mutated targets the same TFM as its corresponding tests, and we want to mutate what the library targets, not add a target just to make Stryker work, as then we're testing code that users don't necessarily use as we're artificially adding a TFM for the purposes of tests.

@rouke-broersma
Copy link
Member

I don't fully understand the previous comment. The test project targets net8.0, but the library doesn't because it doesn't need to. We could add net8.0 but there's no functional need for it. The same could be said for a library exclusively targeting netstandard2.0 but tested through a net8.0 project. There's no always a scenario where the code being mutated targets the same TFM as its corresponding tests, and we want to mutate what the library targets, not add a target just to make Stryker work, as then we're testing code that users don't necessarily use as we're artificially adding a TFM for the purposes of tests.

The problem is that you're explicitly specifying that you wish to target net8.0 and we're trying to respect that, but your library does not actually have that target: https://github.com/App-vNext/Polly/blob/5377ae6003739390091a570bf7870c6880bfd571/eng/stryker-config.json#L22

In that case we used to fall back to 'random TFM'.
Eg:

[09:02:06 WRN] Could not find a project analysis for the chosen target framework net8.0. 
The available target frameworks are: net462,net472,net6.0,netstandard2.0. 
The first available framework will be selected, which is net462.

We no longer do this. Now this doesn't seem to be entirely intentional as that is a breaking change as we can see and we did not consider that. So we can re-add the previous logic where if the requested TFM does not exist, we choose a 'random' TFM to target.

However in essence your configuration is invalid, because you're requesting a TFM that does not exist. So the question is, is it more valid in that case to then hard-fail, or is it more valid to choose a random TFM and keep going.

WDYT?

@martincostello
Copy link
Author

Will stryker "do the right thing" if we remove the explicit TFM from the config completely? Our projects have different available TFMs (by design) and I'd rather avoid needing to have per-project stryker config (as 99% if it will be identical).

@martincostello
Copy link
Author

I tried just removing it, but that breaks a completely different project:

[12:29:02 INF] Analysis starting.
[12:29:02 INF] Analyzing 1 test project(s).
[12:29:15 INF] Found project D:\a\Polly\Polly\src\Polly.Testing\Polly.Testing.csproj to mutate.
[12:29:15 INF] Analysis complete.
[12:29:15 INF] Building test project D:\a\Polly\Polly\test\Polly.Testing.Tests\Polly.Testing.Tests.csproj (1/2)
[12:29:16 INF] Time Elapsed 00:00:13.5617568
Stryker.NET failed to mutate your project. For more information see the logs 
below:


Stryker could not build your project as no solution file was presented. Please 
pass the solution path to stryker.

@dupdob
Copy link
Member

dupdob commented Sep 23, 2024

The test project targets net8.0, but the library doesn't because it doesn't need to. We could add net8.0 but there's no functional need for it

Thanks, that was the key missing information here. I should have though about this. This explains why I have a different result. And it looks like I do reproduce the issue as you experience it, so I should be able to fix it properly now.

Yes, Stryker should work seamlessly, as long as the test projects do support the selected target framework. The change introduced between V4.06 and V4.2 aimed at actually removing the random framework selection that happens for the tested project. To achieve that, Stryker picks the exact target project configuration resolved by the test project.
It looks like we do something wrong here, probably by checking that selected configuration uses the configured target framework.

The problem you face when no longer picking a target framework is that Stryker may pick a net framework version (and not a core one), and then it needs the solution for building it.

@dupdob
Copy link
Member

dupdob commented Sep 23, 2024

Disclaimer: I reproduce this issue using Stryker integration test projects. They normally target net 8.0, but I have modified the 'tested project' to target net7.0 instead (and using our XUnit test project as the starting point, to be specific).
With this setup, I reproduce the issue. The fun part is that project analysis report a successful result when requesting for .net8.0 version of the project. Alas, the project is not actually built during a normal build, so the run fails (no assembly found).

I have been able to resolve this issue, by requesting .net8.0 only on the test project. And voila, it does work. I still have a warning about the being able to use .net8.
I will look into this to decide if it is better to remove it or not.

Note that this fix works for single project mode, I sill have to look into solution mode, where it may be a bit trickier

@dupdob
Copy link
Member

dupdob commented Sep 26, 2024

quick update:
I finalized PR #3039 that should fix this problem. If you are in a position where you can do this, feel free to build it and try my PR on your chain for confirmation; or failure details if there are still issues.
But I am pretty confident you will be satisfied

@martincostello
Copy link
Author

Thanks @dupdob - do you have a pre-release feed I can pull the fix down from pre-compiled? Otherwise I think it'll be a bit too many hoops to jump through to validate it against our build.

@dupdob
Copy link
Member

dupdob commented Sep 26, 2024

I don't think there is any feed available for that. No problem anyway

@martincostello
Copy link
Author

Confirmed fixed with 4.3.0 - thanks.

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
3 participants