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

Reporter error improvements #3930

Merged
merged 629 commits into from
May 6, 2020
Merged

Conversation

jennifer-shehane
Copy link
Member

@jennifer-shehane jennifer-shehane commented Apr 10, 2019

User facing changelog

We've improved the display of errors in the Test Runner's Command Log. Errors now show a code frame preview with the highlighted line where the error occurred. You can now click links to files relevant to the errors that can be opened on your computer or in your preferred editor at the exact line of code. You can also click to expand the stack trace inline in the Command Log.

Additional details

  • Split up error messages into separate DOM elements
  • Allow driver errors to return object including docsUrl
  • Update all error messages to include markdown where appropriate
  • Refactor error specific utils into their own file
  • Cover errors with more tests

How has the user experience changed?

Before

Screen Shot 2020-01-10 at 3 33 19 PM

After

Code frame with highlighted line where err is being thrown

Screen Shot 2020-01-10 at 2 58 33 PM

Expandable stack trace
Screen Shot 2020-01-17 at 1 31 54 PM

'Learn more' link in err message if relevant documentation to link to

Screen Shot 2020-01-10 at 3 47 25 PM

Clear 'print error to console' button

Screen Shot 2020-01-10 at 3 00 49 PM

Select File opener Preference (including OS-specific file opener)

Screen Shot 2020-01-17 at 1 13 13 PM

Error if no path given for Other

Screen Shot 2020-01-17 at 1 14 02 PM

Select / change file opener preference from Settings

Screen Shot 2020-01-17 at 1 17 29 PM

PR Tasks

Implementation Notes

The various error scenarios Cypress can encounter and how to address them

Error: err.name
Uncaught: whether it's an uncaught error
Codeframe: what the code frame should display
Original stack: what does the error stack include before we've modified it at all
Replace: whether we should replace the stack with the user invocation stack
Translate: whether we should translate the stack to point to source files instead of the built files served to the browser
Append: whether we should append all or part of the original error stack to the newly replaced stack

====
Scenario 1: command assertion errors
====
cy.wrap({ foo: 'foo' })
  .should('have.property', 'foo')
  .should('equal', 'bar')
//
cy.get('#non-existent') // default assertion

- Error: AssertionError
- Uncaught: false
- Codeframe: cy.should, cy.get
- Original Stack: cypress internals
- Replace: yes, with command invocation stack
- Translate: yes
- Append: no


====
Scenario 2: exceptions
====
// at root level
foo.bar()
// in test
foo.bar()
// at root or in test
setTimeout(() => {
  foo.bar()
}, 20)
cy.wait(10000)
//
cy.wrap({}).should(() => {
  foo.bar()
})
//
Cypress.Commands.add('foo', () => {
  foo.bar()
})
cy.foo()

- Error: ReferenceError, etc
- Uncaught: true/false
- Codeframe: foo.bar()
- Original Stack: spec callsite
- Replace: no
- Translate: yes
- Append: no


====
Scenario 3: assertion errors
====
// at root
expect(true).to.be.false
// in test
expect(true).to.be.false
//
cy.wrap({}).then(() => {
  expect(true).to.be.false
})
//
cy.wrap({}).should(() => {
  expect(true).to.be.false
})

- Error: AssertionError
- Uncaught: true/false
- Codeframe: expect()
- Original Stack: cypress internals
- Replace: yes, with assertion invocation stack
- Translate: yes
- Append: no


====
Scenario 4: validation errors
====
cy.viewport() // invalid args
// double visit
cy.visit('/test.html')
cy.visit('https://google.com')

cy.get('div:first').then(($el) => {
  $el.hide()
})
.click()

expect(true).to.be.nope

- Error: CypressError, Error<Chai validation>
- Uncaught: false
- Codeframe: cy.viewport
- Original Stack: cypress internals / chai internals
- Replace: yes
- Translate: yes
- Append: yes


====
Scenario 5: app sync uncaught errors
====
cy.visit('/visit_error.html') // error synchronously
cy
.visit('/error_on_click.html')
.get('p').click() // error on click event

- Error: ReferenceError, etc
- Uncaught: true
- Codeframe: none (but want to show app code + maybe cy.visit/cy.click)
- Original Stack: app code
- Replace: no
- Translate: yes (but can't now)
- Append: no


====
Scenario 6: app async uncaught errors
====
cy.visit('/html/visit_error.html') // error asynchronously

- Error: ReferenceError, etc
- Uncaught: true
- Codeframe: none (but want to show app code, don't show cy.visit)
- Original Stack: app code
- Replace: no
- Translate: yes (but can't now)
- Append: no


====
Scenario 7: network errors
====
cy.visit('/404')
cy.request('http://localhost:12345')

- Error: RequestError, etc - wrapped in CypressError
- Uncaught: false
- Codeframe: cy.visit
- Original Stack: node, cypress internals
- Replace: yes, with command invocation stack
- Translate: yes
- Append: yes, append both node and cypress internals

====
Rules:
====
- if assertion error, don't append original stack because the rules of why it errored are explicit
- if internal cypress error, append original stack so users can use it to understand the source of the error
- if internal cypress error originating from node, append node stack and internal cypress stack


TODO:
- if visit synchronously fails in app code, point to cy.visit in spec code
  - add 2nd code frame that points to app code
- don't hide stack trace by default, because now it's actually useful and not overly verbose with internal crap
- profile how much time Bluebird spends when we enable longStackTraces (cypress.js) and maybe turn it always off or always on
- splice out cypress internals when there's a user error (Scenario 2 when error is in a command callback)

@cypress

This comment has been minimized.

@chrisbreiding chrisbreiding changed the title [WIP] reporter error improvements Reporter error improvements Jul 31, 2019
@jennifer-shehane

This comment has been minimized.

@jennifer-shehane

This comment has been minimized.

@brian-mann
Copy link
Member

Headsup: the changes to the spec files (like type_spec.coffee) are about to be in conflict because we're merging in a decaffeinated version of them today.

@brian-mann
Copy link
Member

There appears to be a some missing backticks on the errors - and others have backticks but are not highlighted correctly.

I also think the "Learn More" gets lost because it's styled as red like the other content and doesn't appear to be a link.

@jennifer-shehane
Copy link
Member Author

@brian-mann Yeah, I'm working on getting more backticks correct and pulling on docsUrls since that work wasn't finished. I didn't want to touch the errors anymore with the big changes/conflicts before - that have been fixed.

I'll fix the merge conflicts as well.

@jennifer-shehane jennifer-shehane changed the title Reporter error improvements [WIP] Reporter error improvements Sep 24, 2019
andrew-codes and others added 17 commits December 2, 2019 17:39
…, where only one item may be selected at a time- RadioInput is an abstraction over the a normal input type="radio"- CustomRadio enables adding a custom component to represent the selected item
…e same accessbility traits of radio groups exist for other selections; such as checkboxes, selectable lists, etc. Refactored RadioGroup to be more generalized. Its responsibilities include managing the selection of items via keyboard shortcuts, tabbing, etc. much like any other radio/select would work. Hopefully this work can be extended to other single select type components; leveraging these components underneath the hood.
The build was failing with webpack complaining that `lodash/fp` could not be found. Instead of spending any more time on it, I decided to just remove its use. I also worry that it adds more size to the bundle since it’s using a different branch of lodash so some amount of lodash is being duplicated in the bundle, though I couldn’t verify that because I could never get it building with `lodash/fp`
@brian-mann brian-mann merged commit a27bd34 into develop May 6, 2020
@Saibamen
Copy link
Contributor

Saibamen commented May 6, 2020

Nice :)
Wait for next release

@kentcdodds
Copy link

This is terrific! It doesn't appear to show the codeframe in the terminal output when running headlessly with cypress run. Is that supposed to be supported as well?

@chrisbreiding
Copy link
Contributor

@kentcdodds Displaying the code frame in terminal output is not currently supported, but we're planning on adding that in the future.

@kentcdodds
Copy link

Good to hear! Is there an issue for tracking that? Or should I open a new one?

@chrisbreiding
Copy link
Contributor

We have it listed in this Epic issue for the next phase of error improvements. Feel free to create an individual issue for it though. It would help us gauge community interest in it and prioritize it.

@kentcdodds
Copy link

#7819 👍

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