Fix issue with randomly failing tests to due parallel execution by christianhelle · Pull Request #872 · christianhelle/refitter

christianhelle · 2026-01-15T13:52:29Z

Use assembly location as temp folder
Add Refit.HttpClientFactory package reference to test project
chore(deps): update TUnit dependency to v1.11.28
Add conditional test cases for SwaggerPetstore IObservableResponse
Use assembly location as base for temporary directories in WriteRefitterSettingsFile tests.
Skip clean up if build fails

Summary by CodeRabbit

Tests
- Updated testing framework dependencies to the latest stable version.
- Enhanced test coverage for generated code scenarios.
- Improved test infrastructure for better build and path handling.
- Added package dependency validation to generated code tests.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

…terSettingsFile tests.

coderabbitai · 2026-01-15T13:52:39Z

📝 Walkthrough

Walkthrough

These changes update test infrastructure by bumping the TUnit testing framework to version 1.11.28, refactoring the build helper's working path generation to use assembly location, expanding test parameters for non-debug builds, and adding Refit.HttpClientFactory as a package dependency to the embedded project template.

Changes

Cohort / File(s)	Summary
TUnit Framework Upgrade `src/Refitter.SourceGenerator.Tests/Refitter.SourceGenerator.Tests.csproj`, `src/Refitter.Tests/Refitter.Tests.csproj`	Both test project files updated from TUnit 1.9.2 to 1.11.28
Build Path Generation `src/Refitter.Tests/Build/BuildHelper.cs`	Replaced in-process path generation with assembly location-based folder derivation; adds working directory cleanup logic with exception swallowing
Embedded Project Template `src/Refitter.Tests/Build/ProjectFileContents.cs`	Added Refit.HttpClientFactory 8.0.0 PackageReference to Net80App embedded project file template
Test Parameterization `src/Refitter.Tests/SwaggerPetstoreTests.cs`	Added conditional test case arguments (SwaggerPetstoreYamlV3, SwaggerPetstoreJsonV2, SwaggerPetstoreYamlV2) for non-DEBUG builds to two test methods
Test Setup & Assertions `src/Refitter.Tests/WriteRefitterSettingsFileTests.cs`	Updated temporary directory path derivation to use assembly location; enhanced test assertion to deserialize and verify ContractsOutputFolder value

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Migrate unit tests from xUnit to TUnit #830: Both PRs modify test project files and test infrastructure, with this PR building upon framework migration changes through dependency bumps and test parameter expansions.

Suggested labels

dependencies, testing

Poem

🐰 A testing tale so fine and bright,
TUnit bumped to heights of might!
Paths now found where code resides,
Tests expanded far and wide—
Build and verify, our tests unite!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main objective of the PR, which is to fix randomly failing tests caused by parallel execution issues.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

sonarqubecloud · 2026-01-15T13:54:05Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Copilot

Pull request overview

This PR addresses random test failures caused by parallel test execution by changing temporary directory locations and adding missing dependencies. The key improvement is using the assembly location as the base for temporary directories instead of the system temp folder, which helps avoid conflicts between parallel test runs.

Changes:

Modified temporary directory creation to use assembly location instead of system temp path in BuildHelper and one WriteRefitterSettingsFile test
Added Refit.HttpClientFactory package reference to test project files to support generated code compilation
Updated TUnit test framework from v1.9.2 to v1.11.28
Added conditional compilation to limit SwaggerPetstore IObservableResponse test cases to Release builds only
Implemented cleanup skip when builds fail to preserve artifacts for debugging

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/Refitter.Tests/WriteRefitterSettingsFileTests.cs	Updated one test to use assembly location for temp directory; removed trailing whitespace
src/Refitter.Tests/SwaggerPetstoreTests.cs	Added conditional compilation directives to run additional test cases only in Release mode
src/Refitter.Tests/Refitter.Tests.csproj	Updated TUnit dependency from v1.9.2 to v1.11.28
src/Refitter.Tests/Build/ProjectFileContents.cs	Added Refit.HttpClientFactory package reference to test project template
src/Refitter.Tests/Build/BuildHelper.cs	Changed temp directory to use assembly location; added cleanup that only runs on successful builds
src/Refitter.SourceGenerator.Tests/Refitter.SourceGenerator.Tests.csproj	Updated TUnit dependency from v1.9.2 to v1.11.28

Copilot · 2026-01-15T13:55:52Z

+        var folder = Path.GetDirectoryName(GetType().Assembly.Location) ?? Path.GetTempPath();
+        var tempDir = Path.Combine(folder, Guid.NewGuid().ToString());


Only one test method in this file has been updated to use the assembly location pattern, but there are five other test methods (starting at lines 160, 199, 241, 288, and 337) that still use Path.GetTempPath() directly. For consistency and to fully address parallel execution issues, all tests that create temporary directories in this file should use the same pattern: Path.GetDirectoryName(GetType().Assembly.Location) ?? Path.GetTempPath().

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/Refitter.Tests/Build/ProjectFileContents.cs`:
- Line 12: Update the embedded project template in ProjectFileContents (the
string containing the PackageReference) so the Refit.HttpClientFactory
PackageReference line uses Version="9.0.2" instead of "8.0.0"; locate the
PackageReference entry for "Refit.HttpClientFactory" in the ProjectFileContents
constant and change the Version attribute to 9.0.2 to match the rest of the test
projects.

🧹 Nitpick comments (1)

src/Refitter.Tests/Build/BuildHelper.cs (1)
46-53: Cleanup behavior is correct but consider logging cleanup failures.

The cleanup logic correctly:

Only runs on successful builds (after the exception throw on failure)

Swallows exceptions to prevent test failures due to cleanup issues

This aligns with the PR objective to "skip cleanup if build fails" for easier debugging. However, silently swallowing cleanup errors could hide issues in CI environments where disk space accumulates.
💡 Optional: Add trace logging for cleanup failures
         try
         {
             Directory.Delete(path, true);
         }
         catch
         {
-            // Ignore cleanup errors
+            // Ignore cleanup errors - log for diagnostics if needed
+            // System.Diagnostics.Trace.WriteLine($"Failed to cleanup test directory: {path}");
         }

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ba0e5ad and ea1ee5a.

📒 Files selected for processing (6)

src/Refitter.SourceGenerator.Tests/Refitter.SourceGenerator.Tests.csproj
src/Refitter.Tests/Build/BuildHelper.cs
src/Refitter.Tests/Build/ProjectFileContents.cs
src/Refitter.Tests/Refitter.Tests.csproj
src/Refitter.Tests/SwaggerPetstoreTests.cs
src/Refitter.Tests/WriteRefitterSettingsFileTests.cs

🧰 Additional context used

📓 Path-based instructions (1)

**/*.cs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.cs: Always run dotnet format src/Refitter.sln before committing or CI (.github/workflows/build.yml) will fail
Follow existing C# coding conventions defined in .editorconfig
Use PascalCase for public members in C# code
Use camelCase for parameters and local variables in C# code
Include XML documentation for public APIs in C# code
Use meaningful variable and method names in C# code
Keep methods focused with single responsibility in C# code
Code must be properly formatted according to .editorconfig rules for CI to pass

Files:

src/Refitter.Tests/WriteRefitterSettingsFileTests.cs
src/Refitter.Tests/Build/BuildHelper.cs
src/Refitter.Tests/SwaggerPetstoreTests.cs
src/Refitter.Tests/Build/ProjectFileContents.cs

🧠 Learnings (10)

📓 Common learnings

Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-13T07:38:42.714Z
Learning: Applies to src/Refitter.Core/**/*.cs : For Core library changes: Run full test suite and verify generated code builds successfully

Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-13T07:38:42.714Z
Learning: Applies to src/Refitter/**/*.cs : For CLI changes: Test with a real OpenAPI specification to ensure generation works correctly

Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-13T07:38:42.714Z
Learning: Applies to src/Refitter.Tests/Examples/**/*.cs : New CLI options must add unit tests in `src/Refitter.Tests/Examples/` following the established pattern

Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-13T07:38:42.714Z
Learning: Applies to **/*.Tests.cs : All new code must include unit tests following the pattern used in `Refitter.Tests.Examples` namespace

Learnt from: CR
Repo: christianhelle/refitter PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-13T07:38:42.714Z
Learning: When making framework-specific changes, always test both .NET 8.0 and .NET 9.0 targets