-
Notifications
You must be signed in to change notification settings - Fork 1
V8.4.1/hotfix add xunit test logging #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… and forward compatibility)
WalkthroughThe pull request introduces significant updates across multiple projects within the Codebelt.Extensions.Xunit ecosystem. Key changes include semantic revisions to README.md files to clarify the focus on the xUnit.net test platform, enhancements to the Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (10)
- .nuget/Codebelt.Extensions.Xunit.App/README.md (1 hunks)
- .nuget/Codebelt.Extensions.Xunit.Hosting.AspNetCore/README.md (1 hunks)
- .nuget/Codebelt.Extensions.Xunit.Hosting/PackageReleaseNotes.txt (1 hunks)
- .nuget/Codebelt.Extensions.Xunit.Hosting/README.md (1 hunks)
- CHANGELOG.md (1 hunks)
- src/Codebelt.Extensions.Xunit.Hosting/LoggerExtensions.cs (2 hunks)
- src/Codebelt.Extensions.Xunit.Hosting/ServiceCollectionExtensions.cs (2 hunks)
- test/Codebelt.Extensions.Xunit.Hosting.AspNetCore.Tests/AspNetCoreHostTestTest.cs (2 hunks)
- test/Codebelt.Extensions.Xunit.Hosting.AspNetCore.Tests/Codebelt.Extensions.Xunit.Hosting.AspNetCore.Tests.csproj (1 hunks)
- test/Codebelt.Extensions.Xunit.Hosting.AspNetCore.Tests/WebHostTestFactoryTest.cs (5 hunks)
Additional comments not posted (15)
test/Codebelt.Extensions.Xunit.Hosting.AspNetCore.Tests/Codebelt.Extensions.Xunit.Hosting.AspNetCore.Tests.csproj (1)
9-9: LGTM!The change from
Cuemon.AspNetCoretoCuemon.Extensions.AspNetCoreis a valid package reference update. This change may provide additional functionalities or improvements for testing ASP.NET Core extensions.As the version number remains the same, this change is unlikely to introduce any breaking changes or compatibility issues.
.nuget/Codebelt.Extensions.Xunit.App/README.md (1)
3-3: LGTM!The changes to the README.md file effectively clarify the project's focus on the xUnit.net test platform and its applicability across all project types in .NET. The revised description enhances the clarity and relevance of the project for potential users.
.nuget/Codebelt.Extensions.Xunit.Hosting/README.md (1)
3-3: LGTM!The changes made to the README file improve the clarity of the project's purpose by explicitly stating that it targets the xUnit.net test platform. This helps developers better understand the project's scope and applicability.
.nuget/Codebelt.Extensions.Xunit.Hosting/PackageReleaseNotes.txt (1)
9-9: Acknowledge the extension ofServiceCollectionExtensionsclass.The release notes document the addition of three new extension methods to the
ServiceCollectionExtensionsclass:
AddXunitTestOutputHelperAccessorforIServiceCollectionAddXunitTestOutputHelperAccessor{T}forIServiceCollection- Overload of
AddXunitTestLoggingforIServiceCollectionThese changes are consistent with the AI-generated summary. However, the implementation details of these methods should be reviewed in the corresponding source code files to ensure correctness and adherence to best practices.
.nuget/Codebelt.Extensions.Xunit.Hosting.AspNetCore/README.md (1)
3-3: LGTM!The revised paragraph provides a clearer and more specific description of the project's intended use and audience. The changes enhance clarity and specificity regarding the project's focus on the xUnit.net test platform and its applicability to various project types within the .NET ecosystem. The removal of references to specific .NET versions also broadens the project's applicability.
src/Codebelt.Extensions.Xunit.Hosting/LoggerExtensions.cs (1)
17-17: LGTM!The updated documentation comment provides more context about the usage of the
GetTestStoremethod and clarifies that it can return theITestStore<T>from two different overloads of theAddXunitTestLoggingmethod. This improves the clarity and completeness of the documentation.test/Codebelt.Extensions.Xunit.Hosting.AspNetCore.Tests/AspNetCoreHostTestTest.cs (1)
37-38: LGTM!The logging statement is a good addition to enhance traceability during test execution. The
ILoggerinstance is correctly retrieved, and the informational message is logged appropriately.test/Codebelt.Extensions.Xunit.Hosting.AspNetCore.Tests/WebHostTestFactoryTest.cs (4)
23-23: LGTM!The method name change improves clarity and follows a consistent naming convention.
40-40: LGTM!The method name change improves clarity by emphasizing the asynchronous behavior and follows a consistent naming convention.
53-53: LGTM!The method name change improves clarity by emphasizing the asynchronous behavior, the usage of
HostBuilderContext, and follows a consistent naming convention.
81-112: Great addition!The new test method
RunAsync_ShouldWorkWithXunitTestLoggingenhances the test coverage by validating the integration between middleware and logging. It sets up a test environment where server timing is utilized and verifies that the response headers include the expected server timing information.This addition ensures that the middleware behaves correctly when integrated with logging services, improving the robustness of the testing suite.
src/Codebelt.Extensions.Xunit.Hosting/ServiceCollectionExtensions.cs (3)
36-58: LGTM!The
AddXunitTestLoggingmethod is well-implemented and follows the best practices:
- The method signature and XML documentation are clear and informative.
- The null checks for
servicesandaccessorparameters ensure the method's robustness.- The logging configuration is properly set up using the provided
accessorandminimumLevel.Great job!
65-69: Looks good!The renaming of the
AddXunitTestOutputHelperAccessormethod toAddXunitTestLoggingOutputHelperAccessorimproves clarity and maintainability by aligning the method name with its purpose in the context of logging.The method implementation remains unchanged, properly invoking the generic version with the default
TestOutputHelperAccessor.
Line range hint
80-86: Looks good!The renaming of the generic
AddXunitTestOutputHelperAccessor<T>method toAddXunitTestLoggingOutputHelperAccessor<T>improves clarity and maintainability by aligning the method name with its purpose in the context of logging.The method implementation remains unchanged, properly adding the specified implementation of
ITestOutputHelperAccessorto theservicescollection as a singleton.The generic type constraint
where T : class, ITestOutputHelperAccessorensures that the provided typeTis a class that implementsITestOutputHelperAccessor, maintaining type safety.CHANGELOG.md (1)
18-18: LGTM!The changelog entry accurately documents the addition of the new extension methods in the
ServiceCollectionExtensionsclass. The description is clear and consistent with the AI-generated summary.
| services.AddXunitTestLoggingOutputHelperAccessor(); | ||
| services.AddXunitTestLogging(new TestOutputHelperAccessor(TestOutput)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tip
Codebase Verification
Update remaining test files to use the new Xunit test logging approach
The new Xunit test logging approach is not consistently applied across all test files. Two files still use the old approach:
- test/Codebelt.Extensions.Xunit.Hosting.Tests/ServiceCollectionExtensions.cs
- test/Codebelt.Extensions.Xunit.Hosting.AspNetCore.Tests/WebHostTestFactoryTest.cs
Please update these files to use the new approach, following the pattern in AspNetCoreHostTestTest.cs:
services.AddXunitTestLoggingOutputHelperAccessor();
services.AddXunitTestLogging(new TestOutputHelperAccessor(TestOutput));Additionally, perform a manual review of other test files to ensure complete migration to the new approach, as there might be files that weren't caught by our search.
Analysis chain
Verify the consistency of the new Xunit test logging approach.
The changes in the service configuration for adding Xunit test logging look good. The new approach using AddXunitTestLoggingOutputHelperAccessor and AddXunitTestLogging with a TestOutputHelperAccessor instance seems to be an improvement in integrating with the Xunit logging framework.
To ensure consistency, please verify that this new approach is applied consistently throughout the test project. You can use the following script to search for other test files and check if they need to be updated:
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the consistency of the new Xunit test logging approach in other test files.
# Test: Search for other test files. Expect: Occurrences of test files that may need to be updated.
fd --type file --extension cs --exclude Program.cs --exec rg --type cs $'services\.AddXunitTestOutputHelperAccessor\(\);|services\.AddXunitTestLogging\(TestOutput\);'
Length of output: 284
Script:
#!/bin/bash
# Description: Identify files with old and new Xunit test logging approaches and provide context.
# Search for files with the old approach
echo "Files with old approach:"
rg --type cs -n "services\.AddXunitTestLogging\(TestOutput\);" -C 2
echo "\nFiles with new approach:"
rg --type cs -n "services\.AddXunitTestLoggingOutputHelperAccessor\(\);" -C 2
echo "\nFiles with TestOutputHelperAccessor:"
rg --type cs -n "new TestOutputHelperAccessor\(TestOutput\)" -C 2
Length of output: 2894
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5 +/- ##
==========================================
- Coverage 74.84% 73.55% -1.29%
==========================================
Files 26 26
Lines 493 503 +10
Branches 42 42
==========================================
+ Hits 369 370 +1
- Misses 120 129 +9
Partials 4 4 ☔ View full report in Codecov by Sentry. |
PR Classification
API change and documentation update to support xUnit.net test platform.
PR Summary
Updated project to target xUnit.net and added new overload for
AddXunitTestLoggingmethod.ServiceCollectionExtensions.cs: Added new overload ofAddXunitTestLoggingand renamed methods.LoggerExtensions.cs: Updated to reference new overload.AspNetCoreHostTestTest.cs: Updated to use new method names and overload.WebHostTestFactoryTest.cs: Renamed test methods and added new test for logging functionality.README.md: Updated to reflect xUnit.net targeting.Summary by CodeRabbit
New Features
Documentation
Chores