Test code optimization: avoid running ~70 test cases twice #2095
Conversation
Change-type: patch
Change-type: patch
aa39dc1
to
6a55613
Compare
|
|
||
| module.exports = { | ||
| ...commonConfig, | ||
| spec: ['tests/auth/*.spec.ts', 'tests/commands/**/*.spec.ts'], |
There was a problem hiding this comment.
The objective here was to test CLI commands only, which is the second element in this array. However, I found that it was necessary to run the tests under the auth folder first, as they appear to have CLI login side effects. (There are three test files under the auth folder and, interestingly, it was necessary to run all three of them. Running any single one of the three test files was not sufficient. Hence auth/*.spec.ts.)
(By the way, adding the auth patterns to the mocha command line directly, as mentioned in another comment in this PR, is also something that I tried without success.)
| @@ -61,13 +61,13 @@ | |||
| "test:shrinkwrap": "ts-node --transpile-only automation/run.ts test-shrinkwrap", | |||
| "test:source": "cross-env BALENA_CLI_TEST_TYPE=source mocha", | |||
| "test:standalone": "npm run build:standalone && npm run test:standalone:fast", | |||
| "test:standalone:fast": "cross-env BALENA_CLI_TEST_TYPE=standalone mocha", | |||
| "test:standalone:fast": "cross-env BALENA_CLI_TEST_TYPE=standalone mocha --config .mocharc-standalone.js", | |||
There was a problem hiding this comment.
The objective of this commit is to avoid running a sizeable fraction of the test cases twice, redundantly. The CLI still intentionally runs many tests cases twice, first using the Node.js interpreter available in the PATH, and then using the built pkg binary (standalone zip package), in order to test the pkg binary too. However, this only makes sense for the end-to-end test cases that invoke CLI commands, using the runCommand function in tests/helpers.ts. The runCommand function decides whether to execute the built pkg binary as a child process, or whether to call the CLI entry point in the same mocha process, depending on the value of the BALENA_CLI_TEST_TYPE env var. Other test cases, typically unit tests, don't call the runCommand function, and always run in the mocha process, regardless of the value of BALENA_CLI_TEST_TYPE. It is these other test cases that were being run twice, unnecessarily. This commit avoids running these other tests cases for the test:standalone[:fast] script.
Before I came up with the solution of adding the .mocharc.js and .mocharc-standalone.js files, I tried a smaller change:
"test:standalone:fast": "cross-env BALENA_CLI_TEST_TYPE=standalone mocha tests/commands/**/*.ts",
... i.e., I tried specifying the desired set of test cases on the mocha command line itself. It didn't work: it would always run more tests than specified on the command line. I tried enclosing the pattern with single quotes or double quotes: no difference. I tried different patterns for different folders, I tried explicitly specifying files instead of using wildcard characters, but it would either not work for other reasons, or it would not run the test cases I wanted. Then I tried upgrading mocha from v6 to v8: no difference. (I decided the keep the mocha upgrade anyway, because it seems like a good thing.) Then I finally tested with separate configuration files and the --config option, and it worked fine!
| "test:fast": "npm run build:fast && npm run test:source", | ||
| "test:only": "npm run build:fast && cross-env BALENA_CLI_TEST_TYPE=source mocha \"tests/**/${npm_config_test}.spec.ts\"", | ||
| "catch-uncommitted": "ts-node --transpile-only automation/run.ts catch-uncommitted", | ||
| "ci": "npm run test && npm run catch-uncommitted", | ||
| "watch": "gulp watch", | ||
| "lint": "balena-lint -e ts -e js --typescript --fix automation/ lib/ typings/ tests/ bin/balena bin/balena-dev gulpfile.js", | ||
| "lint": "balena-lint -e ts -e js --typescript --fix automation/ lib/ typings/ tests/ bin/balena bin/balena-dev gulpfile.js .mocharc.js .mocharc-standalone.js", |
There was a problem hiding this comment.
.mocha*.js works on Unix, but not on Windows, hence spelling out the names in full.
| @@ -59,7 +59,7 @@ macOS: | `/usr/local/lib/balena-cli/` <br> `/usr/local/bin/balena` | |||
| [macOS](https://www.architectryan.com/2012/10/02/add-to-the-path-on-mac-os-x-mountain-lion/#.Uydjga1dXDg) | | |||
| [Windows](https://www.computerhope.com/issues/ch000549.htm) | |||
|
|
|||
| > * If you are using macOS Catalina (10.15), [check this known issue and | |||
| > * If you are using macOS 10.15 or later (Catalina, Big Sur), [check this known issue and | |||
There was a problem hiding this comment.
This change is in its own commit. I have also updated issue #1479 to cover both Catalina and Big Sur.
Sadly the impact is minor - seconds rather than minutes. Still, unnecessarily running half of test cases twice seemed like something to avoid... |
Change-type: patch