fix(fscheck): register default CancellationToken arbitrary that surfaces TestContext token

[JohnVerheij]

Description

FsCheckPropertyTestExecutor.CreateConfig now registers a default Arbitrary<CancellationToken> that returns the timeout-backed token from TestContext.Current. Without this, FsCheck's built-in reflection-based arbitrary invokes the CancellationToken(bool canceled) constructor with a randomly generated bool, producing a pre-cancelled token on roughly half of invocations and silently defeating [Timeout] cooperation.

This applies the data-source-generator-attribute approach @thomhurst sketched in #2125 to address the lifecycle constraint stated in #3803 ("test data is constructed before tests so doesn't have a test context available yet"). The fix registers an attribute-shaped arbitrary that resolves the token at property-invocation time rather than at discovery.

Three artifacts:

• New internal static CancellationTokenArbitrary with a static factory. Annotated via [DynamicDependency] on CreateConfig for AOT/trimming preservation.
• CreateConfig registers user-supplied arbitraries first, then appends the default. FsCheck's WithArbitrary resolves earlier entries with higher priority, so user overrides win on conflict.
• Seven regression tests in TUnit.Example.FsCheck.TestProject/CancellationTokenBehaviourTests.cs.

Alternatives considered:

Approach	Tradeoff
TestContext CT (this PR)	Full [Timeout] cooperation
CancellationToken.None always	Stops the pre-cancellation surprise but no [Timeout]
Analyzer exemption for FsCheck methods	Hides the symptom, doesn't fix the root cause

Happy to reshape if a different approach is preferred.

Out of scope: No analyzer changes (TUnit0015's advice "add a CancellationToken" becomes correct for FsCheck properties after this fix). No public API changes (CancellationTokenArbitrary is internal static).

Breaking changes: None. Users who supply their own Arbitrary<CancellationToken> via FsCheckPropertyAttribute.Arbitrary keep their override because user registrations appear first in the type list. Verified by the regression test UserSuppliedArbitraryOverridesDefault.

Related Issue

Fixes #5757

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update
• Performance improvement
• Refactoring (no functional changes)

Checklist

Required

• I have read the Contributing Guidelines
• If this is a new feature, I started a discussion first and received agreement
• My code follows the project's code style (modern C# syntax, proper naming conventions)
• I have written tests that prove my fix is effective or my feature works

TUnit-Specific Requirements

• Dual-Mode Implementation: If this change affects test discovery/execution, I have implemented it in BOTH:
  • Source Generator path (TUnit.Core.SourceGenerator)
  • Reflection path (TUnit.Engine)
• Snapshot Tests: If I changed source generator output or public APIs:
  • I ran TUnit.Core.SourceGenerator.Tests and/or TUnit.PublicAPI tests
  • I reviewed the .received.txt files and accepted them as .verified.txt
  • I committed the updated .verified.txt files
• Performance: If this change affects hot paths (test discovery, execution, assertions):
  • I minimized allocations and avoided LINQ in hot paths
  • I cached reflection results where appropriate
• AOT Compatibility: If this change uses reflection:
  • I added appropriate [DynamicallyAccessedMembers] annotations
  • I verified the change works with dotnet publish -p:PublishAot=true

Testing

• All existing tests pass (dotnet test)
• I have added tests that cover my changes
• I have tested both source-generated and reflection modes (if applicable)

Additional Notes

The change lives entirely in FsCheckPropertyTestExecutor.CreateConfig, which is shared by both source-generator and reflection engine modes; Dual-Mode and Snapshot-Tests checkboxes are not applicable. AOT preservation uses [DynamicDependency(DynamicallyAccessedMemberTypes.PublicMethods, typeof(CancellationTokenArbitrary))] on CreateConfig; this is the appropriate annotation for the typeof(...) flow into FsCheck's WithArbitrary(IEnumerable<Type>). The repo's IsAotCompatible=true (in Library.props for net8+) runs the AOT analyzers during normal build; my build produced no new IL warnings on net8.0/net9.0/net10.0. Tested locally on net8.0/net9.0/net10.0 in source-generator mode (49/49 passes across 7 stability iterations). Local dotnet publish -p:PublishAot=true hits NETSDK1207 because TUnit's library set multi-targets to netstandard2.0, which doesn't support AOT; CI's AOT lane will exercise that path.

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 57 complexity

Metric	Results
Complexity	57

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[claude]

Code Review — fix(fscheck): register default CancellationToken arbitrary

Overall this is a well-reasoned, well-scoped fix. The root cause is correctly identified, the solution is minimally invasive, and the regression-test suite is thorough. I have a few observations, mostly minor.

---

What works well

Root cause analysis and approach
The description is accurate: FsCheck's built-in reflection-based arbitrary does call CancellationToken(bool) with a random bool, producing a pre-cancelled token on ~50% of invocations. Registering a default Arbitrary<CancellationToken> that surfaces TestContext.Current is the right layer to fix this.

Override precedence
User-supplied arbitraries are prepended to the list, so FsCheck.WithArbitrary resolves them first. This is documented correctly in the inline comment and backed by a regression test (UserSuppliedArbitraryOverridesDefault). The simplification from the conditional if (Arbitrary != null && Length > 0) to the unconditional .Append is cleaner.

AOT annotation
[DynamicDependency(DynamicallyAccessedMemberTypes.PublicMethods, typeof(CancellationTokenArbitrary))] on CreateConfig is the correct mechanism: it tells the trimmer to preserve the methods FsCheck discovers via reflection without broadening any other scope.

Test suite
Seven tests covering: no-CT parameters (unaffected), user override, user override for an unrelated type, empty arbitrary array, token identity, non-cancellation, and cooperative cancellation via [Timeout]. Good coverage.

---

Issues / suggestions

1. Token capture timing — subtle but correct; worth a comment

CancellationTokenArbitrary.CancellationToken() is a static factory called by FsCheck via reflection when it first needs a CancellationToken generator. At that point TestContext.Current is in scope because CreateConfig() → RunPropertyCheck() runs entirely within ExecuteTest(). Gen.Constant(token) then bakes the token for all iterations of the property — which is correct since we want the same timeout-backed token throughout.

This is non-obvious. A one-line comment inside the method body would help future maintainers:

// Called by FsCheck via reflection during property execution; TestContext.Current is in scope.
public static Arbitrary<CancellationToken> CancellationToken()

2. Excessive XML documentation on an internal class

CancellationTokenArbitrary.cs is ~32 lines of code but contains ~20 lines of XML <summary> and <remarks> on an internal static class. The project's guidelines say to avoid multi-paragraph docstrings and only comment when the why is non-obvious to a reader. The detailed prose belongs in the PR description (where it already lives), not in the source. Consider stripping the remarks block and keeping only the type-level summary if anything.

3. new Type[0] in test — use [] for consistency

// current
[Test, FsCheckProperty(MaxTest = 1, Arbitrary = new Type[0])]

// consistent with modern C# (LangVersion = preview)
[Test, FsCheckProperty(MaxTest = 1, Arbitrary = [])]

4. CooperativeCancellationViaTimeout timing window is very wide

The test asserts cancellation happened between 200 ms and 2 000 ms with a 500 ms timeout. The lower bound (200 ms) makes sense as slack for startup, but the upper bound is 4× the configured timeout. If CI is very slow the test could pass even when the token is not the timeout token (e.g. the property just happened to run quickly). A tighter window, say <= 1_500, would still be CI-safe and give more signal.

5. XML doc on test methods is noise

Each test method carries a <summary> that essentially restates the test name:

/// <summary>
/// 100 invocations check that the CT is NOT pre-cancelled at entry…
/// </summary>
[Test, FsCheckProperty(MaxTest = 100)]
public bool CancellationTokenIsNotPreCancelled(CancellationToken cancellationToken)

The method name already communicates the intent. The #pragma warning disable TUnit0015 comment (with reason) is useful; the rest could be dropped.

---

Minor nit

The CancellationToken() method name collides with the System.Threading.CancellationToken type name, forcing the fully-qualified fallback System.Threading.CancellationToken.None in the body. This is the FsCheck convention (method name matches the generated type), so it's intentional and FsCheck examples do the same thing — just worth knowing if someone finds it confusing.

---

Summary: The fix is correct and the regression suite is solid. The main actionable items are trimming the excessive internal-class documentation (item 2) and tightening the timing window in the cooperative-cancellation test (item 4). Everything else is cosmetic.

[JohnVerheij]

Thanks for the review. Addressed each point in 4ad9e93:

1. Token capture timing comment: added inside CancellationToken().

2. Excessive XML docs: stripped the <remarks> block on CancellationTokenArbitrary; kept only the brief type-level <summary>.

3. Empty array initializer: switched new Type[0] to [] (collection expression), and applied the same modernisation to the non-empty Arbitrary = [typeof(...)] cases for consistency within the test file.

4. Timing window: tightened from <= 2_000 to <= 1_500 (3x the configured 500ms timeout). Still CI-safe locally (49/49 passes across 7 stability iterations); sharper signal if cancellation propagation regresses.

5. Per-test XML docs: dropped the redundant <summary> blocks (method names self-document the assertion). Kept the class-level summary, the AlwaysNoneTokenArbitrary doc (cross-references the consumer test), and the CooperativeCancellationViaTimeout doc (timing mechanism is non-obvious).

Regarding the method-naming nit: yes, intentional. It matches the FsCheck convention demonstrated by PositiveIntArbitrary.PositiveInt() in the same project.

[JohnVerheij]

CI is hitting a main-branch regression (CollectionAssertionTests.cs:270, also failing #5728). Holding for the upstream fix.

[thomhurst]

Yep my bad. On it 😄

[thomhurst]

Thanks!