Draft
Conversation
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Member
|
Oh, thanks for this. It's certainly more concrete. I need to let @lillo42 finish his generated tests, because it's hard to hit a moving target, but we could certainly think about this after that. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Production changes: - Materialize static pipeline caches from IOrderedEnumerable to IReadOnlyList to fix thread-safety bug with concurrent LINQ enumeration - Fix missing post-attribute caching in BuildPipeline/BuildAsyncPipeline - Remove ClearPipelineCache() methods (no longer needed with materialized caches) Test migration (Core.Tests): - Replace xUnit with TUnit test framework - Convert all test attributes ([Fact]->[Test], [Theory]->[Test]+[Arguments], etc.) - Convert xUnit assertions to TUnit async assertions - Convert void test methods to async Task - Replace [Collection] with [NotInParallel] - Replace IClassFixture/IAsyncLifetime with TUnit equivalents - Rename TestState to SpecificationState to avoid TUnit.Core.TestState conflict - Remove all ClearPipelineCache() calls across all test projects - Enable ImplicitUsings in tests/Directory.Build.props - Update test generator Liquid templates
…ailures Upgrade TUnit from 0.67.10 to 1.33.0 and fix all test failures to achieve 650 tests passing with full parallel execution (0 failures, 7 intentional skips). Production code changes: - PipelineBuilder: add inbox-aware cache key for pre-attributes to prevent cross-contamination between tests with/without global inbox config - RequestHandler: add self-reference guard in SetSuccessor (parity with async) Test fixes for parallel safety: - Clear Activity.Current in test method bodies to prevent TUnit ambient activity from contaminating observability tests (Before hooks and constructors run in different async scopes, so clearing there has no effect) - Convert static fields to instance fields in pipeline test classes - Change Dictionary.Add() to indexer assignment in 7 test doubles to prevent duplicate key exceptions under concurrent invocation - Add [NotInParallel] groups for tests sharing process-global state: Observability (ActivitySource/Listener), Workflows (static handler state), ContextAware (static test strings), ExceptionPolicy (shared handler flag) TUnit assertion fixes: - byte[] comparison: IsEqualTo -> IsEquivalentTo (reference vs value semantics) - ContentType comparison: add .ToString() for object-to-value-type comparison - Header bag deserialization: compare via .ToString() for numeric type mismatches - HasCount().EqualTo() -> Count().IsEqualTo() for TUnit 1.33.0 API - Swap incorrect Assert.That argument order in context test - Add #pragma for TUnit0031 in synchronization context tests
thomhurst
force-pushed
the
feature/tunit-migration
branch
from
635350a to
c76d53c
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Gates Passed
See analysis details in CodeScene
Quality Gate Profile: Clean Code Collective
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You mentioned last week that you wondered if TUnit was worth switching to - So thought I'd just do a draft branch to show you what Brighter would look like switched over.
Might be useful to see if test speeds improve or anything. I attempted to enable parallelisation by having tests use unique topic names, or queue names, etc. to better support test isolation. (Though I don't know if limited CI constraints will fight against more parallelisation!)
Don't feel obliged to adopt it or anything, just thought it'd be a bit more useful to see it in something more tangible.