Capture stderr in e2e tests

[flotwig]

• Closes Snapshot stderr in e2e tests #5269

User facing changelog

N/A - not user facing

Additional details

• This PR makes it so that all tests run through the CLI, instead of using the binary directly.
• This PR interleaves stderr with the stdout that is snapshotted in e2e tests. stderr lines are prefixed with [e2e stderr]: to aid with understanding what parts of a snapshot are stderr and what parts are stdout.
• Surprisingly, there is currently no stderr in Cypress's normal operation, so the snapshots are unchanged.
• Should the e2e test filter stderr using the same rules as in the CLI?
  • I think yes - so the stderr captured is representative of the actual stderr a user would see
  • accomplished by running all tests through cli
• Wait for Connect to Chrome Remote Interface when launching Chrome and record video #4628 and Use Chrome DevTools Protocol to set cookies in Chrome #5206 to merge since it will remove Electron warning from stderr
• Wait for snap-shot-it release with feat: allow empty string values to be snapshotted bahmutov/snap-shot-core#311 so empty stderr can be snapshotted

This captures stderr from e2e tests in addition to stdout. Most specs don't produce any stderr, but this will allow us to keep track of unwanted stderr output in the future.

PR Tasks

• Have tests been added/updated?

[cypress-bot]

Thanks for the contribution! Below are some guidelines Cypress uses when doing PR reviews.

• Please write [WIP] in the title of your Pull Request if your PR is not ready for review - someone will review your PR as soon as the [WIP] is removed.
• Please familiarize yourself with the PR Review Checklist and feel free to make updates on your PR based on these guidelines.

PR Review Checklist

If any of the following requirements can't be met, leave a comment in the review selecting 'Request changes', otherwise 'Approve'.

User Experience

• The feature/bugfix is self-documenting from within the product.
• The change provides the end user with a way to fix their problem (no dead ends).

Functionality

• The code works and performs its intended function with the correct logic.
• Performance has been factored in (for example, the code cleans up after itself to not cause memory leaks).
• The code guards against edge cases and invalid input and has tests to cover it.

Maintainability

• The code is readable (too many nested 'if's are a bad sign).
• Names used for variables, methods, etc, clearly describe their function.
• The code is easy to understood and there are relevant comments explaining.
• New algorithms are documented in the code with link(s) to external docs (flowcharts, w3c, chrome, firefox).
• There are comments containing link(s) to the addressed issue (in tests and code).

Quality

• The change does not reimplement code.
• There's not a module from the ecosystem that should be used instead.
• There is no redundant or duplicate code.
• There are no irrelevant comments left in the code.
• Tests are testing the code’s intended functionality in the best way possible.

Internal

• The original issue has been tagged with a release in ZenHub.

[brian-mann]

This PR isn't necessary - all that's necessary is that we go through the CLI itself for e2e tests, which means we automatically get the logic from the CLI that does the filtering. This has been needed for a long time now - and with the advent of the --dev flag, it's made this possible for months.

Previously the CLI would always expect the raw built binary, but the --dev flag tells it to use the development non built binary. Several of our other npm commands use this flag.

The scripts/e2e file just needs to be updated to go through the CLI.

I'm closing but you can reopen if I'm missing something.

[flotwig]

Even if the e2e test went through the CLI, it doesn't snapshot the stderr from the child process, only the stdout, which is the purpose of this PR. So I think this is still necessary.

I can try refactoring the e2e tests to go through the CLI so that I don't need to require that function directly. That would be a nice improvement for the robustness of the e2e tests as a whole as well.

[brian-mann]

That's true - I'll reopen and we can rearchitect it.

[flotwig]

Been thinking about this, it probably makes more sense to have stderr interleaved with stdout in the snapshots, since it will help snapshot the visible output a user would get, and guarantee that the ordering of stderr vs. stdout does not change. Maybe stderr lines could get prefixed with stderr: or something to make the output clear.

[cypress]

---

Test summary

3511 • 0 • 45 • 0

---

Run details

Project	cypress
Status	Passed
Commit	99ec5c7
Started	Nov 20, 2019 10:40 PM
Ended	Nov 20, 2019 10:44 PM
Duration	03:53 💡
OS	Linux Debian - 9.8
Browser	Multiple

View run in Cypress Dashboard ➡️

---

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

[jennifer-shehane]

Unfortunately we have to close this issue due to inactivity. Please comment if there is new information to provide concerning the original issue and we can reopen.