fix: error regression - strip ansi colors out of cy.fixture() error message #20335

brian-mann · 2022-02-23T17:17:14Z

Closes #20208

User facing changelog

Fix regression that caused ANSI color colors to show up in cy.fixture(...) errors
Fix regression that test config override errors to format incorrectly

Additional details

Errors originating from the server (@packages/errors) were altered as part of a larger errors improvement here #20072. In this one particular code, ANSI color styles were showing up as a command log error.

How has the user experience changed?

Before

After

PR Tasks

Have tests been added/updated?
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?

…ssage

cypress-bot · 2022-02-23T17:17:16Z

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.

cypress · 2022-02-23T17:28:51Z

Test summary

19278 • 0 • 218 • 0 • 1

Run details


Project	cypress
Status	Passed
Commit	`16d98b8`
Started	Feb 28, 2022 4:44 PM
Ended	Feb 28, 2022 4:56 PM
Duration	11:28 💡
OS	Linux Debian - 10.10
Browser	Multiple

View run in Cypress Dashboard ➡️

Flakiness

	cypress/integration/cypress/proxy-logging_spec.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

emilyrohrbough

Did a test need to be updated and/or added to ensure this is working as expected?

brian-mann · 2022-02-23T19:50:47Z

Did a test need to be updated and/or added to ensure this is working as expected?

An existing test was modified to ensure the err.message had ansi stripped. No new test needed to be written.

See here: https://github.com/cypress-io/cypress/pull/20335/files#diff-7bf5eaece2933de0e512e9c339f6f16e0c313696d05d1dab0cfe332f858be614R136

brian-mann · 2022-02-23T19:54:38Z

Actually don't merge this yet - I have one other area to check to ensure ANSI styles aren't leaking out. Has to do with config validation when providing overrides to the it() or describe() definitions

emilyrohrbough · 2022-02-23T21:48:20Z

packages/server/lib/fixture.js

+          // the @packages/error list, it should be written in
+          // the driver since this error can only occur within
+          // driver commands and not outside of the test runner
+          const err = errors.get('FIXTURE_NOT_FOUND', relativePath, extensions)


Would it make more sense to update the error to remove the ansi from the FIXTURE_NOT_FOUND error (it has a // fix me comment we can remove).

I assume there is risk other errors created in the server that's sent back to the driver will also need to strip out asni? It seem like it'd be safer to add this logic to the reporter directly in the error component: https://github.com/cypress-io/cypress/blob/develop/packages/reporter/src/errors/test-error.tsx#L73

That's what I was double checking - there is rarely a situation where we construct the error message in the server with ANSI styling AND send it through to the driver.

That's why I added the FIXME because there's no reason for the server to construct this error. The vast majority of the errors are formatted in the driver in this situation. That's why this broke to begin with. It's like a square peg in a round hole.

I looked into moving this fix more broadly into the server - but because this isn't actually the pattern we want to replicate it seemed fixing it only in this 1 place was most prudent. Once this error is moved into the driver it would obviate a need to ever need to do this in the server.

Sounds good. Sounds like an easy fix that just needs to get done.

emilyrohrbough · 2022-02-23T21:49:31Z

An existing test was modified to ensure the err.message had ansi stripped. No new test needed to be written.

Gottchya. I had expected a test within the packages/server to be updated since that's where the change was made.

jennifer-shehane · 2022-02-25T14:20:05Z

I messaged @brian-mann to revisit.

emilyrohrbough · 2022-02-25T22:23:19Z

Is there another regression example or did markdown syntax use to render in the command log?

…move extra backtick

brian-mann · 2022-02-28T15:41:13Z

Is there another regression example or did markdown syntax use to render in the command log?

@emilyrohrbough I'm not sure what you mean here - it appears this is failing in a different branch (?)

emilyrohrbough · 2022-02-28T15:43:39Z

@brian-mann yes a different branch off of develop - I was looking at the error message format & the markdown syntax in the terminal output.

brian-mann · 2022-02-28T15:45:48Z

@emilyrohrbough I believe you're looking at our own custom formatted error for the snapshotting of mocha events - and this isn't representative of the way the system normally looks or behaves. From that error I can't really tell what is wrong or what should be expected to be there. If you could find a situation regarding a typical error case that's not using internal formatting / utilities I can take a look at it. From what I can tell though - I don't believe this is a regression anywhere.

emilyrohrbough · 2022-02-28T15:46:38Z

@brian-mann Ok! Sounds good to me.

brian-mann · 2022-02-28T15:50:47Z

Added additional user changelog item + before/after pics of the 2nd regression

…cypress-io/cypress into fix/cy-fixture-error-regression

fix cy.fixture regression - strip ansi colors out of fixture error me…

581d66c

…ssage

brian-mann requested a review from a team as a code owner February 23, 2022 17:17

brian-mann requested review from jennifer-shehane and removed request for a team February 23, 2022 17:17

brian-mann changed the title ~~fix cy.fixture regression - strip ansi colors out of fixture error me…~~ fix: cy.fixture regression - strip ansi colors out of fixture error message Feb 23, 2022

brian-mann changed the title ~~fix: cy.fixture regression - strip ansi colors out of fixture error message~~ fix: error regression - strip ansi colors out of cy.fixture() error message Feb 23, 2022

jennifer-shehane removed their request for review February 23, 2022 17:51

empty commit to appease the semantic release gods

891bc5c

mjhenkes previously approved these changes Feb 23, 2022

View reviewed changes

emilyrohrbough reviewed Feb 23, 2022

View reviewed changes

jennifer-shehane requested a review from emilyrohrbough February 23, 2022 21:16

emilyrohrbough reviewed Feb 23, 2022

View reviewed changes

fix 2nd regression formatting when setting invalid config values - re…

6fc37f6

…move extra backtick

brian-mann dismissed mjhenkes’s stale review via 6fc37f6 February 28, 2022 15:38

brian-mann requested review from emilyrohrbough and mjhenkes February 28, 2022 15:48

emilyrohrbough previously approved these changes Feb 28, 2022

View reviewed changes

Merge branch 'develop' into fix/cy-fixture-error-regression

c4e42ee

increase webpack dev server timeout values

aab4d3a

mjhenkes previously approved these changes Feb 28, 2022

View reviewed changes

Merge branch 'fix/cy-fixture-error-regression' of https://github.com/…

16d98b8

…cypress-io/cypress into fix/cy-fixture-error-regression

brian-mann dismissed stale reviews from mjhenkes and emilyrohrbough via 16d98b8 February 28, 2022 16:36

emilyrohrbough mentioned this pull request Feb 28, 2022

9.5.1 Release cypress-io/cypress-documentation#4370

Merged

brian-mann requested review from mjhenkes and emilyrohrbough February 28, 2022 16:57

emilyrohrbough approved these changes Feb 28, 2022

View reviewed changes

mjhenkes approved these changes Feb 28, 2022

View reviewed changes

brian-mann merged commit e0bd6ac into develop Feb 28, 2022

brian-mann deleted the fix/cy-fixture-error-regression branch February 28, 2022 17:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: error regression - strip ansi colors out of cy.fixture() error message #20335

fix: error regression - strip ansi colors out of cy.fixture() error message #20335

brian-mann commented Feb 23, 2022 •

edited

cypress-bot bot commented Feb 23, 2022

cypress bot commented Feb 23, 2022 •

edited

emilyrohrbough left a comment

brian-mann commented Feb 23, 2022 •

edited

brian-mann commented Feb 23, 2022

emilyrohrbough Feb 23, 2022 •

edited

brian-mann Feb 28, 2022 •

edited

emilyrohrbough Feb 28, 2022

emilyrohrbough commented Feb 23, 2022

jennifer-shehane commented Feb 25, 2022

emilyrohrbough commented Feb 25, 2022

brian-mann commented Feb 28, 2022

emilyrohrbough commented Feb 28, 2022

brian-mann commented Feb 28, 2022

emilyrohrbough commented Feb 28, 2022

brian-mann commented Feb 28, 2022

fix: error regression - strip ansi colors out of cy.fixture() error message #20335

fix: error regression - strip ansi colors out of cy.fixture() error message #20335

Conversation

brian-mann commented Feb 23, 2022 • edited

User facing changelog

Additional details

How has the user experience changed?

PR Tasks

cypress-bot bot commented Feb 23, 2022

cypress bot commented Feb 23, 2022 • edited

Test summary

Run details

Flakiness

emilyrohrbough left a comment

Choose a reason for hiding this comment

brian-mann commented Feb 23, 2022 • edited

brian-mann commented Feb 23, 2022

emilyrohrbough Feb 23, 2022 • edited

Choose a reason for hiding this comment

brian-mann Feb 28, 2022 • edited

Choose a reason for hiding this comment

emilyrohrbough Feb 28, 2022

Choose a reason for hiding this comment

emilyrohrbough commented Feb 23, 2022

jennifer-shehane commented Feb 25, 2022

emilyrohrbough commented Feb 25, 2022

brian-mann commented Feb 28, 2022

emilyrohrbough commented Feb 28, 2022

brian-mann commented Feb 28, 2022

emilyrohrbough commented Feb 28, 2022

brian-mann commented Feb 28, 2022

brian-mann commented Feb 23, 2022 •

edited

cypress bot commented Feb 23, 2022 •

edited

brian-mann commented Feb 23, 2022 •

edited

emilyrohrbough Feb 23, 2022 •

edited

brian-mann Feb 28, 2022 •

edited