fix: move node 17 Check from Binary to CLI #19977
Conversation
emilyrohrbough
requested review from
jennifer-shehane
and removed request for
a team
|
Thanks for taking the time to open a PR!
|
emilyrohrbough
changed the title
|
Test summary
Run details
View run in Cypress Dashboard ➡️ Flakiness
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 |
||||||||||||||||||||||||||||||||||
.husky/pre-commit
Outdated
| @@ -1,4 +1,4 @@ | |||
| #!/bin/sh | |||
| . "$(dirname "$0")/_/husky.sh" | |||
|
|
|||
| npx lint-staged | |||
| # npx lint-staged | |||
There was a problem hiding this comment.
Did you intend to check this in?
There was a problem hiding this comment.
lol no..This was when I push the pre-tested changes up.
cli/lib/util.js
Outdated
| @@ -286,14 +288,29 @@ const util = { | |||
| .value() | |||
| }, | |||
|
|
|||
| getOriginalNodeOptions (options) { | |||
| getOriginalNodeOptions ({ processVersions }) { | |||
There was a problem hiding this comment.
Could you get proccess.versions here instead of passing it in? Is this called with the same node version that launched the cli?
| } | ||
|
|
||
| return {} | ||
| return opts |
There was a problem hiding this comment.
what does this return value get used for? Previously it always returned an empty object, but now we actually return stuff.
There was a problem hiding this comment.
previously in the if process.env.NODE_OPTIONS check it'd return fast with
{ ORIGINAL_NODE_OPTIONS: ... }
This removed the fast-return to got through both checks before returning. If neither check are hit, it'll continue to return an empty object.
BlueWinds
requested review from
tbiethman and
ryanthemanuel
and removed request for
tbiethman
* 10.0-release: fix: restore @lmiller1990's changes feat: styling snapshots (#19972) fix bad merge overrides- GOOD CATCH TYLER! fix(unify): Updating reporter to consistently use app-provided "Preferred Editor" dialog (#19933) fix: move node 17 Check from Binary to CLI (#19977) fix: pass correct spec URL in `cypress run` on Windows (#19890) fix: send click event with `cy.type('{enter}')`. (#19726) feat: detect package manager in wizard (#19960) fix: refactor set specs by specPattern (#19953) chore: Update Chrome (beta) to 98.0.4758.74
Move logic for determining whether or not the
--openssl-legacy-providerflag should be passed for Node17 built with OpenSSL v3+ from binary to CLI package. The last change to check for OpenSSL version is checking the version of Node process bundled with Cypress, not the system Node process on the user's machine. This caused the check to be false and always skip adding the--openssl-legacy-providerflag when the system node version is Node 17 built with OpenSSL v3+.Moving this logic aligns with where we determine the system Node version and parse the ORIGINAL_NODE_OPTIONS.
I have updated the cypress-test-node-versions repository to perform a simple test to verify that run mode works as expected. PR. You can verify the commit with this change builds successfully with Node 17.
User facing changelog
--openssl-legacy-providerflag should be passed the plugins' child process. This always resolved to Cypress's bundled Node version of 16.5.0 which was built with OpenSSL v1, when it should have been checking the system Node process's OpenSSL version. Fixes #19712.PR Tasks
cypress-documentation?type definitions?cypress.schema.json?