-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
BREAKING: add breaking changes/deprecations to cypress dependencies, … #27642
BREAKING: add breaking changes/deprecations to cypress dependencies, … #27642
Conversation
41 flaky tests on run #50417 ↗︎Details:
|
Test | Artifacts | |
---|---|---|
cy.origin Cypress API > Commands > adds a custom command |
Test Replay
Output
|
commands/traversal.cy.ts • 1 flaky test • 5x-driver-electron
Test | Artifacts | |
---|---|---|
cy.origin traversal > .children() |
Test Replay
Output
|
commands/actions.cy.ts • 1 flaky test • 5x-driver-electron
Test | Artifacts | |
---|---|---|
cy.origin actions > .type() |
Test Replay
Output
|
logging.cy.ts • 1 flaky test • 5x-driver-electron
Test | Artifacts | |
---|---|---|
cy.origin logging > groups callback commands on a passing test |
Output
|
commands/log.cy.ts • 1 flaky test • 5x-driver-electron
Test | Artifacts | |
---|---|---|
cy.origin log > logs in primary and secondary origins |
Test Replay
Output
|
The first 5 flaky specs are shown, see all 29 specs in Cypress Cloud.
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.
1c4aaea
to
1814db3
Compare
I cannot write a system test yet for the TLS error decorating, but I can add it once we update to Node 18. The only way to do that currently would be to write a system test on node 18 (since we are on 16), which isn't worth the effort. Here is a manual test of what the results would look like if a user ran into this on Node 18+ |
1814db3
to
7945637
Compare
We attempted to make an http request to this URL but the request failed without a response. | ||
// if we detect there are TLS v1/v1.1 errors, we want to direct users on | ||
// ways they might be able to resolve their issue as the default error is cryptic | ||
// TODO: UPDATE system-tests/test/visit_spec.js TLSv1 test to check for this error once on Electron 25 / Node 18 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ill update the system test in the electron upgrade that actually checks for the printing of this message
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, this has nothing to do with the shipped cypress node version. As lonmg as the users origin server is running a node version where they can support tls 1 or 1.1 this will be fine.
@@ -24,12 +24,6 @@ describe('e2e binary CI environments', () => { | |||
}, | |||
) | |||
|
|||
// TODO: Where is this image located? Needs to be bumped to Node 16.16.0 or later | |||
smokeTestDockerImage( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We aren't sure where this image lives and is just making sure plain xvfb fails in node 12 with cypress. I don't think this test is incredibly valuable to keep around, let alone we don't know what the image is or where its maintained
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AtofStryker This image is here. Not sure if you want to update the image or remove. https://github.com/cypress-io/cypress-internal-docker-images
…such as typescript and node that may affect users moving forward on v13
7945637
to
34ae9da
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All looks good. Thanks for investigating all the nuances @AtofStryker
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
…such as typescript, node, and TLS, that may affect users moving foward on v13
Additional details
In order to get ahead of the Electron update and account for known breaking changes ahead of time, we want to remove/deprecate support for TLS, node versions, and typescript ahead of time so we don't have to publish a breaking change with the electron update, which includes an update to Node 18. These deprecations/removals include
4.x
to guarantee compatibility with Node 18 types. The@types/node
change will take place when the electron update lands. Typescript3.x
hasn't been updated in 3 years and isn't used by many (most are on v5)TLS
versions1.0
and1.1
and are on node 18. This is mainly due to the change in supported ciphers between Node 16 and Node 18, whereTLS
versions1.0
and1.1
require disabled ciphers. The cypress proxy server still supports these versions, but the cipher in node needs to be enabled. This can we worked around by users if they pass in--tls-cipher-list=DEFAULT@SECLEVEL=0
into theirNODE_OPTIONS
, such as:NODE_OPTIONS="--tls-cipher-list=DEFAULT@SECLEVEL=0" yarn
Steps to test
How has the user experience changed?
PR Tasks
cypress-documentation
? add changelog, remove node 16 support, and update docker image refere… cypress-documentation#5434type definitions
?