V10.0.3/service update #40
Conversation
… if ITestOutputHelperAccessor is registered, favor this.
…ing present and be forgiving if TestOutput is null
WalkthroughThis update increments package dependency versions, updates the nginx base image in the documentation Dockerfile, and enhances the Changes
Sequence Diagram(s)sequenceDiagram
participant TestHost
participant ServiceCollection
participant AddXunitTestLogging
participant ITestOutputHelperAccessor
participant LoggerProvider
TestHost->>ServiceCollection: ConfigureServices()
ServiceCollection->>AddXunitTestLogging: AddXunitTestLogging()
AddXunitTestLogging->>ServiceCollection: Check for ITestOutputHelperAccessor
alt Accessor present
AddXunitTestLogging->>LoggerProvider: Register with accessor
else Accessor absent
AddXunitTestLogging->>LoggerProvider: Register default provider
end
TestHost->>LoggerProvider: Retrieve logger
LoggerProvider->>TestHost: Logger instance
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR delivers a service update by upgrading dependencies, enhancing test logging robustness, and expanding test coverage.
- Bumped Docker base images and library package versions for compatibility.
- Made
AddXunitTestLoggingtolerant of a nullITestOutputHelperand improved the logger to swallow inactive-test errors. - Added host-based test cases and updated the Docker test runner image.
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| testenvironments.json | Updated the Ubuntu Docker test runner image tag to net8.0.411-9.0.301. |
| test/Codebelt.Extensions.Xunit.Hosting.Tests/ServiceCollectionExtensions.cs | Introduced two new host-based [Fact] tests for AddXunitTestLogging. |
| src/Codebelt.Extensions.Xunit.Hosting/XunitTestLogger.cs | Wrapped TestOutput.WriteLine in a try/catch to suppress InvalidOperationException. |
| src/Codebelt.Extensions.Xunit.Hosting/ServiceCollectionExtensions.cs | Added conditional registration of the XunitTestLoggerProvider when an ITestOutputHelperAccessor is present. |
| Directory.Packages.props | Upgraded multiple Cuemon.*, Microsoft.NET.Test.Sdk, and related package versions. |
| .docfx/Dockerfile.docfx | Bumped nginx image from 1.27.5-alpine to 1.28.0-alpine. |
| CHANGELOG.md | Added the 10.0.3 entry summarizing dependency and logging updates. |
| .nuget/*/PackageReleaseNotes.txt | Updated all sub-package release notes to Version 10.0.3 and adjusted ALM/change sections. |
Comments suppressed due to low confidence (2)
test/Codebelt.Extensions.Xunit.Hosting.Tests/ServiceCollectionExtensions.cs:127
- Consider adding a test where
ITestOutputHelperAccessoris registered but itsTestOutputremains null to verify that no exception is thrown and no log entries are produced.
[Fact]
CHANGELOG.md:16
- [nitpick] The changelog notes the logging change but omits the Docker test runner image bump and newly added host-based tests; consider adding entries to document those updates.
- AddXunitTestLogging method on the ServiceCollectionExtensions class in the Codebelt.Extensions.Xunit.Hosting namespace to use same logic as the overload equivalent and be forgiving when ITestOutputHelper is null (before an InvalidOperationException was thrown)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/Codebelt.Extensions.Xunit.Hosting/ServiceCollectionExtensions.cs (1)
26-45: Conditional branch duplicates the same logging setup – extract to reduce repetitionThe two branches differ only in how the
ILoggerProviderinstance is created. Consider moving the common
builder.SetMinimumLevel(minimumLevel)call outside theif/elseand extracting the provider-creation logic to a local
function to keep the method compact.- services.AddLogging(builder => - { - builder.SetMinimumLevel(minimumLevel); - builder.Services.AddSingleton<ILoggerProvider>(provider => - { - var accessor = provider.GetRequiredService<ITestOutputHelperAccessor>(); - return new XunitTestLoggerProvider(accessor); - }); - }); + services.AddLogging(builder => + { + builder.SetMinimumLevel(minimumLevel); + builder.Services.AddSingleton<ILoggerProvider>(CreateProvider); + }); ... - services.AddLogging(builder => - { - builder.SetMinimumLevel(minimumLevel); - builder.AddProvider(new XunitTestLoggerProvider()); - }); + services.AddLogging(builder => + { + builder.SetMinimumLevel(minimumLevel); + builder.AddProvider(CreateProvider(null)); + });This keeps the two paths easier to maintain and guarantees they never diverge unintentionally.
CHANGELOG.md (1)
14-17: Minor grammar nit – missing article“to use same logic” ➜ “to use the same logic”
- - AddXunitTestLogging method on the ServiceCollectionExtensions class in the Codebelt.Extensions.Xunit.Hosting namespace to use same logic as the overload equivalent and be forgiving when ITestOutputHelper is null + - AddXunitTestLogging method on the ServiceCollectionExtensions class in the Codebelt.Extensions.Xunit.Hosting namespace to use **the** same logic as the overload equivalent and be forgiving when ITestOutputHelper is null🧰 Tools
🪛 LanguageTool
[uncategorized] ~16-~16: You might be missing the article “the” here.
Context: ...tensions.Xunit.Hosting namespace to use same logic as the overload equivalent and be...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
.nuget/Codebelt.Extensions.Xunit.Hosting/PackageReleaseNotes.txt (1)
5-8: Release-notes wording – same missing articleSame sentence as in the changelog – add “the”.
- - CHANGED AddXunitTestLogging method on the ServiceCollectionExtensions class in the Codebelt.Extensions.Xunit.Hosting namespace to use same logic as the overload equivalent ... + - CHANGED AddXunitTestLogging method on the ServiceCollectionExtensions class in the Codebelt.Extensions.Xunit.Hosting namespace to use **the** same logic as the overload equivalent ...🧰 Tools
🪛 LanguageTool
[uncategorized] ~8-~8: You might be missing the article “the” here.
Context: ...tensions.Xunit.Hosting namespace to use same logic as the overload equivalent and be...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
test/Codebelt.Extensions.Xunit.Hosting.Tests/ServiceCollectionExtensions.cs (2)
130-141: Add null-checks and assert expected entry countBefore accessing the store and its first element, add explicit
Assert.NotNullvalidations.
Additionally, you expect exactly one log entry; asserting the count (or usingSingle()) conveys that intent more clearly thanFirst().@@ - var logger = test.Host.Services.GetRequiredService<ILogger<ServiceCollectionExtensions>>(); - logger.LogInformation("Test"); - - var store = logger.GetTestStore(); - - Assert.Equal("Information: Test", store.Query().First().Message); + var logger = test.Host.Services.GetRequiredService<ILogger<ServiceCollectionExtensions>>(); + logger.LogInformation("Test"); + + var store = logger.GetTestStore(); + + Assert.NotNull(logger); + Assert.NotNull(store); + Assert.Single(store.Query()); + Assert.Equal("Information: Test", store.Query().Single().Message);
152-160: Replicate the stronger assertions here as wellMirror the additional null-checks and count assertion in the second host-based test for consistency and clarity.
@@ - var store = logger.GetTestStore(); - - Assert.Equal("Information: Test", store.Query().First().Message); + var store = logger.GetTestStore(); + + Assert.NotNull(logger); + Assert.NotNull(store); + Assert.Single(store.Query()); + Assert.Equal("Information: Test", store.Query().Single().Message);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.docfx/Dockerfile.docfx(2 hunks).nuget/Codebelt.Extensions.Xunit.App/PackageReleaseNotes.txt(1 hunks).nuget/Codebelt.Extensions.Xunit.Hosting.AspNetCore/PackageReleaseNotes.txt(1 hunks).nuget/Codebelt.Extensions.Xunit.Hosting/PackageReleaseNotes.txt(1 hunks).nuget/Codebelt.Extensions.Xunit/PackageReleaseNotes.txt(1 hunks)CHANGELOG.md(1 hunks)Directory.Packages.props(2 hunks)src/Codebelt.Extensions.Xunit.Hosting/ServiceCollectionExtensions.cs(1 hunks)src/Codebelt.Extensions.Xunit.Hosting/XunitTestLogger.cs(1 hunks)test/Codebelt.Extensions.Xunit.Hosting.Tests/ServiceCollectionExtensions.cs(1 hunks)testenvironments.json(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
.nuget/Codebelt.Extensions.Xunit.Hosting.AspNetCore/PackageReleaseNotes.txt (1)
Learnt from: gimlichael
PR: codebeltnet/xunit#17
File: src/Codebelt.Extensions.Xunit.Hosting/Codebelt.Extensions.Xunit.Hosting.csproj:22-26
Timestamp: 2024-10-09T14:10:40.721Z
Learning: In the `src/Codebelt.Extensions.Xunit.Hosting/Codebelt.Extensions.Xunit.Hosting.csproj` file, only certain .NET 8 packages are available in version 8.0.1, while others remain at 8.0.0, so package versions may differ due to availability.
.nuget/Codebelt.Extensions.Xunit/PackageReleaseNotes.txt (1)
Learnt from: gimlichael
PR: codebeltnet/xunit#17
File: src/Codebelt.Extensions.Xunit.Hosting/Codebelt.Extensions.Xunit.Hosting.csproj:22-26
Timestamp: 2024-10-09T14:10:40.721Z
Learning: In the `src/Codebelt.Extensions.Xunit.Hosting/Codebelt.Extensions.Xunit.Hosting.csproj` file, only certain .NET 8 packages are available in version 8.0.1, while others remain at 8.0.0, so package versions may differ due to availability.
.nuget/Codebelt.Extensions.Xunit.App/PackageReleaseNotes.txt (1)
Learnt from: gimlichael
PR: codebeltnet/xunit#17
File: src/Codebelt.Extensions.Xunit.Hosting/Codebelt.Extensions.Xunit.Hosting.csproj:22-26
Timestamp: 2024-10-09T14:10:40.721Z
Learning: In the `src/Codebelt.Extensions.Xunit.Hosting/Codebelt.Extensions.Xunit.Hosting.csproj` file, only certain .NET 8 packages are available in version 8.0.1, while others remain at 8.0.0, so package versions may differ due to availability.
.nuget/Codebelt.Extensions.Xunit.Hosting/PackageReleaseNotes.txt (1)
Learnt from: gimlichael
PR: codebeltnet/xunit#17
File: src/Codebelt.Extensions.Xunit.Hosting/Codebelt.Extensions.Xunit.Hosting.csproj:22-26
Timestamp: 2024-10-09T14:10:40.721Z
Learning: In the `src/Codebelt.Extensions.Xunit.Hosting/Codebelt.Extensions.Xunit.Hosting.csproj` file, only certain .NET 8 packages are available in version 8.0.1, while others remain at 8.0.0, so package versions may differ due to availability.
Directory.Packages.props (1)
Learnt from: gimlichael
PR: codebeltnet/xunit#17
File: src/Codebelt.Extensions.Xunit.Hosting/Codebelt.Extensions.Xunit.Hosting.csproj:22-26
Timestamp: 2024-10-09T14:10:40.721Z
Learning: In the `src/Codebelt.Extensions.Xunit.Hosting/Codebelt.Extensions.Xunit.Hosting.csproj` file, only certain .NET 8 packages are available in version 8.0.1, while others remain at 8.0.0, so package versions may differ due to availability.
🧬 Code Graph Analysis (1)
src/Codebelt.Extensions.Xunit.Hosting/ServiceCollectionExtensions.cs (1)
src/Codebelt.Extensions.Xunit.Hosting/XunitTestLoggerProvider.cs (4)
XunitTestLoggerProvider(9-56)XunitTestLoggerProvider(15-17)XunitTestLoggerProvider(19-22)XunitTestLoggerProvider(24-27)
🪛 LanguageTool
.nuget/Codebelt.Extensions.Xunit.Hosting/PackageReleaseNotes.txt
[uncategorized] ~8-~8: You might be missing the article “the” here.
Context: ...tensions.Xunit.Hosting namespace to use same logic as the overload equivalent and be...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
CHANGELOG.md
[uncategorized] ~16-~16: You might be missing the article “the” here.
Context: ...tensions.Xunit.Hosting namespace to use same logic as the overload equivalent and be...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: call-build (Release) / 🛠️ Build
- GitHub Check: call-build (Debug) / 🛠️ Build
🔇 Additional comments (7)
testenvironments.json (1)
12-12: Bump Docker test image version
TheDocker-Ubuntuenvironment now referencesgimlichael/ubuntu-testrunner:net8.0.411-9.0.301, aligning with the updated .NET SDK versions..docfx/Dockerfile.docfx (1)
1-1: Upgrade nginx base image
Both thebaseandfinalstages have been updated tonginx:1.28.0-alpine. This aligns with the broader dependency bump—please verify that this version is available and compatible with your doc hosting setup.Also applies to: 11-11
.nuget/Codebelt.Extensions.Xunit.Hosting.AspNetCore/PackageReleaseNotes.txt (1)
1-6: Add release notes for v10.0.3
New entry for version 10.0.3 correctly documents the dependency upgrades under ALM. Format and placement are consistent with previous entries..nuget/Codebelt.Extensions.Xunit/PackageReleaseNotes.txt (1)
1-6: Add release notes for v10.0.3
The version header and ALM note are added in line with other Codebelt.Extensions.Xunit packages..nuget/Codebelt.Extensions.Xunit.App/PackageReleaseNotes.txt (1)
1-6: Add release notes for v10.0.3
New version entry and ALM change align with the overall service update.src/Codebelt.Extensions.Xunit.Hosting/ServiceCollectionExtensions.cs (1)
26-27: Order-of-registration gotcha
services.Any(sd => sd.ServiceType == typeof(ITestOutputHelperAccessor))only sees registrations added before the
current call. If the accessor is registered afterAddXunitTestLogging, the method falls back to the “no accessor”
branch and you silently lose directed output.Document this requirement in XML-comments or throw if the accessor is registered later to avoid surprise behaviour.
Directory.Packages.props (1)
7-11: Confirm transitive compatibility after version bumpsSeveral foundational packages (
Microsoft.Extensions.*,xunit.runner.visualstudio,Microsoft.AspNetCore.TestHost)
were bumped to 9.0.6/8.0.17. These are normally safe, but they occasionally introduce binding-redirect issues when
running tests under older SDKs.Please run the test suite on all supported TFMs to ensure there are no version-load exceptions at startup.
Also applies to: 23-33, 40-40
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #40 +/- ##
==========================================
+ Coverage 91.99% 92.09% +0.10%
==========================================
Files 48 48
Lines 937 949 +12
Branches 122 122
==========================================
+ Hits 862 874 +12
Misses 65 65
Partials 10 10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/Codebelt.Extensions.Xunit.Hosting/ServiceCollectionExtensions.cs (2)
26-37: Conditional branch still duplicates logger-configuration logicAfter the refactor, both paths (
AddTestOutputHelperAccessorand theelseblock) still repeat theSetMinimumLevelcall and add a provider differing only by constructor argument.
Consider extracting the commonbuilder.SetMinimumLevel(minimumLevel)and provider-registration into a single helper (e.g., a local function that receives aFunc<IServiceProvider, ILoggerProvider>). This removes duplication and makes future changes (e.g., adding filters) less error-prone.
56-67: Second overload contains the same duplication patternThe same two-branch duplication exists here. Consolidating as suggested above will keep both overloads aligned and avoid diverging behaviour if one branch is updated but the other is forgotten.
🧹 Nitpick comments (1)
src/Codebelt.Extensions.Xunit.Hosting/ServiceCollectionExtensions.cs (1)
26-37: Order-of-registration edge caseBecause the decision to use
AddTestOutputHelperAccessoris based on current presence ofITestOutputHelperAccessor, callingAddXunitTestLoggingbefore registering the accessor will silently fall back to the “no-accessor” provider and never switch, leading to lost test output.
You can make the registration resilient to ordering by always registering a provider that resolvesITestOutputHelperAccessorlazily:builder.Services.AddSingleton<ILoggerProvider>(sp => { var accessor = sp.GetService<ITestOutputHelperAccessor>(); return accessor != null ? new XunitTestLoggerProvider(accessor) : new XunitTestLoggerProvider(); });This removes the need for the upfront
Anycheck altogether.Also applies to: 56-67
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Codebelt.Extensions.Xunit.Hosting/ServiceCollectionExtensions.cs(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/Codebelt.Extensions.Xunit.Hosting/ServiceCollectionExtensions.cs (1)
src/Codebelt.Extensions.Xunit.Hosting/XunitTestLoggerProvider.cs (4)
XunitTestLoggerProvider(9-56)XunitTestLoggerProvider(15-17)XunitTestLoggerProvider(19-22)XunitTestLoggerProvider(24-27)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: call-test (ubuntu-24.04, Debug) / 🧪 Test
- GitHub Check: call-test (windows-2022, Release) / 🧪 Test
- GitHub Check: call-test (ubuntu-24.04, Release) / 🧪 Test
- GitHub Check: call-test (windows-2022, Debug) / 🧪 Test
- GitHub Check: call-pack (Debug) / 📦 Pack
- GitHub Check: call-pack (Release) / 📦 Pack
|
1 participant
This pull request includes updates to dependencies, improvements to logging functionality, and enhancements to testing environments. The most significant changes involve upgrading package versions, refining the
AddXunitTestLoggingmethod for better handling of nullITestOutputHelperinstances, and adding new test cases to ensure robust functionality.Dependency Updates:
.docfx/Dockerfile.docfx: Upgraded the base image fromnginx:1.27.5-alpinetonginx:1.28.0-alpine. [1] [2]Directory.Packages.props: Updated multiple package versions, includingCuemon.Core,Cuemon.Extensions.AspNetCore, andMicrosoft.NET.Test.Sdkto their latest versions for improved compatibility. [1] [2]Logging Enhancements:
src/Codebelt.Extensions.Xunit.Hosting/ServiceCollectionExtensions.cs: Modified theAddXunitTestLoggingmethod to avoid throwing anInvalidOperationExceptionwhenITestOutputHelperis null, making it more forgiving and robust.src/Codebelt.Extensions.Xunit.Hosting/XunitTestLogger.cs: Updated the logging logic to handle scenarios where there is no active test, preventing potential exceptions.Testing Improvements:
test/Codebelt.Extensions.Xunit.Hosting.Tests/ServiceCollectionExtensions.cs: Added new test cases to validate theAddXunitTestLoggingmethod, including scenarios with and withoutITestOutputHelperAccessor.testenvironments.json: Updated the Docker image for the Ubuntu test runner togimlichael/ubuntu-testrunner:net8.0.411-9.0.301.Summary by CodeRabbit