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

feat: Add 'slowTestThreshold' and fix this.slow() inside specs #18371

Merged
merged 10 commits into from
Oct 14, 2021

Conversation

BlueWinds
Copy link
Contributor

@BlueWinds BlueWinds commented Oct 5, 2021

User facing changelog

  • Adds slowTestThreshold config option to customize slow test threshold
  • BREAKING: Changes the default slow test threshold from 75ms (mocha's default) to 10000ms. In the default spec reporter, this will hide test timing for any test under 5000ms. To restore the old behavior, you can add "slowTestThreshold": 75 to your cypress configuration.

Additional details

A test that executes for half of the “slowTestThreshows” time will be highlighted in yellow with the default spec reporter; a test that executes for entire “slow” time will be highlighted in red. This is a visual change only - slow tests still pass. See https://mochajs.org/#-slow-ms-s-ms for more details.

PR Tasks

@BlueWinds BlueWinds requested a review from a team as a code owner October 5, 2021 21:56
@BlueWinds BlueWinds requested review from flotwig and chrisbreiding and removed request for a team October 5, 2021 21:56
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Oct 5, 2021

Thanks for taking the time to open a PR!

@BlueWinds BlueWinds changed the title Add 'slow' option to configure slow test threshold draft: Add 'slow' option to configure slow test threshold Oct 5, 2021
@BlueWinds BlueWinds changed the title draft: Add 'slow' option to configure slow test threshold feat: Add 'slow' option to configure slow test threshold Oct 5, 2021
@cypress
Copy link

cypress bot commented Oct 5, 2021



Test summary

4204 0 50 2Flakiness 2


Run details

Project cypress
Status Passed
Commit 30bd7ae
Started Oct 14, 2021 5:16 PM
Ended Oct 14, 2021 5:25 PM
Duration 09:40 💡
OS Linux Debian - 10.9
Browser Chrome beta 95

View run in Cypress Dashboard ➡️


Flakiness

cypress/integration/cypress/proxy-logging-spec.ts Flakiness
1 Proxy Logging > request logging > xhr log has response body/status code
2 ... > works with forceNetworkError

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

@BlueWinds BlueWinds force-pushed the issue-447-configure-slow-test-threshold branch from be506fb to 203072f Compare October 6, 2021 17:35
…shold inside tests, hooks and suites."

This reverts commit 1496833.
@BlueWinds
Copy link
Contributor Author

After struggling with it for a bit, I've eventually decided not to throw errors if the user uses this.slow() - Mocha calls it internally in several circumstances, and distinguishing them from user-written calls grew too complicated to warrant the effort over simply leaving it a no-op.

cli/schema/cypress.schema.json Outdated Show resolved Hide resolved
cli/types/cypress-npm-api.d.ts Outdated Show resolved Hide resolved
cli/types/cypress.d.ts Outdated Show resolved Hide resolved
cli/schema/cypress.schema.json Outdated Show resolved Hide resolved
@BlueWinds BlueWinds changed the title feat: Add 'slow' option to configure slow test threshold feat (breaking): Add 'slow' option to configure slow test threshold Oct 7, 2021
@BlueWinds BlueWinds changed the base branch from develop to 9.0-release October 7, 2021 16:30
@BlueWinds BlueWinds changed the title feat (breaking): Add 'slow' option to configure slow test threshold feat: Add 'slow' option to configure slow test threshold Oct 7, 2021
Copy link
Contributor

@flotwig flotwig left a comment

Choose a reason for hiding this comment

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

Nice 👍

@BlueWinds
Copy link
Contributor Author

Latest commit fixes this.slow() inside tests, and greatly simplifies how slowTestThreshold is handled. It also changes the default spec reporter to always show test timings, even for fast tests.

Still TODO:

  • Add tests around this functionality
  • Move slowTestThreshold to be e2e/component specific rather than a global value

@BlueWinds BlueWinds changed the title feat: Add 'slow' option to configure slow test threshold feat: Add 'slowTestThreshold' and fix this.slow() inside specs Oct 13, 2021
@BlueWinds BlueWinds force-pushed the issue-447-configure-slow-test-threshold branch from 037edc3 to a2784af Compare October 13, 2021 20:10
Copy link
Contributor

@chrisbreiding chrisbreiding left a comment

Choose a reason for hiding this comment

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

Other than the nitpicks about yellow vs orange, looks great.

packages/server/test/e2e/0_reporters_spec.js Outdated Show resolved Hide resolved
packages/server/test/e2e/0_reporters_spec.js Outdated Show resolved Hide resolved
packages/server/test/e2e/0_reporters_spec.js Outdated Show resolved Hide resolved
Co-authored-by: Chris Breiding <chrisbreiding@users.noreply.github.com>
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.

Cypress should set a mocha slow threshold for more reasonable reporting
3 participants