stryker-mutator / stryker-net

Mutation testing for .NET core and .NET framework!
https://stryker-mutator.io
Apache License 2.0
1.75k stars 176 forks source link

Unity support #2381

Open codmw44 opened 1 year ago

codmw44 commented 1 year ago

Is your feature request related to a problem? Please describe.

Add support of Unity 3D. It has C# code but do not use the whole infrastructure of .NET

Describe the solution you'd like

Add special runner for unity. Unity doesn't support concurrent test running. Add special detector of test csproj as Unity uses its own UnityEditor.TestRunner Add able to execute Stryker in folder that contains .sln and csproj at one place Add able to rerun unity tests without closing in batchmode (should save many time by overhead of opening Unity)

Describe alternatives you've considered

I have concerns about cecil. Will it works with Unity DLLs? by csproj it has the signature of .NET Framework 4.7. I made some tests and have warnings with them

[10:57:56 WRN] Stryker is running in single threaded mode due to concurrency being set to 1.
[10:57:56 INF] Identifying project to mutate.
[10:57:57 INF] The project /Users/andrewv/Desktop/DummyProject/Common.ExtensionPoints.PublicAPI.csproj will be mutated.
[10:57:58 INF] Analysis complete.
[10:57:58 INF] Building test project /Users/andrewv/Desktop/DummyProject/Common.ExtensionPoints.Tests.csproj (1/1)
[10:58:13 INF] Total number of tests found: 12.
[10:58:13 INF] Initial testrun started.
[10:58:27 WRN] Original project under test /Users/andrewv/Desktop/DummyProject/Library/ScriptAssemblies/Common.ExtensionPoints.PublicAPI.dll could not be loaded. 
Embedded Resources might be missing.
System.IO.FileNotFoundException: Could not find file '/Users/andrewv/Desktop/DummyProject/Library/ScriptAssemblies/Common.ExtensionPoints.PublicAPI.dll'.
File name: '/Users/andrewv/Desktop/DummyProject/Library/ScriptAssemblies/Common.ExtensionPoints.PublicAPI.dll'
   at Interop.ThrowExceptionForIoErrno(ErrorInfo errorInfo, String path, Boolean isDirectory, Func`2 errorRewriter)
   at Microsoft.Win32.SafeHandles.SafeFileHandle.Open(String path, OpenFlags flags, Int32 mode)
   at Microsoft.Win32.SafeHandles.SafeFileHandle.Open(String fullPath, FileMode mode, FileAccess access, FileShare share, FileOptions options, Int64 preallocationSize)
   at System.IO.Strategies.OSFileStreamStrategy..ctor(String path, FileMode mode, FileAccess access, FileShare share, FileOptions options, Int64 preallocationSize)
   at System.IO.Strategies.FileStreamHelpers.ChooseStrategy(FileStream fileStream, String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, FileOptions options, Int64 preallocationSize)
   at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share)
   at Mono.Cecil.ModuleDefinition.GetFileStream(String fileName, FileMode mode, FileAccess access, FileShare share)
   at Mono.Cecil.ModuleDefinition.ReadModule(String fileName, ReaderParameters parameters)
   at Stryker.Core.Initialisation.EmbeddedResourcesGenerator.LoadModule(String assemblyPath, ILogger logger) in /Users/andrewv/Desktop/stryker-net/src/Stryker.Core/Stryker.Core/Initialisation/EmbeddedResourcesGenerator.cs:line 37
[10:58:28 WRN] Original project under test /Users/andrewv/Desktop/DummyProject/Library/ScriptAssemblies/Common.ExtensionPoints.PublicAPI.dll could not be loaded. 
Embedded Resources might be missing.
System.IO.FileNotFoundException: Could not find file '/Users/andrewv/Desktop/DummyProject/Library/ScriptAssemblies/Common.ExtensionPoints.PublicAPI.dll'.
File name: '/Users/andrewv/Desktop/DummyProject/Library/ScriptAssemblies/Common.ExtensionPoints.PublicAPI.dll'
   at Interop.ThrowExceptionForIoErrno(ErrorInfo errorInfo, String path, Boolean isDirectory, Func`2 errorRewriter)
   at Microsoft.Win32.SafeHandles.SafeFileHandle.Open(String path, OpenFlags flags, Int32 mode)
   at Microsoft.Win32.SafeHandles.SafeFileHandle.Open(String fullPath, FileMode mode, FileAccess access, FileShare share, FileOptions options, Int64 preallocationSize)
   at System.IO.Strategies.OSFileStreamStrategy..ctor(String path, FileMode mode, FileAccess access, FileShare share, FileOptions options, Int64 preallocationSize)
   at System.IO.Strategies.FileStreamHelpers.ChooseStrategy(FileStream fileStream, String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, FileOptions options, Int64 preallocationSize)
   at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share)
   at Mono.Cecil.ModuleDefinition.GetFileStream(String fileName, FileMode mode, FileAccess access, FileShare share)

Additional context

I already started the implementation of this feature at link to diff Right now it works with the next parameters '--unity -c 1 --log-to-file --test-project "./Common.ExtensionPoints.Tests.csproj" --project "Common.ExtensionPoints.PublicAPI.csproj"'

rouke-broersma commented 1 year ago

Add special runner for unity. Unity doesn't support concurrent test running.

We disable concurrent test running for known test frameworks. Does unity not support vstest (dotnet test) for running unit tests?

Add special detector of test csproj as Unity uses its own UnityEditor.TestRunner

Do you mean supplement the test project auto detection with a specific indicator that it's a unity test project?

Add able to execute Stryker in folder that contains .sln and csproj at one place

I would think this already works. Does it not?

Cecil

Cecil simply inspects dotnet DLLs, since Unity is dotnet it should work fine I would think. Furthermore the only reason we use Cecil is so we can extract embedded resources. This seems especially necessary for Unity projects because it seems likely to me that they have assets as embedded resources (translations, fonts etc) which are required during a unit test. Missing assets likely cause false positive test failures. If cecil does not work for Unity another solution needs to be found for embedded resources.

That said our current way of obtaining embedded resources using Cecil is already super flaky, which is likely the reason why you have errors. This isn't specific to Unity, it just doesn't work on some systems or for some projects, but works for others.

Review

I've looked at your work so far and I'm surprised by how far you are! Before you submit the PR I would like you to remove the whitespace changes you've made as they make reviewing difficult. I also don't like that the user needs to pass the type of project, is there no way to gather this information automatically?

codmw44 commented 1 year ago

@rouke-broersma

We disable concurrent test running for known test frameworks. Does unity not support vstest (dotnet test) for running unit tests?

Unity doesn't. Unity doesn't use the.net infrastructure and uses custom nunit and custom run environment that works only with Unity

Do you mean supplement the test project auto-detection with a specific indicator that it's a unity test project?

I do. Code there

Add able to execute Stryker in folder that contains .sln and csproj at one place I had that issue during tests #2371

Review

Thanks so much for your feedback! I am now still in progress and I have created that issue because of contribute guide. That says to create an issue in advance to know what to expect It's not ready for merging now. There are no tests, some todo comments, and some not finished features.

I also don't like that the user needs to pass the type of project, is there no way to gather this information automatically?

Good point! Will update Unity has some common patterns as having all csproj and sln in the same directory and one of them is Assembly-CSharp.csproj and which contains a lot of references to UnityEngine files. And I think with this pattern we could precisely recognize Unity project.

codmw44 commented 1 year ago

I am still about to add a .sln supporting for unity and have no full understanding of working flow for multiple projects.

sequenceDiagram
    Stryker->>IInitialisationProcess:Initialize process
    IInitialisationProcess->>IInputFileResolver:Inject MutantControl.cs in root
    loop Each project:
        IInitialisationProcess->>IInitialBuildProcess:Build project
        IInitialisationProcess->>ITestRunner:InitialTestRun
        IInitialisationProcess->>MutationTestProcess:Mutate
    end
    loop Each project:
        Stryker->>IMutationTestProcess:Run tests for all mutations in the project
        IMutationTestProcess->>ITestRunner:Run tests for each mutation id. One by one
    end

And the main issue that I have is how to setup ActiveMutantId in MutantControl.cs for each project separately. Each project mutates with Mutant ids 0, 1, 2, 3... and as result, they are duplicates with other project's mutation ids and all these projects use the same MutantControl.cs. As result, when I change ActiveMutantId in MutantControl.cs to 0, then I applied some mutants from some projects. Initially, I think that MutantControl should be individual for each project but in the code, it was injected only one time by design.

May someone help with the understanding flow of choosing ActiveMutantId in multiple projects solutions?

rouke-broersma commented 1 year ago

Solution run does not test multiple projects in parallel, and the mutants are not stored together, so mutant ids do not need to be unique across different projects.

codmw44 commented 1 year ago

But mutants used the same MutantControl.cs and when we choose ActiveMutant 1 then it going to apply two mutants in two projects. I tried to visualize it there

CleanShot 2023-01-29 at 11 52 11@2x
rouke-broersma commented 1 year ago

A mutation test run tests a source project with a set of related test projects, not a test project with the source projects it targets. You have the relationship reversed in your diagram.

Edit: actually mhm you might be right... @richardwerkman ???

codmw44 commented 1 year ago

How it works in case when we have two mutated project and both of them used in a single test project? When we will change ActiveMutantId it will apply to both projects

On Sun, Jan 29, 2023 at 12:26 Rouke Broersma @.***> wrote:

A mutation test run tests a source project with a set of related test projects, not a test project with the source projects it targets. You have the relationship reversed in your diagram.

— Reply to this email directly, view it on GitHub https://github.com/stryker-mutator/stryker-net/issues/2381#issuecomment-1407636775, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACQTHHZKFREOIIJIRQZ4HDLWUZHXXANCNFSM6AAAAAAT3WX4CU . You are receiving this because you authored the thread.Message ID: @.***>

dupdob commented 1 year ago

What you describe here is similar to how beta versions of Stryker worked. But the mutant control logic has been refined several times since then. In a nutshell, Stryker obviously only activate one mutant at any time. Here is the design:

  1. Mutants are identified id, which is not unique across projects and their project/assembly, achieving uniqueness within a solution.

  2. Mutant control is injected within each project assembly with a random namespace. So each project gets its own mutant control instance, and the assembly <-> namespace mapping is ket for reference. While there is no logic to guarantee its uniqueness within the solution, the random part makes a risk of collision negligible.

  3. At run/test time, the Stryker custom data collectors activate the desired mutation by injecting its identifier within the proper static variable, using the appropriate namespace, ensuring there is no conflict.

dupdob commented 1 year ago

May I suggest you open a PR in draft mode? It would lower the bar for us to help you. I had a glance at your work, and it is looking good. I am not familiar enough with Unity to have opinions on how to best support it, but I would like to share a few comments with you, in the hope they may help you.

  1. Supporting coverage based optimization is probably a feature you want to keep. While diff based testing can significantly reduce test time, coverage optimization routinely provides a greater boost.
  2. On the same topic, enabling concurrent test sessions would be high on my priority list
  3. Controlling the active mutation using environment variables can work, but you need to inject the value within the child process, not the current one. You would be facing unexpected behavior when enabling concurrent test sessions. (see https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.processstartinfo.environment?view=net-7.0)
  4. Alternatively, I would contemplate implementing a custom unity test runner that would provide the needed infrastructure to properly support capture coverage and related features (probably a mid, long term goal)
codmw44 commented 1 year ago

It was my initial idea, that it should be injected per each csproj, it also has the sense to have a random namespace. But I see that CodeInjection.HelperNamespace has only one write access at the static ctor of CodeInjection CleanShot 2023-01-29 at 16 00 30

Btw, maybe the key is in my issue is that all csproj are placed in the same directory, as result, all of them have only one rootFolderComposite and I have only one injection

CleanShot 2023-01-29 at 16 04 11 CleanShot 2023-01-29 at 16 05 55

codmw44 commented 1 year ago

Supporting coverage based optimization is probably a feature you want to keep. While diff based testing can significantly reduce test time, coverage optimization routinely provides a greater boost.

Good point! I'm focusing on the base working with unity projects now. I will take a look at it, and how I might add it

On the same topic, enabling concurrent test sessions would be high on my priority list

Unfortunately, Unity doesn't support opening multiple Unity instances on the same project. And doesn't support parallel test runs (btw it even has no supporting of Task tasks on the non-experimental test frameworks) I did every manipulation with unity without reopening Unity. For each test run, recompiling saves 1-2 min for each test run. That significantly decreases the working time I know only one way to run tests parallel in Unity, which is to duplicate the whole project with a cache and run on that copy. In my experience, the success of this strategy is based on the speed of hardware memory and the size of the project. Our project with cache weights ~20-30 gb, and on some build agents duplicating takes 15 min, which may decrease the benefit of such strategy.

Controlling the active mutation using environment variables can work, but you need to inject the value within the child process, not the current one. You would be facing unexpected behavior when enabling concurrent test sessions. (see https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.processstartinfo.environment?view=net-7.0)

You are absolutely correct, I already faced this issue during development. I suppose, that I have to deal with it for supporting mutations of some projects

Alternatively, I would contemplate implementing a custom unity test runner that would provide the needed infrastructure to properly support capture coverage and related features (probably a mid, long term goal

Fortunately (or not), Unity has a own package for this, it may be easily auto-added in a project with Stryker.UnitySDK. I suppose that the main difficulty would be in parsing reports

dupdob commented 1 year ago

I must confess I never tried nor dig into solution support so far. But your finding are surprising, I need to dig this further. It looks like you are onto something here.

Thanks for the detailed feedback.

dupdob commented 1 year ago

Fortunately (or not), Unity has a own package for this, it may be easily auto-added in a project with Stryker.UnitySDK. I suppose that the main difficulty would be in parsing reports

By the way, Stryker does mutation coverage analysis, which is different from source code coverage analysis. I am not even sure we could use line coverage analysis to establish the mutation coverage.

But this data is easy to gather if you have your own test runner or a way to have real time notification from an existing test runner. We use the later for Stryker as of today, being able to rely on our own test runners would help solve many issues and improve performance further. But this is not a realistic goal as of today.

codmw44 commented 1 year ago

@rouke-broersma was this issue closed by mistake? I checked #2400 but didn't find any solutions special for Unity As far as I remember #2400 is a precondition for multi-project mutant injection for Unity support

rouke-broersma commented 1 year ago

Yep sorry about that