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.

Sign up for GitHub

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

Jump to bottom

fix: regression where failed events could cause a passing test to display as failed #15037

Merged
merged 5 commits into from Feb 15, 2021
Merged

fix: regression where failed events could cause a passing test to display as failed #15037

merged 5 commits into from Feb 15, 2021

Conversation

panzarino
Copy link
Contributor

@panzarino panzarino commented Feb 11, 2021 •
edited

User facing changelog

Fixes an issue where tests would display as failed if an event (such as an XHR request) failed after the test was already completed. This only affects the display of the test and not what was reported in run mode, so it would not have had an effect on suites running in CI.

Additional details

In addition to the changes described above, there are a number of additional changes that ensure the correct error state is still shown when a user-inputted URL causes a failed cy.visit() in studio mode. This was the intent of the original code that caused the regression, and tests have been added for both this intended functionality and to ensure this regression isn't duplicated again in the future. I also added a couple of random extra studio ui tests for good measure

How has the user experience changed?

With the following test code

it('regression', () => {
  cy.visit('failing_xhr.html')
  cy.get('button').click()
})
<body>
  <button onclick="sendxhr()">my button</button>
</body>
<script>
  function sendxhr() {
    var xhr = new XMLHttpRequest()
    xhr.open('GET', 'http://yupyupyupyup.net', true)

    xhr.send(null)

    setTimeout(function () {
      xhr.abort()
    }, 1000)
  }
</script>

Before:
Screen Shot 2021-02-11 at 12 53 52 AM

After:
Screen Shot 2021-02-11 at 12 54 06 AM

PR Tasks

  • Have tests been added/updated?
  • Has the original issue or this PR been tagged with a release in ZenHub?

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Feb 11, 2021

Thanks for taking the time to open a PR!

@panzarino panzarino marked this pull request as ready for review February 11, 2021 07:35
@cypress
Copy link

cypress bot commented Feb 11, 2021 •
edited


Test summary

9323 • 0 • 118 • 3 • Flakiness 1

Run details
Project cypress
Status Passed
Commit 90024e6
Started Feb 15, 2021 3:39 PM
Ended Feb 15, 2021 3:50 PM
Duration 10:35 💡
OS Linux Debian - 10.5
Browser Multiple

View run in Cypress Dashboard ➡️

Flakiness

cypress/integration/retries.ui.spec.js 1 Flakiness
1 runner/cypress retries.ui.spec > opens attempt on each attempt failure for the screenshot, and closes after test passes

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

bahmutov
bahmutov approved these changes Feb 15, 2021
View reviewed changes
Copy link
Contributor

@bahmutov bahmutov left a comment

Choose a reason for hiding this comment

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

Perfect, the problem is gone from the reproducible repo I provided, looks nice

@bahmutov bahmutov merged commit b4c0117 into develop Feb 15, 2021
@cypress-bot cypress-bot bot mentioned this pull request Feb 15, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bahmutov bahmutov bahmutov approved these changes

@jennifer-shehane jennifer-shehane Awaiting requested review from jennifer-shehane

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

If the last record in the Command Log is a canceled or aborted XHR the test shows failure
2 participants
@panzarino @bahmutov