-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
fix: ct testing support for node 17+ #21430
Merged
Merged
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
8d88b3b
fix: ct testing support for node 17+
ZachJW34 495cc81
address feedback
ZachJW34 9c548dc
add binary tests, fix NODE_OPTIONS not being set on windows
ZachJW34 0b9b011
dont use bin since it breaks on windows
ZachJW34 986c177
revert gulp change
ZachJW34 0654882
adjust test descriptions
ZachJW34 56ddea8
Merge remote-tracking branch 'origin/10.0-release' into zachw/ct-node-17
ZachJW34 de551f7
Merge branch '10.0-release' into zachw/ct-node-17
ZachJW34 d8ff77e
Merge branch '10.0-release' into zachw/ct-node-17
ZachJW34 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
const cp = require('child_process') | ||
const path = require('path') | ||
const semver = require('semver') | ||
|
||
const webpackCli = path.join(__dirname, '..', 'node_modules', 'webpack-cli', 'bin', 'cli.js') | ||
|
||
// https://github.com/cypress-io/cypress/issues/18914 | ||
// Node 17+ ships with OpenSSL 3 by default, so we may need the option | ||
// --openssl-legacy-provider so that webpack@4 can use the legacy MD4 hash | ||
// function. This option doesn't exist on Node <17 or when it is built | ||
// against OpenSSL 1, so we have to detect Node's major version and check | ||
// which version of OpenSSL it was built against before spawning the process. | ||
// | ||
// Can be removed once the webpack version is upgraded to >= 5.61, | ||
// which no longer relies on Node's builtin crypto.hash function. | ||
|
||
let NODE_OPTIONS = process.env.NODE_OPTIONS || '' | ||
|
||
if (process.versions && semver.satisfies(process.versions.node, '>=17.0.0') && semver.satisfies(process.versions.openssl, '>=3', { includePrerelease: true })) { | ||
NODE_OPTIONS = `${NODE_OPTIONS} --openssl-legacy-provider` | ||
} | ||
|
||
cp.execSync(`node ${webpackCli} ${process.argv.slice(2).join(' ')}`, { stdio: 'inherit', env: { ...process.env, NODE_OPTIONS } }) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,15 @@ function smokeTestDockerImage (dockerImage: string) { | |
specDir: 'tests', | ||
project: 'todos', | ||
}) | ||
|
||
systemTests.it(`can run in ${dockerImage}`, { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. clean up the hierarchy of tests here? right now it's:
but something like this would be better:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch, addressed in 0654882 |
||
withBinary: true, | ||
browser: 'electron', | ||
dockerImage, | ||
testingType: 'component', | ||
project: 'simple-ct', | ||
spec: 'src/simple_passing_component.cy.js', | ||
}) | ||
} | ||
|
||
describe('e2e binary node versions', () => { | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
In general, we should never use
'localhost'
in the App due to issues like this. We've even had users with an incorrectlocalhost
entry in/etc/hosts
, which causes mind boggling issues.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.
I was absolutely baffled as to what was going on until I stumbled upon the issues I listed above.