Speed up Engine.Tests by removing ProcessorCount parallelism cap

[thomhurst]

Summary

• Remove [assembly: ParallelLimiter<ProcessorCountParallelLimit>] from TUnit.Engine.Tests/GlobalSettings.cs
• Add missing [Test] attribute to TimeoutTests1.Test() which was silently not running

Why

TUnit.Engine.Tests has ~119 test methods across 58 classes, each of which spawns a subprocess via CliWrap and waits for it to complete. These tests are I/O-bound — the outer process does no meaningful CPU work.

ProcessorCountParallelLimit is designed for CPU-bound tests. Applied here it forces serial batching: on a 4-core CI machine, 119 tests run in ~30 sequential rounds. Removing the cap lets TUnit schedule all subprocess tests concurrently, so total runtime approaches the duration of the slowest single subprocess rather than ceil(N / cores) × avg_time.

The three [NotInParallel] tests (JUnitReporterTests, GitHubReporterTests, FirstEventReceiversRegressionTest) are unaffected — they still serialize via their own attribute.

[claude]

Code Review

GlobalSettings.cs — Remove ProcessorCountParallelLimit

The reasoning here is sound. ProcessorCountParallelLimit is explicitly designed for CPU-bound work (it batches tests to match the number of logical processors). These tests are pure I/O — each spawns a subprocess and waits — so the limiter was causing unnecessary serialization. Removing it is the correct fix.

One concern worth considering: going from ~4 concurrent tests to 119 fully unconstrained concurrent subprocesses is a large jump. Each subprocess starts a full TUnit test runner, which means 119 concurrent processes loading assemblies, holding file handles, and consuming memory simultaneously. On memory-constrained CI agents this could cause OOM conditions or excessive swapping that ends up slower than the original.

A middle ground worth considering: set a higher fixed limit (e.g., 32–64) rather than fully unbounded, or measure peak memory usage per subprocess to validate the assumption that full concurrency is safe. Something like:

That said, if CI numbers show this working reliably, fully unbounded is also fine — subprocess tests typically have low CPU/memory footprints.

---

TimeoutTests1.cs — Add missing [Test]

Good catch. This is a real bug — a public async Task method named Test() that was silently not running because it lacked the attribute.

Worth checking: Given that TUnit doesn't auto-discover tests without [Test], it's possible other test classes in TUnit.Engine.Tests have the same silent omission. A quick grep for public async Task without a preceding [Test] across the project would be worth running to rule out other cases.

---

Overall

Clean, well-reasoned PR with a clear description explaining the why. The TimeoutTests1 fix is clearly correct. The parallelism change is architecturally sound — just worth validating empirically that full concurrency doesn't introduce resource pressure on constrained CI environments.

[claude]

Code Review

GlobalSettings.cs — Remove ProcessorCountParallelLimit

The reasoning here is sound. ProcessorCountParallelLimit is explicitly designed for CPU-bound work (it batches tests to match the number of logical processors). These tests are pure I/O — each spawns a subprocess and waits — so the limiter was causing unnecessary serialization. Removing it is the correct fix.

One concern worth considering: going from ~4 concurrent tests to 119 fully unconstrained concurrent subprocesses is a large jump. Each subprocess starts a full TUnit test runner, which means 119 concurrent processes loading assemblies, holding file handles, and consuming memory simultaneously. On memory-constrained CI agents this could cause OOM conditions or excessive swapping that ends up slower than the original.

A middle ground worth considering: set a higher fixed limit (e.g., 32–64) rather than fully unbounded, or measure peak memory usage per subprocess to validate the assumption that full concurrency is safe. Something like:

// A high but bounded limit avoids the serialization problem
// without risking resource exhaustion
[assembly: ParallelLimiter<FixedParallelLimit>]  // with an appropriate value

That said, if CI numbers show this working reliably, fully unbounded is also fine — subprocess tests typically have low CPU/memory footprints.

---

TimeoutTests1.cs — Add missing [Test]

Good catch. This is a real bug — a public async Task method named Test() was silently not running because it lacked the attribute.

Worth checking: Given that TUnit doesn't auto-discover tests without [Test], it's possible other test classes in TUnit.Engine.Tests have the same silent omission. A quick grep for public async Task without a preceding [Test] across the project would be worth running to rule out other cases.

---

Overall

Clean, well-reasoned PR with a clear description explaining the why. The TimeoutTests1 fix is clearly correct. The parallelism change is architecturally sound — just worth validating empirically that full concurrency doesn't introduce resource pressure on constrained CI environments.

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

_{TIP This summary will be updated as you push new changes. Give us feedback}