Skip to content
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

fix: prevent navigating to about:blank #30864

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

mschile
Copy link
Contributor

@mschile mschile commented Jan 11, 2025

Additional details

If test isolation is disabled and the last test fails and then retries, we were incorrectly navigating to about:blank due to an incorrect calculation of isLastTestThatWillRunInSuite that was not accounting for retries. In order to resolve this issue, we are now accounting for retries by checking test.final.

Steps to test

Run a spec in run mode with test isolation off where the last test in the spec fails and then retries and verify we do not navigate to about:blank in-between the retries.

How has the user experience changed?

Before:
Screenshot 2025-01-10 at 5 47 50 PM

After:
Screenshot 2025-01-10 at 5 46 53 PM

PR Tasks

Copy link

cypress bot commented Jan 11, 2025

cypress    Run #59863

Run Properties:  status check passed Passed #59863  •  git commit 3609cc19d8: fix changelog
Project cypress
Branch Review mschile/about_blank_with_retry
Run status status check passed Passed #59863
Run duration 17m 42s
Commit git commit 3609cc19d8: fix changelog
Committer Matthew Schile
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 8
Tests that did not run due to a developer annotating a test with .skip  Pending 1090
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 26423
View all changes introduced in this branch ↗︎
UI Coverage  45.56%
  Untested elements 191  
  Tested elements 164  
Accessibility  92.54%
  Failed rules  3 critical   8 serious   2 moderate   2 minor
  Failed elements 888  


// there can only be one test in this file to ensure we are testing the scenario
// where a test fails and the runner does not navigate to about:blank between retries
it('test 1', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we give this a more descriptive name to what we are testing?

if (test.failedFromHookId && (test.hookName === 'before all')) {
return null
}

// if this test hasn't been finalized, then we will be retry'ing it so just return this test
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// if this test hasn't been finalized, then we will be retry'ing it so just return this test
// if this test hasn't been finalized, then we will be retrying it so just return this test

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.

Default blank page, this page was cleared by navigating to about:blank
3 participants