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

cypress-cli type for CypressRunResult::runs is incorrect (can be null/undefined) #6126

Closed
xdumaine opened this issue Jan 8, 2020 · 5 comments · Fixed by #6127
Closed

cypress-cli type for CypressRunResult::runs is incorrect (can be null/undefined) #6126

xdumaine opened this issue Jan 8, 2020 · 5 comments · Fixed by #6127
Labels
good first issue type: typings

Comments

@xdumaine
Copy link
Contributor

@xdumaine xdumaine commented Jan 8, 2020

Current behavior:

In the event that the browser fails to connect, the run promise resolves with results which has runs of null or undefined. (Which is likely a separate issue altogether. It should probably not resolve at all - it should reject).

However, the typings indicate that the type of runs is RunResult[] which doesn't match null

Desired behavior:

The type should indicate that runs can be undefined or null, or run() should not resolve on an error running the tests altogether.

Test code to reproduce

  1. Clone https://github.com/xdumaine/cypress-test-tiny
  2. npm i && npm test
@cypress-bot cypress-bot bot added the stage: ready for work label Jan 13, 2020
@jennifer-shehane jennifer-shehane added topic: typescript stage: ready for work good first issue and removed stage: ready for work labels Jan 13, 2020
@ddayguerrero
Copy link

@ddayguerrero ddayguerrero commented Jan 20, 2020

👋 Hey! While there might be work in progress, I'd be interested in helping and ultimately achieving a first time contribution for Cypress.

Following the instructions within the cypress-test-tiny fork, I've been able to reproduce the error with the following environment:

Cypress:    3.8.1
Browser:    Chrome 79

As previously mentioned and from my understanding, there are currently a couple of ways of solving this:

  1. Playing around with the CypressRunResult type interface to support null or undefined special types.
  2. Handling the rejected state upon invoking the promise.

Option 2 sounds like the long term solution. I think changes will be around the lines of https://github.com/cypress-io/cypress/blob/develop/packages/server/lib/plugins/util.coffee#L37 however it seems like we would need to distinguish before:browser:launch and task events.

Can I help with with 2, with the short term solution 1, or with any other suggested solution?

@jennifer-shehane
Copy link
Member

@jennifer-shehane jennifer-shehane commented Mar 3, 2020

@ddayguerrero Yes we would love an open PR - even one that is a work in progress is fine. Check out our contributing doc and ask us if you get stuck.

@jennifer-shehane jennifer-shehane added type: typings and removed topic: typescript labels Mar 3, 2020
@xdumaine
Copy link
Contributor Author

@xdumaine xdumaine commented Mar 3, 2020

@jennifer-shehane my PR is ready for final review...

@cypress-bot cypress-bot bot added the stage: pending release label Apr 14, 2020
@cypress-bot
Copy link

@cypress-bot cypress-bot bot commented Apr 14, 2020

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

@cypress-bot cypress-bot bot removed the stage: ready for work label Apr 14, 2020
@cypress-bot
Copy link

@cypress-bot cypress-bot bot commented Apr 20, 2020

Released in 4.4.1.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v4.4.1, please open a new issue.

@cypress-bot cypress-bot bot removed the stage: pending release label Apr 20, 2020
@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Apr 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue type: typings
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants