test: fix flaky ObjectInitializer continuation-thread test - #6932
Conversation
The initializing thread is a LongRunning dedicated thread that exits once initialization finishes. The runtime can then reuse its ManagedThreadId for a thread-pool thread that runs a correctly queued waiter continuation, which made the IsNotEqualTo(threadId) check fail spuriously (seen on macOS CI). Compare Thread object references instead, which stay distinct. Co-Authored-By: Claude <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughThe object initializer test now tracks the initializing thread and waiter continuation threads as ChangesObject initializer thread test
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~7 minutes Change: Other Suggested reviewers: Merge Risk: ⚪ Minimal · up to This test-only change avoids false failures from reused thread IDs while preserving the regression scenario. No actionable merge risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. A rabbit checks each thread with care, Comment |
|
Review: LGTM. The diagnosis is sound. Minor, non-blocking: holding the No issues found. |
|
Updated [TUnit](https://github.com/thomhurst/TUnit) from 1.71.0 to 1.72.4. <details> <summary>Release notes</summary> _Sourced from [TUnit's releases](https://github.com/thomhurst/TUnit/releases)._ ## 1.72.4 <!-- Release notes generated using configuration in .github/release.yml at v1.72.4 --> ## What's Changed ### Other Changes * fix(source-gen): stop parameter resolver keeping every non-public test-class method (IL2111) by @thomhurst in thomhurst/TUnit#6937 ### Dependencies * chore(deps): update tunit to 1.72.0 by @thomhurst in thomhurst/TUnit#6934 **Full Changelog**: thomhurst/TUnit@v1.72.0...v1.72.4 ## 1.72.0 <!-- Release notes generated using configuration in .github/release.yml at v1.72.0 --> ## What's Changed ### Other Changes * perf(source-gen): resolve parameter reflection info through a shared runtime helper by @thomhurst in thomhurst/TUnit#6923 * perf(analyzers): trim remaining analyzer hot-path symbol lookups and binds by @thomhurst in thomhurst/TUnit#6928 * perf(mocks): move shared MockCall wrapper plumbing into runtime base classes by @thomhurst in thomhurst/TUnit#6929 * perf(source-gen): close incremental caching gaps in static property and property injection generators by @thomhurst in thomhurst/TUnit#6925 * perf(source-gen): stop InfrastructureGenerator pinning an old Compilation by @thomhurst in thomhurst/TUnit#6926 * perf(assertions-analyzers): cache assertion symbols and cut per-call work by @thomhurst in thomhurst/TUnit#6927 * perf(source-gen): emit hooks per class with direct, non-async bodies by @thomhurst in thomhurst/TUnit#6924 * test: fix flaky ObjectInitializer continuation-thread test by @thomhurst in thomhurst/TUnit#6932 * fix: CI flakes from leaked hook contexts, ActivityCollector race and Repro5700 rendezvous by @thomhurst in thomhurst/TUnit#6933 * fix(aspnetcore): honor WebApplicationFactoryClientOptions in CreateClient by @thomhurst in thomhurst/TUnit#6931 ### Dependencies * chore(deps): update tunit to 1.71.0 by @thomhurst in thomhurst/TUnit#6920 **Full Changelog**: thomhurst/TUnit@v1.71.0...v1.72.0 Commits viewable in [compare view](thomhurst/TUnit@v1.71.0...v1.72.4). </details> Updated [TUnit.AspNetCore](https://github.com/thomhurst/TUnit) from 1.71.0 to 1.72.4. <details> <summary>Release notes</summary> _Sourced from [TUnit.AspNetCore's releases](https://github.com/thomhurst/TUnit/releases)._ ## 1.72.4 <!-- Release notes generated using configuration in .github/release.yml at v1.72.4 --> ## What's Changed ### Other Changes * fix(source-gen): stop parameter resolver keeping every non-public test-class method (IL2111) by @thomhurst in thomhurst/TUnit#6937 ### Dependencies * chore(deps): update tunit to 1.72.0 by @thomhurst in thomhurst/TUnit#6934 **Full Changelog**: thomhurst/TUnit@v1.72.0...v1.72.4 ## 1.72.0 <!-- Release notes generated using configuration in .github/release.yml at v1.72.0 --> ## What's Changed ### Other Changes * perf(source-gen): resolve parameter reflection info through a shared runtime helper by @thomhurst in thomhurst/TUnit#6923 * perf(analyzers): trim remaining analyzer hot-path symbol lookups and binds by @thomhurst in thomhurst/TUnit#6928 * perf(mocks): move shared MockCall wrapper plumbing into runtime base classes by @thomhurst in thomhurst/TUnit#6929 * perf(source-gen): close incremental caching gaps in static property and property injection generators by @thomhurst in thomhurst/TUnit#6925 * perf(source-gen): stop InfrastructureGenerator pinning an old Compilation by @thomhurst in thomhurst/TUnit#6926 * perf(assertions-analyzers): cache assertion symbols and cut per-call work by @thomhurst in thomhurst/TUnit#6927 * perf(source-gen): emit hooks per class with direct, non-async bodies by @thomhurst in thomhurst/TUnit#6924 * test: fix flaky ObjectInitializer continuation-thread test by @thomhurst in thomhurst/TUnit#6932 * fix: CI flakes from leaked hook contexts, ActivityCollector race and Repro5700 rendezvous by @thomhurst in thomhurst/TUnit#6933 * fix(aspnetcore): honor WebApplicationFactoryClientOptions in CreateClient by @thomhurst in thomhurst/TUnit#6931 ### Dependencies * chore(deps): update tunit to 1.71.0 by @thomhurst in thomhurst/TUnit#6920 **Full Changelog**: thomhurst/TUnit@v1.71.0...v1.72.0 Commits viewable in [compare view](thomhurst/TUnit@v1.71.0...v1.72.4). </details> Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Summary
ObjectInitializerTests.Waiting_Continuations_Do_Not_Run_On_The_Initializing_Threadfailed on macOS CI (run 36556399225) withExpected to not be equal to 17.Cause: the test runs initialization on a
TaskCreationOptions.LongRunningdedicated thread, which exits as soon as initialization finishes. It then comparedEnvironment.CurrentManagedThreadIdvalues. The runtime can give a dead thread's managed ID to a new thread-pool thread, and that thread can run a correctly queued waiter continuation. The IDs then match even though the product code behaved correctly.Fix: capture
Thread.CurrentThreadand assertIsNotSameReferenceAs. A new thread-pool thread is always a differentThreadobject, even when it gets the same ID. The assertion still checks the same behaviour.Test plan
ObjectInitializerTestsclass passes: 11/11.🤖 Generated with Claude Code
Summary by CodeRabbit