feat: Close and reopen a new tab in between tests to get around a memory leak by ryanthemanuel · Pull Request #19915 · cypress-io/cypress · GitHub

ryanthemanuel · 2022-01-26T15:20:07Z

Closes UNIFY-946

User facing changelog

Additional details

With this work, we will be changing how run mode works with respect to transitioning from spec to spec. As @brian-mann stated in the comments today we:

open browser to about:blank with fresh user profile + data dir folder
bind CDP to the browser / tab
start screencast
navigate tab to URL
browser yields screencast frames
write to ffmpeg
...spec finishes...
close the browser
stop screencast frames
process video
...close browser and repeat steps for next spec...

We will be changing this so that on step 11 we:

Stopping the video
Clear the browser state
Close the browser tab
Clear the server state
Open a new tab directly to 'about:blank'
Rebind CDP to the fresh tab
Navigate to the new url
Start the video

A couple of important points to note:

before:browser:launch will now only fire once per overall run rather than once per spec file for chrome and firefox. This is due to the fact that the browser is only launched once at the beginning of the run and then subsequent specs close existing tabs and open new ones
Irreconcilable CDP disconnects will now be treated as fatal errors since we can't use CDP to continue on with subsequent specs (previously we could at least attempt to launch a new browser)
Firefox will have a slightly different functional behavior than chrome. Chrome will close browser tabs in between spec files; however, firefox will close browser tabs, but closing a tab will cause a blank tab to immediately reopen. This is necessary in order to keep the necessary debugging connections available.

How has the user experience changed?

PR Tasks

Have tests been added/updated?
[na] Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
[na] Has a PR for user-facing changes been opened in cypress-documentation?
[na] Have API changes been updated in the type definitions?
[na] Have new configuration options been added to the cypress.schema.json?

cypress-bot · 2022-01-26T15:20:09Z

Thanks for taking the time to open a PR!

Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

brian-mann · 2022-01-26T15:25:41Z

Background

Today we currently close the browser in between specs for e2e - which adds a considerable amount of time to the overall run. The desired functionality for CT is to not close the browser and navigate to the new spec URL. Unfortunately, there is currently a memory leak in Unification, and because we use hash based routing - navigating to the new spec URL does not cause a full page refresh, thus building up memory until its fully exhausted.

While I expect us to eventually fix the memory leak, we can take a "middle-of-the-road" approach here and utilize this improvement for both e2e + ct, which would be much more consistent. The "middle-of-the-road" approach avoids needing to close the browser in between specs, and we could simply "clear" or "close" the primary tab to ensure that specs start clean.

Cypress Today (first spec - with video)

opens browser to about:blank with fresh user profile + data dir folder
binds CDP to the browser / tab
starts screencast
navigates tab to URL
browser yields screencast frames
writes to ffmpeg
...spec finishes...
closes the browser
stop screencast frames
process video
...repeat steps for next spec...

Cypress Today (first spec - without video)

opens browser to about:blank
binds CDP to the browser / tab
navigates tab to URL
browser yields screencast frames
...spec finishes...
closes the browser
...repeat steps for next spec...

Option 1: Closing the primary tab

Consider the experience and technical ramifications of this approach. This may not be a great user experience depending on how long these operations take.

stopping the video
closing the primary tab
clearing the browser state
clearing the server state (AUT target domain)
opening a fresh tab about:blank
rebinding to the fresh tab
starting the video
then navigating the tab to the next spec URL

Option 2: Navigating the primary tab

We'd need to have a design for /__cypress/spec-reset

stopping the video
navigating the primary tab to a special /__cypress/spec-reset URL
clearing the browser state
clearing the server state (AUT target domain)
starting the video
then navigating the primary tab to the next spec URL

cypress · 2022-01-26T15:36:54Z

Test summary

17929 • 0 • 211 • 0 • 2

Run details


Project	cypress
Status	Passed
Commit	`449dda7`
Started	Feb 8, 2022 2:39 PM
Ended	Feb 8, 2022 2:51 PM
Duration	12:34 💡
OS	Linux Debian - 10.10
Browser	Multiple

View run in Cypress Dashboard ➡️

Flakiness

	commands/xhr.cy.js	1
1	... > no status when request isnt forced 404
	cypress/proxy-logging.cy.ts	1
1	Proxy Logging > request logging > xhr log has response body/status code

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

ryanthemanuel · 2022-01-28T16:54:52Z

The ordering that has seemed to work best so far for Option 1 (no intermediate url) is:

Stopping the video
Clearing the browser state
Clearing the server state
Opening a new tab directly to the new test URL
Rebind CDP to the fresh tab
Start the video
Close the old tab

…sed first

emilyrohrbough · 2022-02-07T15:02:38Z

      originalBrowserKill.apply(launchedBrowser, args)
    }

+    const criClient = await browserCriClient.attachToTargetUrl('about:blank')


It's not clear to me from the names what the difference is between browserCriClient and criClient.

I have updated the names to pageCriClient to try and highlight the distinction: ab49d53

emilyrohrbough · 2022-02-08T15:37:13Z

+      this._handleDownloads(pageCriClient, options.downloadsFolder, automation),
+    ])


just curious, why move the navigate & download order?

Both of these can be done in parallel and aren't dependent upon the navigation to occur. It's a slight performance boost.

Temporary prototype to try and close tab and reopen

5c59e99

emilyrohrbough reviewed Jan 26, 2022

View reviewed changes

Comment thread packages/server/lib/browsers/protocol.ts Outdated

emilyrohrbough reviewed Jan 26, 2022

View reviewed changes

Comment thread packages/server/lib/open_project.ts Outdated

ryanthemanuel added 6 commits January 26, 2022 12:08

Clean up port logic

39848f5

Improve upon prototype

246913a

Refactoring clean up further prototyping

416cb40

Electron fun

9159f00

Add firefox support

5f4daca

Temporary electron fix

1058cdf

ryanthemanuel added 8 commits January 28, 2022 13:53

Fix ordering

1b95c07

Update order of events when running tests. Ensure that the tab is clo…

d3b1c59

…sed first

Undo system test fix

04594c1

Fix electron

4271b14

Fix sessions issue

c63f572

Fix system tests

78590b2

Fix lint

445e758

Merge 10.0-release

57047ce

ryanthemanuel marked this pull request as ready for review January 31, 2022 22:17

ryanthemanuel marked this pull request as draft January 31, 2022 22:17

ryanthemanuel added 3 commits February 1, 2022 14:35

Fix system test

c6dd762

Code clean up and a few tests

aca3a22

Clean up code

e98cfd6

ryanthemanuel commented Feb 2, 2022

View reviewed changes

Comment thread system-tests/__snapshots__/deprecated_spec.ts.js

ryanthemanuel commented Feb 2, 2022

View reviewed changes

Comment thread system-tests/__snapshots__/cdp_spec.ts.js

ryanthemanuel added 2 commits February 2, 2022 17:02

Fix timing issue with chrome

a17b64b

Update system tests for electron

073818e

emilyrohrbough reviewed Feb 7, 2022

View reviewed changes

ryanthemanuel added 5 commits February 7, 2022 09:42

PR updates

f3e5a74

PR comments

ab49d53

Code review comments

142ad0e

Additional refactoring

6c2f7fb

More PR comments

1efb6ce

ryanthemanuel changed the title ~~feat: Close and reopen a new tab in between tests to get around a memory leak~~ feat: Close and reopen a new tab in between tests to get around a memory leak. Feb 7, 2022

ryanthemanuel changed the title ~~feat: Close and reopen a new tab in between tests to get around a memory leak.~~ feat: Close and reopen a new tab in between tests to get around a memory leak Feb 7, 2022

ryanthemanuel added 2 commits February 7, 2022 16:46

Additional performance improvements around start up and shut down

196af3c

Additional performance improvements around start up and shut down

398db08

flotwig suggested changes Feb 7, 2022

View reviewed changes

Comment thread packages/extension/test/integration/background_spec.js Outdated

Comment thread system-tests/test/deprecated_spec.ts Outdated

Comment thread system-tests/test/browser_reset_spec.js

PR comments. Simplify system tests

cc71a67

ryanthemanuel requested a review from flotwig February 7, 2022 23:37

flotwig previously approved these changes Feb 7, 2022

View reviewed changes

Attempt to parallelize a couple more calls in chrome

8f302b1

ryanthemanuel dismissed flotwig’s stale review via 8f302b1 February 8, 2022 01:24

ryanthemanuel added 2 commits February 7, 2022 19:54

Attempt to parallelize a couple more calls in chrome

3ad48a9

Undo inadvertent commit

449dda7

ryanthemanuel changed the title ~~feat: Close and reopen a new tab in between tests to get around a memory leak~~ feat: Close and reopen a new tab in between tests to get around a memory leak. Feb 8, 2022

ryanthemanuel changed the title ~~feat: Close and reopen a new tab in between tests to get around a memory leak.~~ feat: Close and reopen a new tab in between tests to get around a memory leak Feb 8, 2022

emilyrohrbough reviewed Feb 8, 2022

View reviewed changes

Comment thread packages/server/lib/browsers/chrome.ts

emilyrohrbough reviewed Feb 8, 2022

View reviewed changes

emilyrohrbough approved these changes Feb 8, 2022

View reviewed changes

flotwig changed the title ~~feat: Close and reopen a new tab in between tests to get around a memory leak~~ feat: Close and reopen a new tab in between tests to get around a memory leakx Feb 8, 2022

flotwig changed the title ~~feat: Close and reopen a new tab in between tests to get around a memory leakx~~ feat: Close and reopen a new tab in between tests to get around a memory leak Feb 8, 2022

flotwig approved these changes Feb 8, 2022

View reviewed changes

ryanthemanuel merged commit 6550903 into 10.0-release Feb 8, 2022

ryanthemanuel deleted the ryanm/feat/memory-leak branch February 8, 2022 16:16

ryanthemanuel mentioned this pull request Feb 10, 2022

fix: reorder electron launching sequence to eliminate timing issue #20140

Merged

1 task

DJSdev mentioned this pull request Jun 3, 2022

Tests causing CI agents to run out of memory on cypress 8.2 & above #17853

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Close and reopen a new tab in between tests to get around a memory leak#19915

feat: Close and reopen a new tab in between tests to get around a memory leak#19915
ryanthemanuel merged 76 commits into
10.0-releasefrom
ryanm/feat/memory-leak

ryanthemanuel commented Jan 26, 2022 •

edited

Loading

Uh oh!

cypress-bot Bot commented Jan 26, 2022

Uh oh!

brian-mann commented Jan 26, 2022 •

edited

Loading

Uh oh!

cypress Bot commented Jan 26, 2022 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

ryanthemanuel commented Jan 28, 2022

Uh oh!

Uh oh!

Uh oh!

emilyrohrbough Feb 7, 2022

Uh oh!

ryanthemanuel Feb 7, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

emilyrohrbough Feb 8, 2022

Uh oh!

ryanthemanuel Feb 8, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		this._handleDownloads(pageCriClient, options.downloadsFolder, automation),
		])

Conversation

ryanthemanuel commented Jan 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User facing changelog

Additional details

How has the user experience changed?

PR Tasks

Uh oh!

cypress-bot Bot commented Jan 26, 2022

Uh oh!

brian-mann commented Jan 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Background

Cypress Today (first spec - with video)

Cypress Today (first spec - without video)

Option 1: Closing the primary tab

Option 2: Navigating the primary tab

Uh oh!

cypress Bot commented Jan 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test summary

Run details

Flakiness

Uh oh!

Uh oh!

Uh oh!

ryanthemanuel commented Jan 28, 2022

Uh oh!

Uh oh!

Uh oh!

emilyrohrbough Feb 7, 2022

Choose a reason for hiding this comment

Uh oh!

ryanthemanuel Feb 7, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

emilyrohrbough Feb 8, 2022

Choose a reason for hiding this comment

Uh oh!

ryanthemanuel Feb 8, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ryanthemanuel commented Jan 26, 2022 •

edited

Loading

brian-mann commented Jan 26, 2022 •

edited

Loading

cypress Bot commented Jan 26, 2022 •

edited

Loading