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

Do not crash if onBeforeLog returns false #958

Closed

Conversation

derfred
Copy link

@derfred derfred commented Nov 24, 2017

I am running into out-of-memory crashes as described in #882 when using the interactive UI. My tests run fine in headless mode.

These crashes are making it impossible for me to use the interactive UI for developing my tests. However we writing new tests, I mostly care about the last few log entries anyway. Therefore my idea was to suppress the logging of earlier commands using onBeforeLog with something like the following:

before(() => {
  Cypress.state('onBeforeLog', function () {
    return Cypress._logGate;
  })
});

it('runs a long test', function () {
  cy.then(function () {
    Cypress._logGate = false;
  })
  ... long list of commands ...
  cy.then(function () {
    Cypress._logGate = true;
  })
  cy.contains("Text I really care about").click();
});

This approach fails because the log function does not return a log object in case onBeforeLog returns false.

@CLAassistant
Copy link

CLAassistant commented Nov 24, 2017

CLA assistant check
All committers have signed the CLA.

@brian-mann
Copy link
Member

brian-mann commented Nov 24, 2017

The UI is not designed for running multiple tests.

This issues goes into all the detail about the problem: #681

You can simply set numTestsKeptInMemory: 0 in cypress.json if you don't want Cypress to track the commands.

The thing is though - just use the UI for running a test or a tiny group of tests. It's for writing tests not running tests. Use the CLI to run all of your tests.

@derfred
Copy link
Author

derfred commented Nov 24, 2017

I am encountering this crash when writing/running a single test. After about 200 commands the UI consistently crashes at the same step.

And I do want to keep track of the commands in the log. Ideally I could keep the whole history, but as a workaround I implemented the log suppression for the early parts.

@brian-mann
Copy link
Member

The better fix for this is that when numTestsKeptInMemory is 0 we don't store any snapshots - even for the current test. That would solve your issue and the Command Log would still log as normal.

@matt-psaltis
Copy link

I'd love to see the patch for this get merged in, whilst snapshots account for the majority of memory resource usage, during long test runs, the logs account for several hundred megabytes of memory. This change would also assist with reducing the noise in memory snapshots for application memory leak analysis.

@brian-mann
Copy link
Member

@SaltyDH by chance - since it sounds like you're doing some advanced things... have you read the comments on these other issues?

The reason I point those out is because if you're attempting to do memory analysis you'll need to better understand the internals of how Cypress works. The logs account for several hundred megabytes of memory because those are the things that have references to all of the values the commands return. When they return DOM - tada, it's the same thing as snapshots. Commands that don't return DOM won't be nearly as large, but they will still hold all object references in memory.

The solution is to not to do memory analysis via cypress open and instead opt for cypress run. However, I can see that that would be pretty annoying if you're attempting to iterate on a test - so maybe if you could explain your use case and process a bit better we could come up with a new API on Cypress to turn off the bits that are for debugging, so there are no preserved references kept in memory.

@matt-psaltis
Copy link

matt-psaltis commented Mar 13, 2018

Thanks @brian-mann (sorry for the novel below). I had come across the first of those other issues. Its been an interesting read getting up to speed with the mindset and philosophies that drive the Cypress development. Most nights last week were spent reading through the source for the driver and runner. I haven't fully grokked everything going on in there but have gained enough that the brief description above about what a Command in the log references / holds on to in memory as well as the difference between interactive vs non interactive modes.

In terms of my use case, this stems from ongoing work in the space of Endurance testing (#1435)

I'm really trying to minimize the amount of memory consumed by the Cypress runner (we're hitting 3GB pretty quickly in these endurance tests even in non-interactive) as well as reducing the noise in memory snapshots. As you've surmised, I'd like to have additional options to disable logs and snapshots when running in the interactive mode as we visually inspect the long running tests to confirm they are stitching the tests together correctly and iterating on that is much easier in the interactive mode vs reviewing the video after the fact.

To that end, I've been using the following in my tests to remove snapshots:

(cy as any).createSnapshot = (_: any) => ({ 
            body: undefined, 
            htmlAttrs: undefined, 
            headStyles: undefined, 
            bodyStyles: undefined, 
        }); 
 
        Cypress.config("isInteractive" as any, false);

And now I would like to remove logs. It seemed like the above hook onBeforeLog would provide a reasonable part of this requirement if not for what appears to be a bug?

I'd love to see more API surface area in this space if you're open to it.

The three options I see initially:-

  • Disable snapshots
  • A way to conditionally filter out logs, the commands and their related DOM elements (I'd still like to log explicit messages to the logs as each test phase begins / ends)
  • A way to completely hide / disable the command log panel so its just the application's canvas.

My hope is these options will remove enough of the Cypress in memory information for helping with memory snapshot inspection and also reduce some of the memory pressure around long running tests in the interactive runner.

Does any of that make any sense?

@jennifer-shehane
Copy link
Member

Thanks for all of the discussion! This PR will be closed in favor of the strategy outline in this issue: #1586

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.

5 participants