-
Notifications
You must be signed in to change notification settings - Fork 15
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 E2E Tests parallelization #3208
base: main
Are you sure you want to change the base?
Changes from 42 commits
246d0b3
d8d8ccd
8ddc6f1
0efd1d4
00b728c
11dc3ce
d596413
96789ab
42d88c6
dbfcb09
4f14c66
e1d2864
9bd31ad
c7b3290
441e1d9
14ad1c9
f432f7e
ce9e2b9
a9f3c36
7288119
0c50871
73b76c9
8383884
7034908
15b9b64
53c173e
796bb36
530fed5
a5993d4
7b45b21
c472819
0ebcfac
fd242bd
a281e3d
bc92f77
55bd49b
77166ea
ca3bfae
43f967c
09a331d
72c817d
1075ea7
4473d68
7e8446a
40b91b0
fb6973d
acd7891
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -15,18 +15,26 @@ | |||
/** | ||||
* @type {Cypress.PluginConfig} | ||||
*/ | ||||
// eslint-disable-next-line no-unused-vars | ||||
|
||||
const cypressSplit = require('cypress-split'); | ||||
const { exec } = require('child_process'); | ||||
const http = require('http'); | ||||
const webpack = require('@cypress/webpack-preprocessor'); | ||||
let heartbeatsIntervals = []; | ||||
|
||||
module.exports = (on, config) => { | ||||
// `on` is used to hook into various events Cypress emits | ||||
// `config` is the resolved Cypress config | ||||
on('before:run', async () => { | ||||
const photofinishBinary = | ||||
await getPhotofinishBinaryAndGiveExecutablePermissions(); | ||||
await runPhotofinishMainScenario(photofinishBinary); | ||||
}); | ||||
cypressSplit(on, config); | ||||
on('task', { | ||||
startAgentHeartbeat(agents) { | ||||
const { web_api_host, web_api_port, heartbeat_interval } = config.env; | ||||
const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); | ||||
const heartbeat = (agentId) => | ||||
http | ||||
.request({ | ||||
|
@@ -37,13 +45,15 @@ module.exports = (on, config) => { | |||
}) | ||||
.end(); | ||||
|
||||
agents.forEach((agentId) => { | ||||
heartbeat(agentId); | ||||
let interval = setInterval( | ||||
() => heartbeat(agentId), | ||||
heartbeat_interval | ||||
); | ||||
heartbeatsIntervals.push(interval); | ||||
sleep(500).then(() => { | ||||
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. issue: Do you think we can better understand why it is needed, to remove it? 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. I'm not a fan of arbitrary waits either and I know they should be avoided as a good practice, for the moment I don't have a conclusive answer tbh but I noticed that with a bit of delay the hosts_overview test flakiness is reduced a lot so I added it as a contingency measure, we can remove it if you want or even we can try the test flakiness tool designed by @gagandeepb then try with and without the arbitrary sleep and compare results. 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. I don't have an alternative so I guess I must live with it 🤷♂️ |
||||
agents.forEach((agentId) => { | ||||
heartbeat(agentId); | ||||
let interval = setInterval( | ||||
() => heartbeat(agentId), | ||||
heartbeat_interval | ||||
); | ||||
heartbeatsIntervals.push(interval); | ||||
}); | ||||
}); | ||||
return null; | ||||
}, | ||||
|
@@ -66,3 +76,26 @@ module.exports = (on, config) => { | |||
|
||||
return config; | ||||
}; | ||||
|
||||
function runCommand(command) { | ||||
return new Promise((resolve, reject) => { | ||||
exec(command, (error, stdout, stderr) => { | ||||
if (error) reject(new Error(`Error: ${stderr || error.message}`)); | ||||
else resolve(stdout.trim()); | ||||
}); | ||||
}); | ||||
} | ||||
|
||||
async function getPhotofinishBinaryAndGiveExecutablePermissions() { | ||||
const photofinishBinary = await runCommand( | ||||
'whereis photofinish | cut -d" " -f2' | ||||
); | ||||
await runCommand(`chmod +x ${photofinishBinary}`); | ||||
return photofinishBinary; | ||||
} | ||||
|
||||
async function runPhotofinishMainScenario(photofinishBinary) { | ||||
return runCommand( | ||||
`cd ../.. && ${photofinishBinary} run --url "http://localhost:4000/api/collect" healthy-29-node-SAP-cluster` | ||||
); | ||||
} | ||||
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. question: Why can't we use the Also, why the need to rewrite how we configure the photofinish command? See web/test/e2e/cypress/support/commands.js Line 98 in c9c0024
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. because when this piece of code is executed, cy commands are still not available, cypress env parameters are not available either at that point of execution. Maybe we could use regular node env variables with process.env 🤔 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.
Maybe it's better, we keep the same structure. Also, if we don't use the 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. well, actually it is still used in some tests, specially to restore scenarios that previous test modified, and to load some other different scenarios. |
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.
question: This means the scenario is loaded once per process, right?
So, given the target application is the same, the CI will be executed 8 times while on local execution just once.
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.
that's it, once per process, so if you execute the whole test suite locally (sequential) this will be executed just once at the very beginning.