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

Snapshotting still occurs when numTestsKeptInMemory is 0 #4104

Closed
CoryDanielson opened this issue May 2, 2019 · 6 comments · Fixed by #4406
Closed

Snapshotting still occurs when numTestsKeptInMemory is 0 #4104

CoryDanielson opened this issue May 2, 2019 · 6 comments · Fixed by #4406

Comments

@CoryDanielson
Copy link
Contributor

CoryDanielson commented May 2, 2019

Current behavior:

When running cypress in GUI mode with numTestsKeptInMemory set to 0, snapshotting still occurs even though none of them should be kept in memory. This has a negative impact on performance and memory consumption.

In this screenshot, you can see that numTestsKeptInMemory is 0, but 5 seconds is spent taking snapshots that will not be used.

Screen Shot 2019-05-03 at 12 35 28 PM

In this screenshot you can see where the majority of the snapshot execution took place, 3.4s converting rules from large stylesheets into a string, and 0.4s replacing URLs in those strings with absolute paths.

Screen Shot 2019-05-03 at 12 40 53 PM

Desired behavior:

When running with numTestsKeptInMemory set to 0, snapshotting should be avoided in the same way that occurs while running in headless mode.

Steps to reproduce:

  1. Set numTestsKeptInMemory to 0.
  2. Run cypress GUI mode.

Versions

Cypress: 3.2.0
Chrome: Chrome 74

@jennifer-shehane
Copy link
Member

Hey @CoryDanielson, can you take a look at your resolved configuration and confirm that the numTestsKeptInMemory configuration is being read as set to 0? https://on.cypress.io/configuration#Resolved-Configuration

@jennifer-shehane jennifer-shehane added the stage: needs information Not enough info to reproduce the issue label May 3, 2019
@CoryDanielson
Copy link
Contributor Author

@jennifer-shehane I updated the original post with new screenshots.

@CoryDanielson
Copy link
Contributor Author

CoryDanielson commented May 3, 2019

I opened up a PR to address this #4123

I extended the if statement to include a check for numTestsKeptInMemory

    snapshot: (name, options = {}) ->
      ## bail early and dont snapshot
      ## if we're in headless mode
      ## TODO: fix this
      if not config("isInteractive") || _.toString(config("numTestsKeptInMemory")) is '0'
        return @

@cypress-bot cypress-bot bot added stage: needs review The PR code is done & tested, needs review and removed stage: work in progress labels May 6, 2019
@brian-mann
Copy link
Member

I'll take this PR from here and we'll get it out in the next patch release or so.

@cypress-bot cypress-bot bot added stage: work in progress stage: ready for work The issue is reproducible and in scope stage: needs review The PR code is done & tested, needs review and removed stage: needs review The PR code is done & tested, needs review stage: work in progress stage: ready for work The issue is reproducible and in scope labels Jun 6, 2019
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jun 17, 2019

The code for this is done in cypress-io/cypress#4406, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

@cypress-bot cypress-bot bot added stage: pending release and removed stage: needs review The PR code is done & tested, needs review labels Jun 17, 2019
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jun 27, 2019

Released in 3.3.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants