Skip to content

Fix tests that use AppContextSwitch#67492

Draft
dariatiurina wants to merge 2 commits into
dotnet:mainfrom
dariatiurina:fix-navigation-tests
Draft

Fix tests that use AppContextSwitch#67492
dariatiurina wants to merge 2 commits into
dotnet:mainfrom
dariatiurina:fix-navigation-tests

Conversation

@dariatiurina

@dariatiurina dariatiurina commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Fix tests that use AppContextSwitch

Summary

Several Components E2E tests toggled feature switches at test time with AppContext.SetSwitch(...), but those switch values are read once into static fields at static initialization. Because the E2E suite hosts servers in-process and runs serially, the static field is already initialized by the time the test calls SetSwitch, so the call has no effect and the test exercises the wrong code path. This PR sets the cached static fields directly via reflection instead.

Changes

  • HttpNavigationManager and HotReloadManager: changed the cached switch fields (s_throwNavigationException and s_isSupported) from static readonly to static (with an IDE0044 pragma). This is required so reflection writes are reliably observed — a static readonly primitive can be const-folded by the JIT, which would make a reflection write invisible.
  • RedirectionTest: replaced every AppContext.SetSwitch("...DisableThrowNavigationException", ...) call with a SetDisableThrowNavigationException(...) helper that sets HttpNavigationManager.s_throwNavigationException via reflection. Added supporting helpers GetThrowNavigationExceptionField() and GetCachedThrowNavigationException().
  • HotReloadStartup: in addition to the existing AppContext.SetSwitch(...IsSupported, true), force HotReloadManager.s_isSupported to true via reflection so hot reload is active for this server regardless of when the field was first initialized.
  • RazorComponentEndpointsStartup: reset HotReloadManager.s_isSupported back to the runtime default (MetadataUpdater.IsSupported) via reflection, so the value forced true by the hot-reload server does not leak into other in-process servers that expect the default.

Details

  • The reflection targets the Microsoft.AspNetCore.Components core copy of HotReloadManager (typeof(HotReloadManager) resolves there because only core exposes it to Components.TestServer via InternalsVisibleTo).
  • All reflection writes are documented as safe only because the E2E suite runs serially; enabling parallelization would let servers/tests needing opposite values race on these process-global fields.
  • MetadataUpdater.IsSupported is used as the "default" for the hot-reload reset because the runtime caches it once and it is unaffected by the later AppContext.SetSwitch call, making it a stable reference for the process default.

Testing

  • Reuses the existing Components E2E tests: RedirectionTest (server-side redirection across streaming/enhanced/non-Blazor scenarios with disableThrowNavigationException true/false) and HotReloadTest.
  • No new test files are added; the change is to how the existing tests set the switches.

Breaking Changes

None. The only production change is removing readonly from two internal static fields; the observable behavior of IsSupported / navigation-exception throwing is unchanged. The remaining changes are test-only.

Fixes #67491

@dariatiurina dariatiurina self-assigned this Jun 30, 2026
@dariatiurina dariatiurina added this to the 11.0-preview7 milestone Jun 30, 2026
private const string _disableThrowNavigationException = "Microsoft.AspNetCore.Components.Endpoints.NavigationManager.DisableThrowNavigationException";

private static readonly bool s_throwNavigationException =
#pragma warning disable IDE0044 // Add readonly modifier - mutated via reflection by E2E tests; keeping it non-readonly avoids JIT const-folding making those writes invisible.

@Youssef1313 Youssef1313 Jun 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not necessary to address it now, but consider if it's a better idea to have some internal abstraction for this that the tests can override.

internal interface IAppContext
{
    bool TryGetSwitch(string switchName, out bool isEnabled);
}

internal static class AppContextSingleton
{
    public static IAppContext Instance { get; set; } = SystemAppContext.Instance;
}

internal sealed class SystemAppContext : IAppContext
{
    public static SystemAppContext Instance { get; } = new();

    private SystemAppContext()
    {
    }

    public bool TryGetSwitch(string switchName, out bool isEnabled)
        => AppContext.TryGetSwitch(switchName, out isEnabled);
}

Then, all calls to AppContext.TryGetSwitch in product code change to AppContextSingleton.Instance.TryGetSwitch.

And, in test assemblies, add a module initializer that replaces AppContextSingleton.Instance with a test mock that uses async local for retrieving the values so that while it will be a singleton used by all tests, you can still achieve isolation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants