Skip to content

perf: ensure the pausing event listeners are removed #31596

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

Merged
merged 9 commits into from
Apr 29, 2025

Conversation

mschile
Copy link
Contributor

@mschile mschile commented Apr 28, 2025

  • Closes n/a

Additional details

In open mode, when a new spec is run or the existing spec is rerun, we aren't clearing the reporter listeners in pausing.ts. Updated handlePausing() to ensure the previous event listeners are removed before new ones are added.

Steps to test

In open mode, rerun a spec more than 10 times and verify the event emitters memory leak message is not received:

index-BJMNOmDZ.js:80727 MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 runner:next listeners added. Use emitter.setMaxListeners() to increase limit
    at _addListener (https://example.cypress.io/__cypress/assets/assets/index-BJMNOmDZ.js:80848:18)
    at EventEmitter.addListener (https://example.cypress.io/__cypress/assets/assets/index-BJMNOmDZ.js:80859:12)
    at handlePausing (https://example.cypress.io/__cypress/assets/assets/index-BJMNOmDZ.js:81077:15)
    at EventManager._addListeners (https://example.cypress.io/__cypress/assets/assets/index-BJMNOmDZ.js:86525:5)
    at EventManager.setup (https://example.cypress.io/__cypress/assets/assets/index-BJMNOmDZ.js:86359:10)
    at Object.executeSpec (https://example.cypress.io/__cypress/assets/assets/index-BJMNOmDZ.js:86909:27)
    at async runSpec (https://example.cypress.io/__cypress/assets/assets/Runner-B30ckd2e.js:7914:5)
    at async EventEmitter.<anonymous> (https://example.cypress.io/__cypress/assets/assets/Runner-B30ckd2e.js:7920:9)

How has the user experience changed?

Before (test fails showing we have two listeners attached when there should've only been one):
Screenshot 2025-04-28 at 9 41 46 AM

After (test passes showing the correct amount of listeners attached):
Screenshot 2025-04-28 at 9 51 30 AM

PR Tasks

@mschile mschile self-assigned this Apr 28, 2025
Copy link

cypress bot commented Apr 28, 2025

cypress    Run #62044

Run Properties:  status check passed Passed #62044  •  git commit 8e82264aa0: Merge branch 'develop' into mschile/handle_pausing
Project cypress
Branch Review mschile/handle_pausing
Run status status check passed Passed #62044
Run duration 19m 06s
Commit git commit 8e82264aa0: Merge branch 'develop' into mschile/handle_pausing
Committer Matt Schile
View all properties for this run ↗︎

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

@mschile mschile merged commit 9f86cec into develop Apr 29, 2025
89 of 91 checks passed
@mschile mschile deleted the mschile/handle_pausing branch April 29, 2025 17:29
@cypress-bot
Copy link
Contributor

cypress-bot bot commented May 6, 2025

Released in 14.3.3.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v14.3.3, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators May 6, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants