-
Notifications
You must be signed in to change notification settings - Fork 115
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: broken ci flow in Node 18 #947
Conversation
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
0d6dcfb
to
28f5700
Compare
Signed-off-by: SuZhou-Joe <[email protected]>
28f5700
to
007a14f
Compare
Signed-off-by: SuZhou-Joe <[email protected]>
9f95faf
to
2ca3977
Compare
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
1468dcc
to
2093eff
Compare
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
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.
Shall we make --dns-result-order=ipv4first
the default option?
cypress.json
Outdated
@@ -20,6 +20,7 @@ | |||
"VISBUILDER_ENABLED": true, | |||
"DATASOURCE_MANAGEMENT_ENABLED": false, | |||
"ML_COMMONS_DASHBOARDS_ENABLED": true, | |||
"WAIT_FOR_LOADER_BUFFER_MS": 0 | |||
"WAIT_FOR_LOADER_BUFFER_MS": 0, | |||
"NO_COMMAND_LOG": 1 |
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.
Q: could you elaborate this change? seems not related to this PR
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.
On Chrome 117 the test will hang because of browser crash. According to the doc here it seems the only option we can choose.
It is better to leave a comment here but it seems we can not write comment in json file.
cypress/plugins/index.js
Outdated
@@ -24,4 +24,20 @@ | |||
module.exports = (on, config) => { | |||
// `on` is used to hook into various events Cypress emits | |||
// `config` is the resolved Cypress config | |||
on('before:browser:launch', (browser = {}, launchOptions) => { |
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.
The same here, could you elaborate a bit? seems not related to this PR
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.
Sure, added comment above the code.
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
We should, and I moved the server="0.0.0.0" to NODE_OPTIONS env. |
I mean, shall we add it to |
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>
Description
For now all CI flows failed as OSD upgrade Node version to v18, which use ipv6 by default. Add --server.host="0.0.0.0" to enforce CI to setup OSD server in ipv4.
Issues Resolved
#902
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.