[thomhurst]

Fixes #7088

Root cause

TestGroupingService sorted keyed [NotInParallel] tests correctly. It then passed TestPriority.GetHashCode() to ConstraintKeyScheduler as each test's int sort key. The scheduler sorts on that key and also uses it to choose which waiting test starts when a key is released. The compiler-generated record struct hash is roughly (int)Priority * -1521134295 + Order, computed with wrap-around, so it does not preserve ordering:

• With the default Normal priority, any Order above about 894M wraps to a negative number. That includes the default Order (int.MaxValue / 2), so keyed tests without an Order ran before tests with Order = 1.
• [ExecutionPriority] values landed in arbitrary positions.

Both hash sites were affected: the top-level keyed list and the keyed list of each constrained parallel group ([ParallelGroup] + keyed [NotInParallel]).

Fix

The change is confined to TestGroupingService; ConstraintKeyScheduler is untouched. Both keyed lists are now sorted with TestPriority.CompareTo (highest priority first, then ascending Order), with class name as the tie-break. Each test then gets a dense rank as its int key instead of the hash: tests that compare equal on priority share a rank. Because the scheduler's sort is stable on the original index, it keeps this order. The change adds no allocations: one int counter per list, and the sort lambdas are now static.

For the common case (default priority, small Order values), the resulting run order is the same as before, because the hash happened to be monotonic there.

Tests

• New tests/TUnit.TestProject/Bugs/7088/KeyedNotInParallelOrderTests.cs covers keyed tests at top level and inside a [ParallelGroup]. They mix Order = 1/2/3, Order = 1_000_000_000, the default Order, and an [ExecutionPriority(High)] test. Each test checks that the tests that started before it are exactly the expected prefix.
  • Before the fix, all 12 tests failed in both source-gen and reflection mode. Actual start order: Order1000000000, DefaultOrder, HighPriority_Order50, Order1, Order2, Order3.
  • After the fix, all 12 pass in both modes.
• New tests/TUnit.Engine.Tests/Scheduling/KeyedNotInParallelOrderTests.cs runs the above through the engine harness. Reflection passes locally; the AOT cases are CI-only.
• Existing TestProject filters pass in both modes: NotInParallelOrderExecutionTests, NotInParallelMixedTests, *ConstraintKey* and *NotInParallel*. The only failure under *NotInParallel* is the intentional NotInParallelWithDependsOnTests.Step1.
• TUnit.UnitTests passes (526 passed, 3 skipped). The TUnit.Engine.Tests filters Scheduling/* and *NotInParallel* pass, with AOT cases skipped locally.
• There are no generator or public API changes, so snapshot and PublicAPI tests are not affected.

Overlap with open PRs

#7084 and #7087 both edit src/TUnit.Engine/Scheduling/ConstraintKeyScheduler.cs. This PR does not touch that file, so there is no textual conflict. The scheduler still treats Priority as "lower runs first", which now holds by construction.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 5 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 8 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: e8edf365-eb3a-4c64-b6f6-c99161d03d89

📥 Commits

Reviewing files that changed from the base of the PR and between 7bb4dc3 and 9c804d1.

📒 Files selected for processing: 4

• src/TUnit.Engine/Models/GroupedTests.cs
• src/TUnit.Engine/Services/TestGroupingService.cs
• tests/TUnit.Engine.Tests/Scheduling/KeyedNotInParallelOrderTests.cs
• tests/TUnit.TestProject/Bugs/7088/KeyedNotInParallelOrderTests.cs

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-17T14:49:50.489145Z	9c804d1	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge.

Summary

Replaces hash-based sort keys with ordered ranks for keyed [NotInParallel] tests.

• Keyed tests start by execution priority, then ascending order.

_{Reviews (1) · Last reviewed commit: "fix(engine): order keyed NotInParallel t..." · Reviewed by Greptile}

[github-actions]

Review

The fix is correct and well-scoped. I found no blocking issues.

• Root cause: TestPriority.GetHashCode() was being used as an ordering key, and a record-struct hash doesn't preserve order. Replacing it with a dense rank derived from TestPriority.CompareTo fixes both the top-level keyed list and the [ParallelGroup] keyed list.
• Ordering semantics: CompareTo sorts higher Priority first, then ascending Order. That matches the scheduler's "lower rank runs first" assumption.
• Rank assignment: equal priorities share a rank, and the sort sits on top of the scheduler's stable index sort, so the class-name tie-break is preserved.
• Single comparer: both sites now use one CompareKeyed helper, so they can't drift apart again. The lambdas are static, so there are no new allocations.
• Tests: the regression tests cover both paths, including a huge Order, the default Order, and ExecutionPriority. The prefix-check probe is a clean way to assert start order.

Two non-blocking notes:

1. Sort order change: the previous sort made class name the primary key, then priority. It is now priority first, then class name. That is arguably the intended behavior, since the old order let class name override ExecutionPriority, but it is a visible change. The PR description could mention it.
2. Future-proofing: the rank is only meaningful relative to the list it was computed over. A short doc comment on ConstraintKeyScheduler's priority parameter saying "dense rank, lower first" would stop someone from putting a hash back later. The GroupedTests comments already cover most of this.

LGTM.