You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Remove nullable and assertion warnings from unit tests - #7670
Unit-test builds emitted nullable-reference and assertion-style warnings that obscured new diagnostics. Use void-returning async exception assertions, guard optional test metadata before initialization, and use dedicated null/boolean assertions. Replace constant success assertions with completed fixture bodies or explicit no-throw checks, while preserving the empty-source fixture's failure if it executes.
Also replace newly present boolean comparison warnings in PlaywrightEnvironmentTests and obsolete count assertions in SharedObjectDisposeSpanTests.
Validation:
dotnet build tests/TUnit.UnitTests -f net10.0: zero warnings and errors after the changes; baseline reproduced the reported diagnostics.
Separate net10.0 targeted filters passed for ParallelMapTests, PlaywrightVideoTests, PlaywrightTraceContextTests, PropertyDataSourceInjectionTests, DerivedPropertyInjectionTests, CustomPropertyDataSourceTests, TestContextAvailabilityDuringPropertyInjectionTests, EmptyDataSourceTests, DependencyResolutionFailureTests, LogSinkRouterTests, GenericTestGenerationTests, GenericTestClass_Int/String/Bool, EngineCancellationTokenTests, PlaywrightEnvironmentTests, and SharedObjectDisposeSpanTests. EmptyDataSourceTests retains one expected skip.
git diff --check passed. No shipped public API or generator output changes. Engine.Tests was not run, per repository instructions.
Warnings in referenced analyzer/generator projects are outside this change; the baseline log retains them. An initial filter for InheritedPropertyDataSourceTests matched no tests; the actual DerivedPropertyInjectionTests filter passed.
No actionable comments were generated in the recent review.
Walkthrough
Unit-test sources were updated to address reported nullable-reference and assertion-style diagnostics. Changes replace assertion forms, correct async exception assertion delegates, adjust test method forms, and guard specialized data-source creation when test information is null.
Changes: Unit-test diagnostics
Layer / File(s)
Summary
Update test assertions and method forms tests/TUnit.UnitTests/DependencyResolutionFailureTests.cs, tests/TUnit.UnitTests/EmptyDataSourceTests.cs, tests/TUnit.UnitTests/EngineCancellationTokenTests.cs, tests/TUnit.UnitTests/GenericTestGenerationTests.cs, tests/TUnit.UnitTests/LogSinkRouterTests.cs, tests/TUnit.UnitTests/ParallelMapTests.cs, tests/TUnit.UnitTests/Playwright*Tests.cs, tests/TUnit.UnitTests/SharedObjectDisposeSpanTests.cs
Tests now use explicit boolean, null, and count assertions; assert that routing throws nothing; and check async exceptions through async delegates. Dependency test methods return Task.CompletedTask, and the empty-data-source test uses a synchronous method with Assert.Fail.
Specialized creation runs only when TestInformation is present. Failed or skipped specialized creation falls back to Activator.CreateInstance(typeof(T)).
The changes appear to preserve the affected test behavior. No merge-blocking issue was identified.
Pre-merge checks: 5
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title "Remove nullable and assertion warnings from unit tests" matches the main change. The raw summary and objectives show edits across tests/TUnit.UnitTests that remove CS8602, CS8619, CS8604, a…
Linked Issues check
[#7438] Requires removing CS8602, CS8619, CS8604, TUnitAssertions0005, TUnitAssertions0014, and TUnitAssertions0015 from tests/TUnit.UnitTests without suppression, while preserving test behavior. The …
Out of Scope Changes check
The changes stay within the unit-test warning scope of #7438. Two files outside the reported list are also in scope: - PlaywrightEnvironmentTests.cs: five Headless comparisons now use IsTrue or IsFals…
Docstring Coverage
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Full details: Title check
Explanation
The title "Remove nullable and assertion warnings from unit tests" matches the main change. The raw summary and objectives show edits across tests/TUnit.UnitTests that remove CS8602, CS8619, CS8604, and TUnitAssertions diagnostics. The title is specific and concise, and it names the primary intent (warning removal in unit tests) without file lists or vague terms.
Full details: Linked Issues check
Explanation
[#7438] Requires removing CS8602, CS8619, CS8604, TUnitAssertions0005, TUnitAssertions0014, and TUnitAssertions0015 from tests/TUnit.UnitTests without suppression, while preserving test behavior. The whole-PR diff (merge base 6bc28cd to head b2bd83f) changes each listed site:
ParallelMapTests.cs: SelectorException_PropagatesToCaller uses a fluent async Throws<InvalidOperationException>().WithMessage("boom") assertion. The expected exception and message are unchanged.
PlaywrightVideoTests.cs and PlaywrightTraceContextTests.cs: NewContext assertions use async delegates. The expected PlaywrightException and InvalidOperationException outcomes are unchanged.
PropertyDataSourceInjectionTests.cs: TryCreateWithInitializerAsync runs only when TestInformation is non-null. This addresses CS8604.
EmptyDataSourceTests.cs: the SkipIfEmpty test is void and calls Assert.Fail if it executes. The intentional failure is preserved.
DependencyResolutionFailureTests.cs, LogSinkRouterTests.cs: constant success assertions are replaced with Task.CompletedTask or no-throw checks. Dependency-driven outcomes remain defined by the existing attributes.
GenericTestGenerationTests.cs and EngineCancellationTokenTests.cs: boolean and default comparisons use IsTrue, IsFalse, or null assertions.
No #pragma or other suppression appears in the change summary. The author reports a zero-warning build and passing targeted filters. I did not run the build or tests, so that result rests on the author's report and the source diffs.
Full details: Out of Scope Changes check
Explanation
The changes stay within the unit-test warning scope of #7438. Two files outside the reported list are also in scope:
PlaywrightEnvironmentTests.cs: five Headless comparisons now use IsTrue or IsFalse. These are boolean-comparison diagnostics in the same unit-test project.
SharedObjectDisposeSpanTests.cs: HasCount assertions become Count().IsEqualTo(...) with the same expected counts. This is an assertion-style change in the same project.
The PropertyDataSourceInjectionTests change adds a fallback to Activator.CreateInstance when TestInformation is null. Before this change, a null value reached a non-nullable parameter. This is a small runtime difference in a test helper, and it is the direct result of the CS8604 fix. No production code, public API, or generator output changes appear in the summary. Warnings in analyzer and generator projects are excluded by the issue and by the PR.
Full details: Docstring Coverage
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 11 files.
Finishing Touches
Generate docstrings
Commit to this branch
Create a new PR
Generate unit tests (beta)
Commit to this branch
Create a new PR
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.
A rabbit checks each assertion with care, And finds the async calls waiting there. Null data gets a guarded way, The fallback steps in when needed that day. The test suite thumps its tidy feet, With clearer checks from nose to seat.
Recent review info
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: fb24649d-17d1-4780-9365-85f11c6db55d
Commits
Reviewing files that changed from the base of the PR and between 6bc28cd and b2bd83f.
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 2 remain after this review.
This is a test-only warning cleanup, and I found no blocking issues.
The constant Assert.That(true).IsTrue() bodies became completed Task bodies. The fixtures keep the same semantics, since they only exercise dependency resolution.
The LogSinkRouter "does nothing" tests now use ThrowsNothing(). That still checks the no-throw behavior explicitly, which is better than the tautological assertion it replaces.
The empty-source fixture still fails if it executes, now through Assert.Fail instead of Assert.That(false).IsTrue().
The IsEqualTo(true/false/default) calls moved to IsTrue, IsFalse and IsNull, which are the dedicated assertions.
The TestInformation null guard in PropertyDataSourceInjectionTests falls back to Activator, so behavior is unchanged when the metadata is missing.
Two optional notes:
The async () => { await test.NewContext(...); } wrapper is repeated in several Playwright tests. A small helper would remove the repetition, but it isn't worth blocking on.
The PR description says the build has zero warnings and the targeted tests pass. I haven't re-run them. Engine.Tests was correctly left to CI.
Promoted to ready after positive Claude and Codex verdicts on b2bd83f; all review surfaces have no blocking findings or unresolved threads. The optional helper suggestion is unnecessary for this warning cleanup. Promotion preserved the head. Waiting for fresh Claude review run 42095868415 and ready-state CI before enabling auto-merge.
This is a tests-only cleanup with no shipped code changes. I read the diff but didn't build or run anything, so I'm relying on the PR's reported zero-warning build and its targeted test runs.
What I checked:
Constant assertions: Replacing Assert.That(true).IsTrue() with completed bodies in DependencyResolutionFailureTests keeps the tests' purpose, which is dependency and skip behaviour rather than the assertion.
ThrowsNothing: The LogSinkRouter tests now state the no-throw intent explicitly, which is better than a vacuous assertion.
Empty-source fixture:Assert.Fail in a void method still fails loudly if the SkipIfEmpty test ever executes, so the original guard is preserved.
Exception assertions: The async () => { await ...; } wrappers, and the ThrowsAsync to Assert.That(...).Throws<T>().WithMessage change, remove the warnings from asserting on a returned value. They keep the same semantics.
Property injection: The TestInformation is { } testInformation guard fixes the nullable warning. It falls back to Activator when the metadata is missing, which is what the code already did for an unsuccessful create.
Boolean and count assertions: The switch to IsTrue, IsFalse, IsNull and Count().IsEqualTo is mechanical and correct.
Nit, not blocking: the async () => { await x; } wrapper is repeated in the Playwright tests. A small helper such as static async Task Create(...) could cut the repetition if it spreads further.
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
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.
GitHub Actions
Inactive
Unit-test builds emitted nullable-reference and assertion-style warnings that obscured new diagnostics. Use void-returning async exception assertions, guard optional test metadata before initialization, and use dedicated null/boolean assertions. Replace constant success assertions with completed fixture bodies or explicit no-throw checks, while preserving the empty-source fixture's failure if it executes.
Also replace newly present boolean comparison warnings in PlaywrightEnvironmentTests and obsolete count assertions in SharedObjectDisposeSpanTests.
Validation:
dotnet build tests/TUnit.UnitTests -f net10.0: zero warnings and errors after the changes; baseline reproduced the reported diagnostics.git diff --checkpassed. No shipped public API or generator output changes. Engine.Tests was not run, per repository instructions.Closes #7438
Summary by CodeRabbit