stryker-net
stryker-net copied to clipboard
dev(buildalyzer): Begin encapsulating Buildalyzer
Start encapsulating Buildalyzer, so as to begin exploring possible alternatives in the future.
- New interfaces/classes hierarchy to encapsulate Buildalyzer classes
- ProjectOrchestrator is updated - it's an entry point to the new classes.
- ProjectFileReader - it's not updated; kept old API for now (updating this API to fully encapsulate Buildalyzer would be next step.)
- Added an AnalyzerOption: currently Buildalyzer is the only supported option. (This arguably should have been excluded for now for smaller batch size.)
Other minor changes:
- IAnalyzerResultExtension moved from \Buildalyzer folder. Minimal changes, just added "ToAnalyzerResult", which is needed for now to keep ProjectFileReader on the previous API.
- Buildalyzer.cs moved, repurposed, and renamed to SolutionAnalyzerManagerProvider.cs
- TestHelper.cs: removed PackageReferences as it's not used elsewhere
Hi @bcollamore
We have discussed this internally and we have a few concerns:
- We don't currently know how replacing buildalyzer would work so it would be difficult for us to implement and support this. We would like a proof of concept that shows that it is possible to replace buildalyzer with the roslyn and/or msbuild apis before we introduce a massive abstraction that might not end up being used.
- We are concerned that we would be recreating large parts of Buildalyzer in Stryker, and would from then on be responsible for maintaining a Buildalyzer clone. Right now we have a 3rd party that provides this abstraction for us. A third party with a significant amount of knowledge about this ecosystem that we can rely on. We don't have the same level of understanding.
- I am not convinced that any replacement of Buildalyzer will not have the same issues Buildalyzer has. A proof of concept could alleviate these concerns.
To be clear we are not saying we need to see an implementation of Stryker using this alternative as a proof of concept. But we do want to see a small application that implements these API's and discovers the required information about a project. This would for example list project references, source files, DLL's and preprocessor symbols.
- Updated ProjectFileReader to use abstraction layer. (Now 1 place where Buildalyzer is directly used.)
- Removed AnalyzerOption: (Explicitly only support Buildalyzer.)
- Move Logging of AnalyzerResult details into AnalyzerResult
- BuildalyzerProjectAnalyzer: Move ProjectFileReader targetFramework logic here
- BuildalyzerProjectAnalyzerManager: Move some ProjectFileReader logic here
Understood @rouke-broersma. In the meantime I completed encapsulation of Buildalyzer (i.e., porting ProjectFileReader to use the abstraction layer). There would seem to be benefits of encapsulating Buildalyzer independent of replacing it, and the Buildalyzer API already has some cognitive load associated with it. For example,
- Uses IAnalysisResults despite Stryker only needing one result.
- Redundant/mixed logic between ProjectFileReader and ProjectAnalyzerManager (ie solution analyses) (eg., TargetFramework support in one path only, one path has try/catch, one path has logging, one path uses NugetRestore on failure)
- Buildalyzer seems to be analyzing the same project multiple times (when solution analyses performed)
It's reasonable to believe that a cleaner abstraction layer could simplify the interface with Buildalyzer.
I had an unpolished experiment from ~18 months ago that uses MsBuild in lieu of Buildalyzer. It was not ready for prime time. It's also entirely possible the additional work could create excessive support situations, as you point out. I will dust that off, hopefully soon, to see the proof of concept. It combined the work of encapsulating Buildalyzer and replacing it. This PR focused on the former, to open future investigations, in an attempt to help issues such as https://github.com/stryker-mutator/stryker-net/issues/466 and worked on by @pawelhevy.
Having said all that, I understand if this PR is deemed unworthy or too risky. Stay tuned for a POC on replacing Buildalyzer.
See https://github.com/stryker-mutator/stryker-net/pull/1919 for an as-of-yet non-operational proof-of-concept for using MsBuild for analysis. I can try to make it functional after I return from a vacation in a week.
See the aforementioned PR for a now-operational proof-of-concept of how replacing BuildAlyzer can work (or more specifically, how a compile-time or run-time choice could be made to use BuildAlyzer or MsBuild).