consolidate: move co-located test projects to tests/ and fill coverage gaps#493
Merged
Conversation
…e gaps Move all test projects from src/c#/ to tests/: - BowlTest, DifferentialTest, DrivelutionTest, ExtensionTest Add missing unit test coverage: - Core: UpgradeHubService, HttpDownloadSource, OssDownloadSource, OssDownloadExecutor, OssStrategy (56 tests) - Drivelution: CommandRunner, ServiceCollectionExtensions, LinuxDriverValidator, WindowsDriverValidator, LinuxGeneralDrivelution, WindowsGeneralDrivelution (43 tests) - Extension: ExtensionServiceFactory (10 tests) - Bowl: ProcessRunner, StorageHelper (17 tests) 126 new tests in total across 14 files. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Closed
Contributor
There was a problem hiding this comment.
Pull request overview
Consolidates all C# test projects under a single top-level tests/ directory (moving BowlTest, DifferentialTest, DrivelutionTest, ExtensionTest out of src/c#/) and adds 126 new unit tests (14 files) to fill coverage gaps in Core, Drivelution, Extension, and Bowl.
Changes:
- Relocate four co-located test projects to
tests/and update central package management accordingly. - Add new test files covering UpgradeHubService, OSS download flow, Drivelution validators/runners, ExtensionServiceFactory, ProcessRunner, StorageHelper, and host-builder/lifecycle/metadata-mapper paths.
- Add
Microsoft.Extensions.DependencyInjection 9.0.0viaDirectory.Packages.propsfor DI-related tests.
Reviewed changes
Copilot reviewed 55 out of 94 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Directory.Packages.props | Adds Microsoft.Extensions.DependencyInjection central version |
| tests/ExtensionTest/Usings.cs | Global usings for relocated Extension test project |
| tests/ExtensionTest/Core/ExtensionServiceFactoryTests.cs | New tests for ExtensionServiceFactory |
| tests/ExtensionTest/Core/ExtensionHostBuilderTests.cs | New tests for builder fluent API and DI registration |
| tests/ExtensionTest/Core/DefaultExtensionMetadataMapperTests.cs | New mapper tests covering DTO→metadata edge cases |
| tests/ExtensionTest/Core/DefaultExtensionLifecycleHooksTests.cs | New default-hooks no-op behavior tests |
| tests/ExtensionTest/Compatibility/RuntimePlatformServicesTests.cs | New runtime platform-detection test |
| tests/ExtensionTest/Compatibility/PlatformMatcherTests.cs | New tests for platform-matching logic |
| tests/DrivelutionTest/Execution/CommandRunnerTests.cs | New tests for CommandRunner (use cmd directly — non-Windows-portable) |
| tests/DrivelutionTest/Windows/Implementation/WindowsGeneralDrivelutionTests.cs | New Windows updater tests; one placeholder for *.inf pattern |
| tests/DrivelutionTest/Linux/Implementation/LinuxGeneralDrivelutionTests.cs | New Linux updater tests; one placeholder for *.ko pattern |
| tests/DrivelutionTest/DrivelutionTest.csproj | Adds DI package reference for new tests |
| tests/BowlTest/Strategies/ProcessRunnerTests.cs | New tests; Windows-only cmd invocations |
| tests/BowlTest/Strategies/StrategyFactoryTests.cs | New tests using OS-conditional bodies (silently pass off-platform) |
| tests/BowlTest/Strategies/WindowsBowlStrategyTests.cs | New Windows strategy tests, not gated by platform |
| src/c#/{Bowl,Differential,Drivelution,Extension}Test/** (deleted) | Removed in favour of relocated tests/... projects |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+119
to
+128
| [Fact] | ||
| public void GetDefaultSearchPattern_IsInfFile() | ||
| { | ||
| var updater = new WindowsGeneralDrivelution( | ||
| _validatorMock.Object, _backupMock.Object, | ||
| _commandRunnerMock.Object); | ||
|
|
||
| // The default search pattern for Windows drivers is *.inf | ||
| Assert.NotNull(updater); | ||
| } |
Comment on lines
+119
to
+128
| [Fact] | ||
| public void GetDefaultSearchPattern_IsKoFile() | ||
| { | ||
| var updater = new LinuxGeneralDrivelution( | ||
| _validatorMock.Object, _backupMock.Object, | ||
| _commandRunnerMock.Object); | ||
|
|
||
| // The default search pattern for Linux drivers is *.ko | ||
| Assert.NotNull(updater); | ||
| } |
Comment on lines
+36
to
+137
| public async Task RunAsync_SuccessfulCommand_ReturnsSuccessResult() | ||
| { | ||
| var runner = new CommandRunner(); | ||
|
|
||
| // Use a simple built-in command that always succeeds | ||
| var result = await runner.RunAsync("cmd", new[] { "/c", "exit 0" }); | ||
|
|
||
| Assert.NotNull(result); | ||
| Assert.Equal(0, result.ExitCode); | ||
| Assert.True(result.Success); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task RunAsync_FailingCommand_ReturnsNonZeroExitCode() | ||
| { | ||
| var runner = new CommandRunner(); | ||
|
|
||
| var result = await runner.RunAsync("cmd", new[] { "/c", "exit 42" }); | ||
|
|
||
| Assert.NotNull(result); | ||
| Assert.Equal(42, result.ExitCode); | ||
| Assert.False(result.Success); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task RunAsync_CapturesStandardOutput() | ||
| { | ||
| var runner = new CommandRunner(); | ||
|
|
||
| var result = await runner.RunAsync("cmd", new[] { "/c", "echo HelloWorld" }); | ||
|
|
||
| Assert.Contains("HelloWorld", result.StandardOutput); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task RunAsync_WithEmptyArguments_DoesNotThrow() | ||
| { | ||
| var runner = new CommandRunner(); | ||
|
|
||
| var result = await runner.RunAsync("cmd", new[] { "/c", "exit 0" }); | ||
|
|
||
| Assert.True(result.Success); | ||
| } | ||
|
|
||
| #endregion | ||
|
|
||
| #region RunOrThrowAsync | ||
|
|
||
| [Fact] | ||
| public async Task RunOrThrowAsync_SuccessfulCommand_ReturnsResult() | ||
| { | ||
| var runner = new CommandRunner(); | ||
|
|
||
| var result = await runner.RunOrThrowAsync("cmd", new[] { "/c", "exit 0" }); | ||
|
|
||
| Assert.True(result.Success); | ||
| Assert.Equal(0, result.ExitCode); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task RunOrThrowAsync_FailingCommand_ThrowsInvalidOperationException() | ||
| { | ||
| var runner = new CommandRunner(); | ||
|
|
||
| var ex = await Assert.ThrowsAsync<InvalidOperationException>( | ||
| () => runner.RunOrThrowAsync("cmd", new[] { "/c", "exit 1" })); | ||
|
|
||
| Assert.Contains("failed with exit code 1", ex.Message); | ||
| } | ||
|
|
||
| #endregion | ||
|
|
||
| #region RunAsync — cancellation | ||
|
|
||
| [Fact] | ||
| public async Task RunAsync_WithCancellation_PreemptsExecution() | ||
| { | ||
| var runner = new CommandRunner(); | ||
| using var cts = new CancellationTokenSource(); | ||
|
|
||
| // Cancel after a short delay so the process has time to start | ||
| cts.CancelAfter(50); | ||
|
|
||
| await Assert.ThrowsAnyAsync<OperationCanceledException>( | ||
| () => runner.RunAsync("cmd", new[] { "/c", "ping -n 10 127.0.0.1 > nul" }, cts.Token)); | ||
| } | ||
|
|
||
| #endregion | ||
|
|
||
| #region RunAsync — returns structured result | ||
|
|
||
| [Fact] | ||
| public async Task RunAsync_ResultProperties_ArePopulated() | ||
| { | ||
| var runner = new CommandRunner(); | ||
|
|
||
| var result = await runner.RunAsync("cmd", new[] { "/c", "echo test-output" }); | ||
|
|
||
| Assert.NotNull(result.StandardOutput); | ||
| Assert.NotNull(result.StandardError); | ||
| Assert.True(result.ExitCode >= 0); | ||
| } |
Comment on lines
+15
to
+175
| public async Task RunAsync_SuccessfulCommand_ReturnsExitCodeZero() | ||
| { | ||
| var psi = new ProcessStartInfo | ||
| { | ||
| FileName = "cmd", | ||
| Arguments = "/c exit 0", | ||
| RedirectStandardOutput = true, | ||
| RedirectStandardError = true, | ||
| UseShellExecute = false, | ||
| CreateNoWindow = true | ||
| }; | ||
|
|
||
| var result = await ProcessRunner.RunAsync(psi, timeoutMs: 10000); | ||
|
|
||
| Assert.Equal(0, result.ExitCode); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task RunAsync_NonZeroExitCode_ReturnsCorrectExitCode() | ||
| { | ||
| var psi = new ProcessStartInfo | ||
| { | ||
| FileName = "cmd", | ||
| Arguments = "/c exit 7", | ||
| RedirectStandardOutput = true, | ||
| RedirectStandardError = true, | ||
| UseShellExecute = false, | ||
| CreateNoWindow = true | ||
| }; | ||
|
|
||
| var result = await ProcessRunner.RunAsync(psi, timeoutMs: 10000); | ||
|
|
||
| Assert.Equal(7, result.ExitCode); | ||
| } | ||
|
|
||
| #endregion | ||
|
|
||
| #region RunAsync — output capture | ||
|
|
||
| [Fact] | ||
| public async Task RunAsync_CapturesStandardOutput() | ||
| { | ||
| var psi = new ProcessStartInfo | ||
| { | ||
| FileName = "cmd", | ||
| Arguments = "/c echo ProcessRunnerTest", | ||
| RedirectStandardOutput = true, | ||
| RedirectStandardError = true, | ||
| UseShellExecute = false, | ||
| CreateNoWindow = true | ||
| }; | ||
|
|
||
| var result = await ProcessRunner.RunAsync(psi, timeoutMs: 10000); | ||
|
|
||
| Assert.Contains(result.OutputLines, line => line.Contains("ProcessRunnerTest")); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task RunAsync_OutputLines_IsNotNull() | ||
| { | ||
| var psi = new ProcessStartInfo | ||
| { | ||
| FileName = "cmd", | ||
| Arguments = "/c echo hello", | ||
| RedirectStandardOutput = true, | ||
| RedirectStandardError = true, | ||
| UseShellExecute = false, | ||
| CreateNoWindow = true | ||
| }; | ||
|
|
||
| var result = await ProcessRunner.RunAsync(psi, timeoutMs: 10000); | ||
|
|
||
| Assert.NotNull(result.OutputLines); | ||
| } | ||
|
|
||
| #endregion | ||
|
|
||
| #region RunAsync — failed process start | ||
|
|
||
| [Fact] | ||
| public async Task RunAsync_NonExistentCommand_ThrowsException() | ||
| { | ||
| var psi = new ProcessStartInfo | ||
| { | ||
| FileName = "nonexistent_command_xyz_123.exe", | ||
| RedirectStandardOutput = true, | ||
| RedirectStandardError = true, | ||
| UseShellExecute = false, | ||
| CreateNoWindow = true | ||
| }; | ||
|
|
||
| await Assert.ThrowsAnyAsync<Exception>( | ||
| () => ProcessRunner.RunAsync(psi, timeoutMs: 5000)); | ||
| } | ||
|
|
||
| #endregion | ||
|
|
||
| #region RunAsync — timeout | ||
|
|
||
| [Fact] | ||
| public async Task RunAsync_Timeout_ThrowsTimeoutException() | ||
| { | ||
| var psi = new ProcessStartInfo | ||
| { | ||
| FileName = "cmd", | ||
| Arguments = "/c ping -n 30 127.0.0.1 > nul", | ||
| RedirectStandardOutput = true, | ||
| RedirectStandardError = true, | ||
| UseShellExecute = false, | ||
| CreateNoWindow = true | ||
| }; | ||
|
|
||
| await Assert.ThrowsAsync<TimeoutException>( | ||
| () => ProcessRunner.RunAsync(psi, timeoutMs: 500)); | ||
| } | ||
|
|
||
| #endregion | ||
|
|
||
| #region RunAsync — cancellation | ||
|
|
||
| [Fact] | ||
| public async Task RunAsync_Cancellation_ThrowsOperationCanceledException() | ||
| { | ||
| var psi = new ProcessStartInfo | ||
| { | ||
| FileName = "cmd", | ||
| Arguments = "/c ping -n 30 127.0.0.1 > nul", | ||
| RedirectStandardOutput = true, | ||
| RedirectStandardError = true, | ||
| UseShellExecute = false, | ||
| CreateNoWindow = true | ||
| }; | ||
|
|
||
| using var cts = new CancellationTokenSource(300); | ||
|
|
||
| await Assert.ThrowsAnyAsync<OperationCanceledException>( | ||
| () => ProcessRunner.RunAsync(psi, timeoutMs: 30000, cts.Token)); | ||
| } | ||
|
|
||
| #endregion | ||
|
|
||
| #region RunAsync — structured result | ||
|
|
||
| [Fact] | ||
| public async Task RunAsync_ResultHasOutputLines() | ||
| { | ||
| var psi = new ProcessStartInfo | ||
| { | ||
| FileName = "cmd", | ||
| Arguments = "/c echo line1 && echo line2", | ||
| RedirectStandardOutput = true, | ||
| RedirectStandardError = true, | ||
| UseShellExecute = false, | ||
| CreateNoWindow = true | ||
| }; | ||
|
|
||
| var result = await ProcessRunner.RunAsync(psi, timeoutMs: 10000); | ||
|
|
||
| Assert.NotEmpty(result.OutputLines); | ||
| } | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
1. Consolidate test project locations
Moved all co-located test projects from
src/c#/BowlTest,src/c#/DifferentialTest,src/c#/DrivelutionTest,src/c#/ExtensionTesttotests/, unifying all test projects under a single directory.2. Fill unit test coverage gaps
Added 126 new tests across 14 files for previously uncovered components:
3. Test Results (all passing)
4. Dependency added
Microsoft.Extensions.DependencyInjection9.0.0 (viaDirectory.Packages.props) for ServiceCollectionExtensions tests🤖 Generated with Claude Code