fix: Resource leak in the tests#2230
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reduces long-running test-suite memory growth by preventing generated-override assemblies and some runtime services from permanently accumulating in the default load context, and by disabling memory-hungry runtime behaviors in the test host.
Changes:
- Load generated override assemblies into a collectible
AssemblyLoadContextand dispose/unload it after each test. - Disable MCP HTTP hosting during tests (
McpHttpPort = 0) and only start MCP HTTP when the port is non-zero. - Reduce per-test allocations by using
ILoggerServicesubstitutes in several tests and forcing workstation GC settings for the test project.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Spice86.Tests/Video/VgaRenderer256ColorTestsBase.cs | Replace LoggerService allocation with ILoggerService substitute in renderer test setup. |
| tests/Spice86.Tests/Video/RendererTestsBase.cs | Replace LoggerService allocation with ILoggerService substitute in renderer test setup. |
| tests/Spice86.Tests/Spice86Creator.cs | Disable MCP HTTP server in tests by setting McpHttpPort = 0. |
| tests/Spice86.Tests/Spice86.Tests.csproj | Force workstation GC (and disable concurrent GC) to reduce peak RSS during the full test run. |
| tests/Spice86.Tests/GeneratedOverrideCompiler.cs | Cache metadata references and load compiled overrides into a collectible load context. |
| tests/Spice86.Tests/GeneratedCodeMachineTestRunner.cs | Ensure compiled override is disposed/unloaded via using. |
| tests/Spice86.Tests/GeneratedCodeMachineTest.cs | Ensure compiled override is disposed/unloaded via using. |
| tests/Spice86.Tests/CompiledGeneratedOverride.cs | Wrap compiled override supplier + load context and implement IDisposable to unload. |
| tests/Spice86.Tests/CollectibleAssemblyLoadContext.cs | New collectible ALC type to host generated override assemblies. |
| tests/Spice86.Tests/CfgCpu/InstructionsFeederTest.cs | Use ILoggerService substitute instead of substituting LoggerService. |
| tests/Spice86.Tests/CfgCpu/CfgGraphReloadTest.cs | Silence logs in tests by setting AreLogsSilenced = true on LoggerService. |
| src/Spice86/Spice86DependencyInjection.cs | Start MCP HTTP transport only when McpHttpPort != 0. |
| src/Spice86.Shared/Interfaces/ILoggerService.cs | Remove UseStderrForConsoleOutput() from the shared logging interface. |
| src/Spice86.Logging/LoggerService.cs | Rework logger building to avoid retained configuration state and remove stderr-redirection API. |
| src/Spice86.Core/Emulator/Devices/Video/Renderer.cs | Accept ILoggerService (interface) instead of concrete LoggerService. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dee1982 to
459f39f
Compare
459f39f to
8745138
Compare
maximilien-noal
approved these changes
Jun 18, 2026
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.
Fixes leaks in tests: