Fix tests that use AppContextSwitch#67492
Draft
dariatiurina wants to merge 2 commits into
Draft
Conversation
Youssef1313
reviewed
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. |
Member
There was a problem hiding this comment.
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.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
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 callsSetSwitch, 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
HttpNavigationManagerandHotReloadManager: changed the cached switch fields (s_throwNavigationExceptionands_isSupported) fromstatic readonlytostatic(with anIDE0044pragma). This is required so reflection writes are reliably observed — astatic readonlyprimitive can be const-folded by the JIT, which would make a reflection write invisible.RedirectionTest: replaced everyAppContext.SetSwitch("...DisableThrowNavigationException", ...)call with aSetDisableThrowNavigationException(...)helper that setsHttpNavigationManager.s_throwNavigationExceptionvia reflection. Added supporting helpersGetThrowNavigationExceptionField()andGetCachedThrowNavigationException().HotReloadStartup: in addition to the existingAppContext.SetSwitch(...IsSupported, true), forceHotReloadManager.s_isSupportedtotruevia reflection so hot reload is active for this server regardless of when the field was first initialized.RazorComponentEndpointsStartup: resetHotReloadManager.s_isSupportedback to the runtime default (MetadataUpdater.IsSupported) via reflection, so the value forcedtrueby the hot-reload server does not leak into other in-process servers that expect the default.Details
Microsoft.AspNetCore.Componentscore copy ofHotReloadManager(typeof(HotReloadManager)resolves there because only core exposes it toComponents.TestServerviaInternalsVisibleTo).MetadataUpdater.IsSupportedis used as the "default" for the hot-reload reset because the runtime caches it once and it is unaffected by the laterAppContext.SetSwitchcall, making it a stable reference for the process default.Testing
RedirectionTest(server-side redirection across streaming/enhanced/non-Blazor scenarios withdisableThrowNavigationExceptiontrue/false) andHotReloadTest.Breaking Changes
None. The only production change is removing
readonlyfrom two internal static fields; the observable behavior ofIsSupported/ navigation-exception throwing is unchanged. The remaining changes are test-only.Fixes #67491