-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Improve enhanced navigation suppression in tests #63132
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
base: main
Are you sure you want to change the base?
Changes from all commits
66d2b0b
4b1f572
945c9dd
e9bf1bf
f9601f1
2cd3a18
5fd1050
7c0bf05
b2e8441
08c99cf
73abbda
5ef86a1
357192c
92cad9a
8c5c3b1
189db63
fa057a8
ee4aa34
795b665
a6856fc
249f03a
763ea9d
5696430
e1b6931
1a8d9bf
1339daa
21051c2
0bd2afc
7ac455c
18ee068
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
@page "/" | ||
|
||
<h1>Hello, world!</h1> | ||
<h1 id="session-storage-anchor">Hello, world!</h1> | ||
|
||
Welcome to your new app. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,8 +2,10 @@ | |
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
using Microsoft.AspNetCore.Components.E2ETest.Infrastructure.ServerFixtures; | ||
using Microsoft.AspNetCore.Components.E2ETests.ServerRenderingTests; | ||
using Microsoft.AspNetCore.E2ETesting; | ||
using OpenQA.Selenium; | ||
using OpenQA.Selenium.BiDi.Communication; | ||
using Xunit.Abstractions; | ||
|
||
namespace Microsoft.AspNetCore.Components.E2ETest.Infrastructure; | ||
|
@@ -13,28 +15,42 @@ public abstract class ServerTestBase<TServerFixture> | |
IClassFixture<TServerFixture> | ||
where TServerFixture : ServerFixture | ||
{ | ||
public string ServerPathBase => "/subdir"; | ||
private string _serverPathBase; | ||
public string ServerPathBase => _serverPathBase; | ||
|
||
protected readonly TServerFixture _serverFixture; | ||
|
||
public ServerTestBase( | ||
BrowserFixture browserFixture, | ||
TServerFixture serverFixture, | ||
ITestOutputHelper output) | ||
ITestOutputHelper output, | ||
string serverPathBase = "/subdir") | ||
: base(browserFixture, output) | ||
{ | ||
_serverFixture = serverFixture; | ||
_serverPathBase = serverPathBase; | ||
} | ||
|
||
public void Navigate(string relativeUrl) | ||
{ | ||
Browser.Navigate(_serverFixture.RootUri, relativeUrl); | ||
} | ||
|
||
public override async Task DisposeAsync() | ||
{ | ||
EnhancedNavigationTestUtil.CleanEnhancedNavigationSuppression(this); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Naive question: Is there a world where a test crashes and the enhanced navigation suppression flag is not cleaned up and that has a flaky effect on other tests? Or is that avoided by each test appending different IDs to the flag in the session storage? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That depends on the type of failure. Disposal method should run after each test, regardless of the result. In case the failure was connected with browser getting disconnected/crashed etc, then we loose the connection to the old instance, that is, the old session storage values are gone, we don't need to worry about them. The following tests should have a fresh instance created by selenium (with a clean storage). It's not really a matter of test id because we don't grant test id unless the test wants to suppress enhanced nav. So it's not true that each test has their ID, only suppressing tests have them. I was initially trying an approach of granting it for every test, in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the detailed explanation! |
||
await base.DisposeAsync(); | ||
} | ||
|
||
protected override void InitializeAsyncCore() | ||
{ | ||
// Clear logs - we check these during tests in some cases. | ||
// Make sure each test starts clean. | ||
((IJavaScriptExecutor)Browser).ExecuteScript("console.clear()"); | ||
} | ||
|
||
protected override void GrantTestId() | ||
{ | ||
EnhancedNavigationTestUtil.GrantTestId(this); | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.