Skip to content

Conversation

@gimlichael
Copy link
Member

@gimlichael gimlichael commented Mar 30, 2025

This pull request includes several changes to improve the functionality and maintainability of the Codebelt.Extensions.Xunit.Hosting.AspNetCore library. The most important changes include adding documentation, refactoring the AspNetCoreHostFixture class, introducing a new BlockingAspNetCoreHostFixture class, and updating method signatures to use interfaces for better flexibility.

Documentation updates:

  • README.md: Added a link to the full documentation generated by DocFx.

Refactoring and enhancements:

New class introduction:

  • BlockingAspNetCoreHostFixture.cs: Introduced a new class BlockingAspNetCoreHostFixture that extends AspNetCoreHostFixture to provide synchronous host starting.

Suppressions for code analysis:

  • GlobalSuppressions.cs: Added suppressions for method overloads with default parameter values to avoid bumping the major version.

Summary by CodeRabbit

  • Documentation

    • Added a link in the README that directs users to the full online documentation.
  • New Features

    • Introduced additional API overloads for host configuration, including a new synchronous option to better support varying usage scenarios.
    • Expanded security exception handling with enhanced test coverage.
    • Added new classes for blocking host fixture functionality and extension methods for host fixture validation.
  • Refactor

    • Streamlined host initialization workflows and improved logging integration for increased reliability and maintainability.
    • Simplified host starting logic and improved code organization.
    • Centralized host fixture initialization logic for maintainability.
    • Updated method access levels and added XML documentation for clarity.
  • Chores

    • Updated various package dependencies to their newer versions.

@gimlichael gimlichael self-assigned this Mar 30, 2025
Copilot AI review requested due to automatic review settings March 30, 2025 21:17
@coderabbitai
Copy link

coderabbitai bot commented Mar 30, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

This PR introduces multiple improvements across documentation, host initialization, logging, and testing. A documentation link has been added to the README. The host fixture startup is simplified with a new method (StartConfiguredHost) and new blocking fixture support. Several extension classes have updated visibility and XML documentation while obsolete methods have been replaced with overloads accepting an optional host fixture. Logging classes now separate provider management from in-memory storage. Test classes and factories are updated to use centralized fixture initialization logic and include additional security exception verifications.

Changes

File(s) Change Summary
README.md Added a new line with a link to the full DocFx-generated documentation at https://xunit.codebelt.net/.
src/Codebelt.Extensions.Xunit.Hosting.AspNetCore/AspNetCoreHostFixture.cs
src/Codebelt.Extensions.Xunit.Hosting.AspNetCore/AspNetCoreHostFixtureExtensions.cs
src/Codebelt.Extensions.Xunit.Hosting.AspNetCore/AspNetCoreHostTest.cs
src/Codebelt.Extensions.Xunit.Hosting.AspNetCore/BlockingAspNetCoreHostFixture.cs
Refactored host startup logic by replacing asynchronous task management with a direct call to StartConfiguredHost; updated fixture extension access levels and documentation; removed redundant initialization in test host classes.
src/Codebelt.Extensions.Xunit.Hosting.AspNetCore/WebHostTest.cs
src/Codebelt.Extensions.Xunit.Hosting.AspNetCore/WebHostTestFactory.cs
Updated type parameters from concrete to interface (IAspNetCoreHostFixture), centralized host fixture initialization, introduced new method overloads with an optional host fixture parameter, and marked previous methods as obsolete.
src/Codebelt.Extensions.Xunit.Hosting/HostFixture.cs
src/Codebelt.Extensions.Xunit.Hosting/HostFixtureExtensions.cs
src/Codebelt.Extensions.Xunit.Hosting/HostTest.cs
src/Codebelt.Extensions.Xunit.Hosting/IHostFixture.cs
Consolidated host fixture configuration by adding a new StartConfiguredHost method; unified and streamlined the IHostFixture interface and removed inline initialization logic; updated the Configure method’s accessibility.
src/Codebelt.Extensions.Xunit.Hosting/LoggerExtensions.cs
src/Codebelt.Extensions.Xunit.Hosting/XunitTestLogger.cs
src/Codebelt.Extensions.Xunit.Hosting/XunitTestLoggerProvider.cs
Modified logging functionality: the generic GetTestStore was replaced with a non-generic version; XunitTestLogger now uses provider integration (no longer inherits from an in-memory store); added a new method to write log entries in the provider.
src/Codebelt.Extensions.Xunit.Hosting/GlobalSuppressions.cs Introduced new suppressions for “Blocker Code Smell” on method overloads with default parameter values to avoid a major version bump.
Test files in test/Codebelt.Extensions.Xunit.Hosting.AspNetCore.Tests/ and test/Codebelt.Extensions.Xunit.Hosting.Tests/ Enhanced test initialization logic with state validation via HasValidState(), set up configuration callbacks when needed, and added new security exception tests in WebHostTestFactoryTest with updated invocation signatures using the optional host fixture parameter.

Sequence Diagram(s)

sequenceDiagram
    participant T as Test
    participant HF as HostFixture
    participant CH as StartConfiguredHost
    participant Cfg as Configuration

    T->>HF: Check HasValidState()
    alt Fixture not valid
        HF->>HF: Set ConfigureHost, Configure, Services, & Application callbacks
        HF->>CH: Call StartConfiguredHost()
        CH-->>HF: Host started (blocking/async)
        HF->>T: Provide Host, ServiceProvider, Application
        T->>Cfg: Call Configure(configuration, environment)
    else Fixture valid
        HF-->>T: Existing Host state provided
    end
Loading
sequenceDiagram
    participant Test as WebHostTestFactoryTest
    participant WHF as WebHostTestFactory
    participant HostF as HostFixture

    Test->>WHF: Invoke Create(..., hostFixture)
    alt Using Blocking Fixture
        WHF->>HostF: Initialize blocking host
        HostF-->>WHF: Throws SecurityException
        WHF-->>Test: Exception propagated
    else Using Non-blocking Fixture
        WHF->>HostF: Initialize non-blocking host
        HostF-->>WHF: Captures and logs SecurityException
        WHF-->>Test: Returns response with logged exception
    end
Loading

Possibly related PRs

Poem

I’m a rabbit leaping through the code,
Hopping over changes in a playful mode.
New methods and callbacks now pave the way,
With hosts starting true, both night and day.
Logging and tests dance a jolly tune,
My whiskers twitch under the glowing moon.
CodeRabbit cheers – our fixes make hearts swoon! 🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Free

📥 Commits

Reviewing files that changed from the base of the PR and between 9795cff and 854c2a8.

📒 Files selected for processing (1)
  • testenvironments.json (1 hunks)

Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai plan to trigger planning for file edits and PR creation.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request improves the functionality and maintainability of the Codebelt.Extensions.Xunit.Hosting.AspNetCore library by updating documentation, refactoring host fixture initialization, and introducing a blocking fixture for synchronous host start.

  • Updated documentation with a DocFx-generated link in README.md
  • Refactored host fixture initialization in multiple files to separate configuration, building, and starting the host
  • Introduced BlockingAspNetCoreHostFixture for synchronous host starting and updated method overloads to support a fixture parameter

Reviewed Changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/Codebelt.Extensions.Xunit.Hosting.AspNetCore.Tests/AspNetCoreHostTestTest.cs Adjusted fixture initialization, but potential duplicate invocation of configuration logic.
src/Codebelt.Extensions.Xunit.Hosting/XunitTestLoggerProvider.cs Updated logger provider to extend InMemoryTestStore and pass itself to logger instances.
src/Codebelt.Extensions.Xunit.Hosting/XunitTestLogger.cs Modified logger to delegate log entry storage to its provider.
src/Codebelt.Extensions.Xunit.Hosting/LoggerExtensions.cs Changed generic GetTestStore to non-generic version with XML documentation updates.
src/Codebelt.Extensions.Xunit.Hosting/IHostFixture.cs Shifted ConfigureCallback member from a partial interface to the primary interface definition.
src/Codebelt.Extensions.Xunit.Hosting/HostTest.cs Removed host fixture initialization helper and changed Configure from virtual to non-virtual.
src/Codebelt.Extensions.Xunit.Hosting/HostFixture.cs Refactored ConfigureHost to build the host and start it via a new StartConfiguredHost method.
src/Codebelt.Extensions.Xunit.Hosting/GenericHostTestFactory.cs Added overloaded Create methods to optionally accept a host fixture parameter.
src/Codebelt.Extensions.Xunit.Hosting.AspNetCore/WebHostTestFactory.cs Added overloads for Create, CreateWithHostBuilderContext, and RunAsync accepting an IAspNetCoreHostFixture.
src/Codebelt.Extensions.Xunit.Hosting.AspNetCore/WebHostTest.cs Changed usage from concrete AspNetCoreHostFixture to the IAspNetCoreHostFixture interface and refactored initialization.
src/Codebelt.Extensions.Xunit.Hosting.AspNetCore/AspNetCoreHostFixture.cs Updated ConfigureHost to build the host and invoke StartConfiguredHost.
README.md Added a DocFx documentation link.
Comments suppressed due to low confidence (1)

test/Codebelt.Extensions.Xunit.Hosting.AspNetCore.Tests/AspNetCoreHostTestTest.cs:24

  • [nitpick] The fixture initialization block invokes hostFixture.ConfigureHost(this) and then later calls Configure explicitly, which could result in duplicate configuration. Consider refactoring the initialization to avoid redundant calls to Configure.
if (!hostFixture.HasValidState()) {

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (6)
README.md (1)

9-9: Documentation link should use proper markdown formatting

The added documentation link is valuable but uses a bare URL, which violates markdown best practices.

-Full documentation (generated by [DocFx](https://github.com/dotnet/docfx)) located here: https://xunit.codebelt.net/
+Full documentation (generated by [DocFx](https://github.com/dotnet/docfx)) located here: [https://xunit.codebelt.net/](https://xunit.codebelt.net/)
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)

9-9: Bare URL used
null

(MD034, no-bare-urls)

test/Codebelt.Extensions.Xunit.Hosting.Tests/Assets/ValidHostTest.cs (1)

4-4: Remove unused import.

The import for System.Net.Mime.MediaTypeNames appears to be unused in this class.

- using static System.Net.Mime.MediaTypeNames;
src/Codebelt.Extensions.Xunit.Hosting/GenericHostTestFactory.cs (1)

29-34: New overload supports custom fixtures.

Allowing an optional IHostFixture parameter increases flexibility. Consider throwing an ArgumentNullException if the developer explicitly provides a null fixture, ensuring that any required fixture initialization logic remains consistent.

 public static IGenericHostTest Create(Action<IServiceCollection> serviceSetup = null, Action<IHostBuilder> hostSetup = null, IHostFixture hostFixture = null)
 {
+    if (hostFixture == null)
+    {
+        // e.g. throw new ArgumentNullException(nameof(hostFixture));
+    }
     return new GenericHostTest(serviceSetup, hostSetup, hostFixture ?? new HostFixture());
 }
src/Codebelt.Extensions.Xunit.Hosting/GenericHostTest.cs (1)

28-40: Centralized fixture initialization enhances maintainability.

Encapsulating fixture checks and configuration callbacks in InitializeHostFixture clarifies the constructor flow. Consider an additional safety check for hostFixture null references if potentially passed incorrectly.

src/Codebelt.Extensions.Xunit.Hosting/XunitTestLogger.cs (1)

12-24: Proactive null checks recommended in constructors.

You store _provider and either _output or _accessor without validating they are non-null. For better fail-fast behavior, consider adding ArgumentNullException checks to avoid subtle runtime errors if anything is unexpectedly null.

public XunitTestLogger(XunitTestLoggerProvider provider, ITestOutputHelper output)
{
+    if (provider == null) throw new ArgumentNullException(nameof(provider));
+    if (output == null) throw new ArgumentNullException(nameof(output));
    _provider = provider;
    _output = output;
}

public XunitTestLogger(XunitTestLoggerProvider provider, ITestOutputHelperAccessor accessor)
{
+    if (provider == null) throw new ArgumentNullException(nameof(provider));
    _provider = provider;
    _accessor = accessor;
}
test/Codebelt.Extensions.Xunit.Hosting.AspNetCore.Tests/WebHostTestFactoryTest.cs (1)

143-175: Consider replacing the synchronous delay with an asynchronous wait approach.

Using Thread.Sleep(400) can slow down tests unnecessarily. An asynchronous alternative (e.g., await Task.Delay(400).ConfigureAwait(false)) may be more aligned with async best practices.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4625540 and 945108d.

📒 Files selected for processing (25)
  • README.md (1 hunks)
  • src/Codebelt.Extensions.Xunit.Hosting.AspNetCore/AspNetCoreHostFixture.cs (1 hunks)
  • src/Codebelt.Extensions.Xunit.Hosting.AspNetCore/AspNetCoreHostFixtureExtensions.cs (1 hunks)
  • src/Codebelt.Extensions.Xunit.Hosting.AspNetCore/AspNetCoreHostTest.cs (0 hunks)
  • src/Codebelt.Extensions.Xunit.Hosting.AspNetCore/BlockingAspNetCoreHostFixture.cs (1 hunks)
  • src/Codebelt.Extensions.Xunit.Hosting.AspNetCore/GlobalSuppressions.cs (1 hunks)
  • src/Codebelt.Extensions.Xunit.Hosting.AspNetCore/HttpClientExtensions.cs (1 hunks)
  • src/Codebelt.Extensions.Xunit.Hosting.AspNetCore/WebHostTest.cs (2 hunks)
  • src/Codebelt.Extensions.Xunit.Hosting.AspNetCore/WebHostTestFactory.cs (4 hunks)
  • src/Codebelt.Extensions.Xunit.Hosting/GenericHostTest.cs (1 hunks)
  • src/Codebelt.Extensions.Xunit.Hosting/GenericHostTestFactory.cs (1 hunks)
  • src/Codebelt.Extensions.Xunit.Hosting/GlobalSuppressions.cs (1 hunks)
  • src/Codebelt.Extensions.Xunit.Hosting/HostFixture.cs (2 hunks)
  • src/Codebelt.Extensions.Xunit.Hosting/HostFixtureExtensions.cs (1 hunks)
  • src/Codebelt.Extensions.Xunit.Hosting/HostTest.cs (1 hunks)
  • src/Codebelt.Extensions.Xunit.Hosting/IHostFixture.cs (2 hunks)
  • src/Codebelt.Extensions.Xunit.Hosting/LoggerExtensions.cs (2 hunks)
  • src/Codebelt.Extensions.Xunit.Hosting/XunitTestLogger.cs (3 hunks)
  • src/Codebelt.Extensions.Xunit.Hosting/XunitTestLoggerProvider.cs (2 hunks)
  • test/Codebelt.Extensions.Xunit.Hosting.AspNetCore.Tests/AspNetCoreHostTestTest.cs (1 hunks)
  • test/Codebelt.Extensions.Xunit.Hosting.AspNetCore.Tests/Assets/ValidHostTest.cs (1 hunks)
  • test/Codebelt.Extensions.Xunit.Hosting.AspNetCore.Tests/MvcAspNetCoreHostTestTest.cs (1 hunks)
  • test/Codebelt.Extensions.Xunit.Hosting.AspNetCore.Tests/WebHostTestFactoryTest.cs (1 hunks)
  • test/Codebelt.Extensions.Xunit.Hosting.Tests/Assets/ValidHostTest.cs (1 hunks)
  • test/Codebelt.Extensions.Xunit.Hosting.Tests/HostTestTest.cs (1 hunks)
💤 Files with no reviewable changes (1)
  • src/Codebelt.Extensions.Xunit.Hosting.AspNetCore/AspNetCoreHostTest.cs
🧰 Additional context used
🧬 Code Definitions (4)
src/Codebelt.Extensions.Xunit.Hosting.AspNetCore/AspNetCoreHostFixture.cs (1)
src/Codebelt.Extensions.Xunit.Hosting/HostFixture.cs (1)
  • StartConfiguredHost (80-86)
src/Codebelt.Extensions.Xunit.Hosting/HostFixtureExtensions.cs (1)
src/Codebelt.Extensions.Xunit.Hosting.AspNetCore/AspNetCoreHostFixtureExtensions.cs (1)
  • HasValidState (18-22)
src/Codebelt.Extensions.Xunit.Hosting/XunitTestLoggerProvider.cs (1)
src/Codebelt.Extensions.Xunit.Hosting/XunitTestLogger.cs (3)
  • XunitTestLogger (8-61)
  • XunitTestLogger (14-18)
  • XunitTestLogger (20-24)
test/Codebelt.Extensions.Xunit.Hosting.AspNetCore.Tests/WebHostTestFactoryTest.cs (1)
test/Codebelt.Extensions.Xunit.Hosting.AspNetCore.Tests/AspNetCoreHostTestTest.cs (5)
  • Fact (42-64)
  • Fact (67-75)
  • Fact (78-101)
  • Fact (103-126)
  • Fact (128-139)
🪛 markdownlint-cli2 (0.17.2)
README.md

9-9: Bare URL used
null

(MD034, no-bare-urls)

⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: 🧪 Test (windows-2022, Debug)
  • GitHub Check: 🧪 Test (windows-2022, Release)
🔇 Additional comments (55)
src/Codebelt.Extensions.Xunit.Hosting/GlobalSuppressions.cs (1)

10-11: Suppression correctly addresses code analysis warning to maintain semantic versioning

The added suppressions for rule S3427 properly address the method overload warnings for GenericHostTestFactory.Create and CreateWithHostBuilderContext methods. This is a reasonable approach to avoid a major version bump while providing enhanced API flexibility with optional parameters.

src/Codebelt.Extensions.Xunit.Hosting.AspNetCore/BlockingAspNetCoreHostFixture.cs (1)

1-20: Well-structured synchronous host fixture implementation

The new BlockingAspNetCoreHostFixture class provides a good synchronous alternative to the default asynchronous implementation, with proper XML documentation and inheritance structure. This enhancement supports better exception capturing in synchronous testing scenarios.

src/Codebelt.Extensions.Xunit.Hosting.AspNetCore/HttpClientExtensions.cs (1)

21-21: Documentation properly updated with full method signatures

The XML documentation remarks have been correctly updated to reference the complete method signatures including the new IAspNetCoreHostFixture parameter, maintaining accurate API reference documentation.

test/Codebelt.Extensions.Xunit.Hosting.Tests/HostTestTest.cs (1)

23-33: Well-structured host initialization pattern added.

The new conditional initialization pattern ensures proper host configuration when the fixture doesn't have a valid state. This promotes more reliable test execution by centralizing the initialization logic in the constructor rather than requiring separate initialization methods.

src/Codebelt.Extensions.Xunit.Hosting.AspNetCore/AspNetCoreHostFixture.cs (1)

93-95: Good refactoring of host initialization logic.

The host building and starting logic has been cleanly separated by using the new StartConfiguredHost() method. This refactoring improves code maintainability by centralizing the host starting logic in a dedicated method that handles the asynchronous operations and deadlock prevention.

test/Codebelt.Extensions.Xunit.Hosting.AspNetCore.Tests/AspNetCoreHostTestTest.cs (1)

24-36: Consistent initialization pattern applied.

The conditional initialization logic follows the same pattern as implemented in other test classes, ensuring consistent behavior across the test suite. This approach properly configures the host fixture when it's not in a valid state, establishing the necessary callbacks and properties before test execution.

test/Codebelt.Extensions.Xunit.Hosting.AspNetCore.Tests/Assets/ValidHostTest.cs (1)

10-22: Consistent initialization pattern applied.

The same host initialization pattern has been properly implemented here, maintaining consistency with other test classes. This systematic approach ensures that all test classes handle fixture initialization in the same way, improving code maintainability and test reliability.

test/Codebelt.Extensions.Xunit.Hosting.Tests/Assets/ValidHostTest.cs (1)

12-22: LGTM - Improved host initialization with conditional state checking.

The new conditional host initialization logic is a good improvement, ensuring that the host is properly configured only if its state is invalid. This helps prevent redundant configuration and aligns with the fixture initialization changes mentioned in the PR objectives.

test/Codebelt.Extensions.Xunit.Hosting.AspNetCore.Tests/MvcAspNetCoreHostTestTest.cs (1)

20-32: LGTM - Consistent host fixture initialization pattern.

The conditional initialization pattern matches the approach used in other test classes. This ensures the host fixture is only configured when needed and consistently sets up the test environment properties.

src/Codebelt.Extensions.Xunit.Hosting/XunitTestLoggerProvider.cs (3)

7-7: LGTM - Enhanced logger provider with in-memory storage.

The XunitTestLoggerProvider now inherits from InMemoryTestStore<XunitTestLoggerEntry>, which is a good architectural improvement separating provider management from storage functionality.


25-28: LGTM - Updated logger creation to use the provider instance.

Passing this to the logger constructor properly connects the logger instances with their provider for centralized log management.


30-33: LGTM - Added method for centralized log entry management.

The new WriteLoggerEntry method centralizes the creation and storage of log entries, which improves the separation of concerns between the logger and its provider.

src/Codebelt.Extensions.Xunit.Hosting/HostFixture.cs (4)

71-74: LGTM - Improved host configuration with clearer separation of concerns.

The refactored ConfigureHost method now properly separates host building from host starting, delegating the starting process to the new StartConfiguredHost method. This improves maintainability and makes the code easier to extend or override in derived classes.


76-86: LGTM - New method for centralizing host startup logic.

The new StartConfiguredHost method appropriately encapsulates the host starting logic, including important documentation about potential deadlocks. This makes the code more maintainable and helps prevent issues when working with ASP.NET Core integration tests.


62-66: LGTM - Improved formatting of conditional compilation block.

The formatting improvements to the conditional compilation block maintain the same functionality while improving readability.


191-195: LGTM - Consistent formatting of fallback implementation.

The formatting improvements to the fallback implementation of OnDisposeManagedResourcesAsync maintain the same functionality while improving readability.

src/Codebelt.Extensions.Xunit.Hosting/IHostFixture.cs (2)

17-17: Interface consolidation looks good.

The removal of the partial keyword simplifies the interface structure by consolidating all members into a single declaration, improving code maintainability.


38-42: Property relocation enhances code organization.

Moving the ConfigureCallback property from a separate partial declaration to the main interface improves discoverability of all interface members in one place, making the code easier to understand and maintain.

src/Codebelt.Extensions.Xunit.Hosting/HostFixtureExtensions.cs (3)

3-6: Enhanced accessibility and documentation.

Changing the class from internal to public appropriately exposes this utility to external consumers, which aligns with the PR objectives of enhancing the library's functionality. The added XML documentation improves the usability of the API.


8-17: Comprehensive method documentation.

The added XML documentation clearly explains the purpose, parameters, return value, and conditions for a valid state, which greatly enhances the usability of the method for consumers.


18-23: Improved code formatting.

The reformatted return statement enhances readability while maintaining the same logical conditions for determining a valid state.

src/Codebelt.Extensions.Xunit.Hosting/HostTest.cs (1)

67-67: Removal of virtual modifier restricts extensibility.

Removing the virtual keyword from the Configure method prevents derived classes from overriding this behavior, which appears to be an intentional design change to enforce consistent configuration handling across all host tests.

Ensure this change was intentional as it reduces extensibility. Consider documenting this design decision if extensibility was previously allowed but is now being restricted.

src/Codebelt.Extensions.Xunit.Hosting.AspNetCore/AspNetCoreHostFixtureExtensions.cs (3)

3-6: Enhanced accessibility and documentation.

Making this class public exposes these extension methods to consumers outside the assembly, which aligns with the PR's goal of improving library functionality. The added XML documentation improves API usability.


8-17: Comprehensive method documentation.

The detailed XML documentation clearly explains the purpose, parameters, return value, and conditions for a valid state, making the API more user-friendly.


18-22: Improved implementation with code reuse.

The refactored implementation reduces duplication by delegating part of the validation to the IHostFixture.HasValidState() method, which follows the DRY principle. This change also makes the method more maintainable - if the base interface's validation logic changes, this method will automatically incorporate those changes.

src/Codebelt.Extensions.Xunit.Hosting/LoggerExtensions.cs (2)

25-50: Consider potential fragility with reflection-based retrieval.

This newly introduced GetTestStore(this ILogger logger) method uses reflection to find _logger and its Loggers collection, then searches for an XunitTestLoggerProvider. While effective, reflection can be brittle if internal structures change. Verify whether this is the only feasible approach and consider caching or a more direct mechanism to avoid repeated reflection calls and improve performance.


63-67: Good backward compatibility approach.

Marking the generic GetTestStore<T> as obsolete while delegating to the new non-generic method gracefully preserves existing usage without breaking changes. This helps guide users to the preferred method signature.

src/Codebelt.Extensions.Xunit.Hosting/GenericHostTestFactory.cs (3)

18-22: Obsolete method gracefully redirects to new overload.

Adding the [Obsolete] attribute and calling the new method with a null fixture is a neat way to archive old usage while steering developers toward the updated overloads.


36-46: Consistent deprecation of CreateWithHostBuilderContext.

Similar to the first overload, marking this method as obsolete while internally deferring to the updated API helps maintain backward compatibility.


48-57: Enhanced flexibility in CreateWithHostBuilderContext.

The new overload accepting IHostFixture offers extensibility for users needing specialized fixture logic. This is consistent with the approach used in the standard Create method overload.

src/Codebelt.Extensions.Xunit.Hosting/GenericHostTest.cs (3)

7-7: Switch to interface type improves abstraction.

Changing the inheritance to HostTest<IHostFixture> decouples the class from a concrete HostFixture, allowing alternate fixture implementations.


14-19: Check for null fixture when calling InitializeHostFixture.

While you rely on hostFixture.HasValidState(), consider explicitly handling a null hostFixture to prevent potential null references in future refactors.

internal GenericHostTest(Action<IServiceCollection> serviceConfigurator, Action<IHostBuilder> hostConfigurator, IHostFixture hostFixture)
    : base(hostFixture, callerType: serviceConfigurator?.Target?.GetType() ?? hostConfigurator?.Target?.GetType())
{
+   if (hostFixture == null) throw new ArgumentNullException(nameof(hostFixture));
    _serviceConfigurator = serviceConfigurator;
    _hostConfigurator = hostConfigurator;
    InitializeHostFixture(hostFixture);
}

21-26: Parallel constructor handles context-based service configuration.

The second constructor is consistent with the first, ensuring the optional HostBuilderContext logic gets integrated similarly. No immediate issues spotted.

src/Codebelt.Extensions.Xunit.Hosting/XunitTestLogger.cs (3)

8-8: Sealed class with ILogger & IDisposable is acceptable.

Marking the logger as internal sealed prevents inheritance. This is often appropriate for specialized logging classes, unless there is a need for mocking or customization.


33-33: Confirm thread-safe behavior of _provider.WriteLoggerEntry.

Ensure _provider.WriteLoggerEntry is safe under concurrent calls to Log. If it manipulates shared state, additional synchronization might be necessary.


46-46: Exposed provider supports external test store retrieval.

Providing property-based access to the underlying XunitTestLoggerProvider is consistent with how LoggerExtensions.GetTestStore locates it. No further concerns here.

test/Codebelt.Extensions.Xunit.Hosting.AspNetCore.Tests/WebHostTestFactoryTest.cs (6)

1-25: No issues found in the newly introduced namespace and using directives.

Everything appears consistent with the rest of the codebase.


26-51: Test method verifies the blocking fixture behavior successfully.

This test correctly checks that a SecurityException is thrown with the BlockingAspNetCoreHostFixture.


53-81: Test method correctly captures the SecurityException logs.

The log-based assertion is appropriate, ensuring the exception is properly recorded without halting the host startup.


83-98: No issues found with the caller type assertion logic.

This test accurately verifies the declaring type for the middleware.


100-110: Application name assertion is straightforward and correct.

No issues found with this test’s logic for verifying the assembly name.


113-141: Host builder context test logic is verified properly.

The checks for non-null context properties are valid, and the assembly name assertion is correct.

src/Codebelt.Extensions.Xunit.Hosting.AspNetCore/WebHostTest.cs (4)

8-8: Good interface-based refactor.

Switching to IAspNetCoreHostFixture enhances flexibility and abstraction.


17-23: Constructor logic looks appropriate.

Calling the base constructor and then initializing the fixture is cohesive and readable.


25-31: Second constructor is consistent with the updated interface approach.

No issues found.


32-47: Centralized fixture initialization is a good practice.

The InitializeHostFixture method encapsulates repeated logic, improving maintainability.

src/Codebelt.Extensions.Xunit.Hosting.AspNetCore/WebHostTestFactory.cs (8)

24-29: Obsolete method retains backward compatibility.

Marking it as obsolete while delegating to the new overload is an appropriate migration strategy.


38-41: New overload adds flexibility for custom host fixtures.

Allowing an optional IAspNetCoreHostFixture parameter is a welcome enhancement.


43-54: Obsolete method with host builder context also properly delegates to the new overload.

Retains existing functionality while guiding consumers to the newer method.


64-67: New overload with host builder context looks good.

The logic to instantiate WebHostTest with the optional host fixture is clear.


77-81: Obsolete RunAsync method references the improved overload.

This maintains existing usage patterns without breaking compatibility.


92-96: The updated RunAsync overload supports custom fixtures gracefully.

No issues found with this approach.


106-110: Second obsolete method again defers to the new method.

Keeps older callers functional while prompting a transition.


120-124: Final new overload aligns with the host builder context approach.

All optional parameters and usage patterns appear consistent.

src/Codebelt.Extensions.Xunit.Hosting.AspNetCore/GlobalSuppressions.cs (1)

1-11: Good approach to maintain backward compatibility.

The suppressions are correctly implemented to address the "Method overloads with default parameter values should not overlap" warning for the WebHostTestFactory methods. This approach allows the introduction of the new IAspNetCoreHostFixture parameter while avoiding a major version bump, which aligns well with the PR objectives.

Consider documenting a plan to clean up these suppressions in a future major version update when backward compatibility can be broken.

@codecov
Copy link

codecov bot commented Mar 30, 2025

Codecov Report

Attention: Patch coverage is 71.79487% with 22 lines in your changes missing coverage. Please review.

Project coverage is 84.58%. Comparing base (4625540) to head (9795cff).

Files with missing lines Patch % Lines
...ons.Xunit.Hosting.AspNetCore/WebHostTestFactory.cs 56.25% 7 Missing ⚠️
...debelt.Extensions.Xunit.Hosting/GenericHostTest.cs 0.00% 6 Missing ⚠️
...Extensions.Xunit.Hosting/GenericHostTestFactory.cs 25.00% 6 Missing ⚠️
...Extensions.Xunit.Hosting.AspNetCore/WebHostTest.cs 66.66% 0 Missing and 2 partials ⚠️
...osting.AspNetCore/BlockingAspNetCoreHostFixture.cs 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #26      +/-   ##
==========================================
- Coverage   85.86%   84.58%   -1.29%     
==========================================
  Files          31       32       +1     
  Lines         672      668       -4     
  Branches       84       85       +1     
==========================================
- Hits          577      565      -12     
- Misses         92      100       +8     
  Partials        3        3              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sonarqubecloud
Copy link

@gimlichael gimlichael merged commit 950cf17 into main Mar 31, 2025
0 of 6 checks passed
@gimlichael gimlichael deleted the v9.1.0/fixture-enhancements branch March 31, 2025 19:24
@coderabbitai coderabbitai bot mentioned this pull request Apr 3, 2025
@coderabbitai coderabbitai bot mentioned this pull request Apr 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants