feat(e2e): Enhance end-to-end testing and CI configuration #214
Conversation
- Updated `package.json` to replace the `test:e2e` script with a new `e2eTest.js` script for improved end-to-end testing. - Added multiple new npm scripts for e2e testing, including cleanup, linting, unit tests, and code preparation. - Refactored GitHub Actions CI workflow to streamline e2e testing process, replacing the previous build job with a dedicated `e2e-tests` job. - Introduced `e2eTest.js` script to encapsulate the e2e testing logic, including cleanup and error handling. - Enhanced `importPreferences.js` to provide more informative logging during directory creation.
There was a problem hiding this comment.
I haven't fully reviewed but I've already spotted too multiple weird things for which I'll need answers.
The main thing would be: what's the point grouping everything in a single and big JavaScript file? And then using weird mechanism like calling npm commands from it?
If your E2E tests depend on other npm scripts, you can do things like that:
{
"scripts": {
"compile:js": "sgmf-scripts --compile js",
"test:e2e": "npm run compile:js && ./scripts/e2eTest.js"
}
}
| "e2e:import-preferences": "npm run import:preferences", | ||
| "e2e:run-sfcc-job": "npm run run:sfcc-job", | ||
| "e2e:variation-index-test": "npm run test:variationindex", | ||
| "e2e:frontend-test": "npm run test:frontend" |
There was a problem hiding this comment.
What's the benefit of duplicating the scripts like that?
| - name: Make e2eTest.js executable | ||
| run: chmod +x scripts/e2eTest.js |
There was a problem hiding this comment.
Not needed, you've already committed it with execution rights
| "test:e2e": "./scripts/e2eTest.js", | ||
| "test:variationindex": "jest --testPathPattern='test/e2e/variation_index.test.js'", | ||
| "e2e:cleanup": "rm -rf e2e-tests e2e-tests.zip site_import.zip site_import \"$CODE_VERSION\" \"$CODE_VERSION.zip\"", | ||
| "e2e:check-codeversion": "if [ -z \"$CODE_VERSION\" ]; then echo 'ERROR: CODE_VERSION is not set. Please define it in your .env file or export it in your shell.'; exit 1; fi", |
There was a problem hiding this comment.
What is the reason of doing it like that?
Calling an npm command from a Node child_process to check an env var is really weird.
For other env vars, you simply check process.env[varName], why can't you do the same for CODE_VERSION?
There was a problem hiding this comment.
This script is not used anywhere anymore, is it on purpose?
| } | ||
|
|
||
| function lintJS() { | ||
| execSync('npm run e2e:lint', { stdio: 'inherit' }); |
There was a problem hiding this comment.
Linting is not related to E2E, why doing it here?
| } | ||
|
|
||
| function runUnitTests() { | ||
| execSync('npm run e2e:unit', { stdio: 'inherit' }); |
There was a problem hiding this comment.
Unit tests are unit tests, I don't get why you are mixing everything in a single JavaScript file.
| } | ||
|
|
||
| function deployCode() { | ||
| execSync('npm run e2e:deploy-code', { stdio: 'inherit' }); |
There was a problem hiding this comment.
This is a bit crazy: the npm script executes a JavaScript file. Why not require it directly?
What is Changed
e2e.shto support multiple test configurations for variation and master-level products, ensuring required environment variables are set before running tests.TEST_MASTER_PRODUCT_IDand added matrix strategy for running frontend tests with different record models and index prefixes.frontend.cy.jsfor Algolia search scenarios, replacing the deletedvariation.cy.js.importPreferences.jsto dynamically set preferences based on the record model.variation_index.test.jsto handle product searches based on the current record model.Internal documentation is here
Need to add
TEST_MASTER_PRODUCT_IDandCODE_VERSIONto GitHub vars for running tests correctly