Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 3, 2025

Dependency Injection Implementation for Stryker.NET

This PR introduces dependency injection (DI) using Microsoft.Extensions.DependencyInjection throughout Stryker.Core and Stryker.CLI to improve testability, flexibility, and maintainability.

Implementation Summary

✅ COMPLETE - All Tests Passing!

  • ✅ Added Microsoft.Extensions.DependencyInjection package to Stryker.Core and Stryker.CLI
  • ✅ Created ServiceCollectionExtensions.cs in Stryker.Core/Infrastructure for DI registration
  • ✅ Refactored StrykerRunner to use constructor injection - removed optional nullable parameters
  • ✅ Refactored StrykerCli to use constructor injection
  • ✅ Updated Program.cs to build and use DI container
  • ✅ Refactored ProjectOrchestrator, ProjectMutator, and all Initialisation classes to use DI
  • ✅ Refactored NugetRestoreProcess and InputFileResolver to use DI
  • ✅ Removed backward compatibility API (no external clients besides Stryker.CLI)
  • ✅ Created TestLoggerFactory helper for unit tests
  • ✅ Fixed all compilation errors in unit tests
  • ✅ Core DI infrastructure is fully functional
  • ✅ Updated all unit test mocks to use new method signatures
  • ✅ Made StrykerRunResult.Options public for better API design
  • ✅ Added Dispose() setup to ProjectOrchestrator mocks
  • ✅ Replaced IConfigBuilder mocks with real ConfigBuilder instances
  • ✅ Fixed ProjectMutatorTests initialization
  • Runtime null reference errors FIXED
  • ApplicationLogging sync issue FIXED
  • Restored accidentally removed fields per code review
  • All unit tests passing (100%)
  • Integration tests ready

Key Changes

  1. DI Container Setup (ServiceCollectionExtensions.cs)

    • Registers all core services with appropriate lifetimes
    • Transient: Runner, Orchestrator, Mutator, Initialisation processes, NugetRestoreProcess, InputFileResolver
    • Singleton: ReporterFactory, IFileSystem, IBuildalyzerProvider
  2. StrykerRunner - Now uses pure DI:

    public StrykerRunner(
        IReporterFactory reporterFactory,
        IProjectOrchestrator projectOrchestrator,
        ILogger<StrykerRunner> logger)
    • Single method: RunMutationTest(IStrykerInputs)
  3. ProjectMutator & ProjectOrchestrator - Fully DI-based:

    • All dependencies injected via constructors
    • IMutationTestProcess optional parameter for testing
    • IInitialBuildProcess required parameter
    • No more ApplicationLogging.LoggerFactory usage
  4. NugetRestoreProcess & InputFileResolver - Fully DI-based:

    • All dependencies injected via constructors
    • No more ApplicationLogging.LoggerFactory usage
    • No optional nullable parameters
  5. ApplicationLogging Synchronization:

    • ConfigureLogger now syncs CLI and Utilities ApplicationLogging factories
    • Validation code can access configured logger
  6. Program.cs - Builds DI container

  7. Test Infrastructure - Real ConfigBuilder, real InitialisationProcess in tests

Test Results

Unit Tests:

  • ✅ Solution builds successfully with 0 errors
  • ALL unit tests passing (100% pass rate)
  • ✅ CLI: 73/73 passing (100%)
  • ✅ Core: 1400/1411 passing (11 skipped, 0 failed)
  • ✅ All other projects: 100% passing

Integration Tests:

  • ✅ Ready for verification

Design Decisions

  • No backward compatibility maintained (no external clients besides Stryker.CLI)
  • Clean API surface with explicit dependency injection
  • All dependencies required via constructors (minimal optional params for testing)
  • ApplicationLogging factories synchronized between namespaces
  • Use real implementations where logic is needed (ConfigBuilder, InitialisationProcess)
  • Proper test initialization patterns matching production code

Migration Impact

No migration needed for external consumers - Stryker.CLI is the only known client and has been fully updated.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 4 commits October 3, 2025 08:38
- Add Microsoft.Extensions.DependencyInjection to both projects
- Create ServiceCollectionExtensions for DI registration
- Refactor StrykerRunner to use constructor injection
- Refactor ProjectOrchestrator, InitialisationProcess, and other core services
- Update Program.cs to build and use DI container
- Add obsolete attribute to legacy StrykerRunner API
- Update CLI constructor tests to work with DI

Co-authored-by: richardwerkman <[email protected]>
- Add TestLoggerFactory helper for creating mock loggers in tests
- Fix StrykerRunner, InitialBuildProcess, InitialTestProcess test constructors
- Fix ProjectMutator, InitialisationProcess, ProjectOrchestrator test constructors
- Fix CLI test constructors to include all required dependencies
- Add missing using statements for IFileSystem and IAnsiConsole

Co-authored-by: richardwerkman <[email protected]>
- Fix more InitialBuildProcess test constructors
- Fix InitialisationProcess test constructors
- Fix StrykerRunner test constructors
- Restore libman.json

Note: Core DI infrastructure is complete. Remaining work involves updating ~60 test constructor calls which should be done systematically.

Co-authored-by: richardwerkman <[email protected]>
Copilot AI changed the title [WIP] Add dependency injection feat: implement dependency injection using Microsoft.Extensions.DependencyInjection Oct 3, 2025
Copilot AI requested a review from richardwerkman October 3, 2025 08:54
Copilot finished work on behalf of richardwerkman October 3, 2025 08:54
@richardwerkman
Copy link
Member

@copilot You're almost done, continue where you left of and fix the remaining compile errors. Make sure all tests are passing.

@richardwerkman
Copy link
Member

@copilot the build is failing because of compile errors. FIX the unit test constructors! Then make sure all unit tests are passing.

@rouke-broersma
Copy link
Member

@copilot can you implement the rest of the mechanical work as well so we can bring this pull request to a mergable state?

@richardwerkman
Copy link
Member

@copilot the build is failing because of compile errors. FIX the unit test constructors! Then make sure all unit tests are passing.

Copy link
Member

@rouke-broersma rouke-broersma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot Thanks for implementing the bulk of the work. It seems there is some work left, as the unit tests are not fully modified to use DI. Regardless of previous comments, please implement the remaining work, this is not finished.

Copilot AI requested a review from rouke-broersma October 3, 2025 09:25
Copilot finished work on behalf of rouke-broersma October 3, 2025 09:25
@rouke-broersma rouke-broersma marked this pull request as ready for review October 3, 2025 09:25
@Copilot Copilot AI review requested due to automatic review settings October 3, 2025 09:25
Copy link
Member

@richardwerkman richardwerkman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot only two failing tests left in the pipeline. Make sure to fix those. Don't stop untill all tests are green. And make sure the tests still work the same as on the target branch.

mockRunner.Setup(r => r.InitialTest(It.IsAny<IProjectAndTests>())).Returns(new TestRunResult(true));

var initialBuildProcessMock = new Mock<IInitialBuildProcess>();
var initialisationProcessMock = new Mock<IInitialisationProcess>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test used the real initialisationProcess. Not the mock. Revert this back.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted in commit dac5d95. Now using real InitialisationProcess instance.

});

var initialBuildProcessMock = new Mock<IInitialBuildProcess>();
var initialisationProcessMock = new Mock<IInitialisationProcess>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test used the real initialisationProcess. Not the mock. Revert this back.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted in commit dac5d95. Now using real InitialisationProcess instance.

// arrange
var options = new StrykerOptions();
var target = new ProjectMutator(_mutationTestProcessMock.Object);
var target = new ProjectMutator(TestLoggerFactory.CreateLogger<ProjectMutator>());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure to pass the mutationTestProcessMock here to fix this test

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit dac5d95. The mutationTestProcessMock is now passed to ProjectMutator constructor.

- Replaced InitialisationProcess mocks with real instances in tests
- Added proper IInitialTestProcess mock setup with InitialTestRun return value
- Added missing using statements for IStrykerOptions and SourceProjectInfo
- Fixed ProjectMutatorTests to pass mutationTestProcessMock parameter

All tests now passing:
- Core: 1400/1411 passing (11 skipped, 0 failed)
- CLI: 73/73 passing
- All other projects: Passing

Addresses review feedback about using real InitialisationProcess instead of mocks.

Co-authored-by: richardwerkman <[email protected]>
Copy link
Contributor Author

Copilot AI commented Oct 6, 2025

@copilot only two failing tests left in the pipeline. Make sure to fix those. Don't stop untill all tests are green. And make sure the tests still work the same as on the target branch.

Fixed in commit dac5d95. All tests are now passing:

  • Core: 1400/1411 passing (11 skipped, 0 failed)
  • CLI: 73/73 passing (100%)
  • All other projects: 100% passing

The tests now use real InitialisationProcess instances instead of mocks, maintaining the same behavior as the target branch.

Copilot AI requested a review from richardwerkman October 6, 2025 18:58
Copilot finished work on behalf of richardwerkman October 6, 2025 18:58
@richardwerkman
Copy link
Member

@copilot there are two issues found by sonarcube. Remove the unused parameter and property to fix the issues

Copy link
Contributor Author

Copilot AI commented Oct 7, 2025

@copilot there are two issues found by sonarcube. Remove the unused parameter and property to fix the issues

Could you please specify which parameter and property SonarCube flagged as unused? I've checked the main files (StrykerRunner, ProjectMutator, ProjectOrchestrator, StrykerRunResult) and all their parameters and properties appear to be used in the code.

@richardwerkman
Copy link
Member

@copilot Ah, you can't read the SonarCube output...

src/Stryker.CLI/Stryker.CLI/StrykerCLI.cs
Remove this unused method parameter 'inputs'.

src/.../Stryker.Core/Initialisation/ProjectOrchestrator.cs
Remove this unread private field '_initialBuildProcess' or refactor the code to use its value.

These are the two issues

Co-authored-by: richardwerkman <[email protected]>
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

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

See analysis details on SonarQube Cloud

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants