stryker-net icon indicating copy to clipboard operation
stryker-net copied to clipboard

Starting MsbuildAnalyzer

Open bcollamore opened this issue 3 years ago • 4 comments

This is not worthy of serious consideration, as it is not operational yet. But it gives a flavor for how this could be accomplished. I only present what is available thus far because tomorrow I'm heading out on a family vacation for the next week.

The only serious deviation from my other PR (that abstracts Buildalyzer) is the creation of: BuildLocator.cs, MsBuildProjectAnalyzer.cs, and MsBuildProjectsAnalyzerManager.cs.

bcollamore avatar Feb 20 '22 02:02 bcollamore

Apologies @rouke-broersma for the delay! I finally made my way back to this investigation.

Most (or all?) of the BuildAlyzer replacement logic is in the file MsBuildProjectAnalyzer.cs, with some initialization in BuildLocator.cs.

It successfully ran against a small local sln containing a .NET Core 3.1 project, a .NET Standard 2.0 project, and a .NET Core 3.1 Test Project. I did not wire up multi-targeting, .NET Framework projects, F#, etc. This was just to demonstrate a proof-of-concept as requested for things such as project references, source files, DLL's and preprocessor symbols.

image

If there were an interest in potentially productizing, I recommend the other PR first, which just encapsulates BuildAlyzer w/o replacing it. That constitutes the majority of the file change set. Just let me know if you're interested in pursuing the other PR and I will rebase it and study it again for improvements.

Then this MsBuild analysis option could be added (with more cleanup), and only wired up via a command-line option for anyone interested in beta testing it. I can commit to improving it more to prepare for production if desired.

I do believe that increased cognitive load of not using BuildAlyzer is (more than or less than?) offset by a cleaner API boundary. E.g., I suspect there would be no need to implement Nuget Restore logic between runs, because the output folders should no longer be decimated with this approach. (Buildalyzer's corrupting of the output folder is what originally started me on this journey a couple years ago - I was setting up stryker.net to run in a containerized GitHub Action, and it was requiring nuget restore from within the container, which seemed weird. I worked around the issue by flowing Azure DevOps Package Manager credentials into the container, but for whatever reason I've been intrigued by this BuildAlyzer stuff since then.)

bcollamore avatar Apr 12 '22 20:04 bcollamore

I'll try to take a look tomorrow 👍👍

rouke-broersma avatar Apr 14 '22 06:04 rouke-broersma

I tested this example. It worked ok for our dotnet core integration test project. For some reason there were rollback exceptions so I could not fully test the scenario. Could be that your branch is a couple important bug fixes behind that caused this.

Dotnet framework on a 4.6.1 project did not work, once I upgraded to 4.8 the project compiled successfully but unit tests could not be discovered so no mutation test was executed. However no crashes so that's positive.

The analyzer seems simple enough for now, we already deal with a lot of these concepts these days even though we use Buidalyzer. The main thing I'm still worried about is all the edge cases we do not yet know about and which we won't know about until we have already invested heavily into our own solution. I am not convinced that we will not at some point end up with a copy of Buildalyzer to be able to deal with all possible scenarios.

I will discuss these findings with the team and let you know if we are interested in further efforts on your part. Thank you so much for starting the discussion in any case and for providing an initial reference so we can see how it would shape up, it is appreciated!

rouke-broersma avatar Apr 15 '22 13:04 rouke-broersma

I have been looking at various types of sample projects and have already observed multiple interesting (that is, eye-opening) edge cases. I will document them herein in the next couple days.

bcollamore avatar Apr 21 '22 10:04 bcollamore

@bcollamore I would like to sincerely apologize for leaving your efforts to collect dust for such a long long time. Truth be told I simply did not know what to do with it. On the one hand you've provided a proof of concept that works in some cases, and it might solve some issues we currently have with buildalyzer, but on the other hand I see the effort that has been and continues to be put in to Buildalyzer and I don't want to pull that can of worms into Stryker.

I am very thankful for your contributions and for putting in the time to provide us with these proof of concepts, however I feel the most useful thing for me to do at this point is to reject the proposal and close the PRs. I don't currently have the bandwidth to do anything else with this, and at least then we know where we stand.

Thank you, and again, I am sorry for the way this was handled.

rouke-broersma avatar Nov 25 '22 13:11 rouke-broersma