oui
oui copied to clipboard
[BUG] `scripts/test-staged.js` raise error when commit only include a Delete operation
Describe the bug
Reviewing the base code, I realised that when you commit new changes, scripts/test-staged.js only considers files that are Added, Copied, Modified or Renamed but never a Deleted. When trying to commit a change that only has a Delete, the variable stagedFiles is an empty list (['']) and then the testing command from jest with flag --findRelatedTests complains because no location is given.
To Reproduce Steps to reproduce the behaviour:
- Add a txt file in the root folder with some text (e.g:
Hello World) - Add Added changes in the staging area and the commit (
git add .andgit -sm "adding random txt file) - After the previous commit, Delete the txt file created.
- Add Delete changes in the staging area and the commit (
git add .andgit -sm "adding random txt file) - See error.
Expected behavior
When committing you should not see an error from jest as described below
Error Message
Here below is the error I get:
oui git:(docs/modal-width) ✗ git commit -sm "removing file modal_width.js"
yarn run v1.22.19
$ yarn tsc --noEmit && yarn lint-es && yarn lint-sass
$ '/Users/samuel/Project Code/OpenSearch_Initiative_Programme/oui/node_modules/.bin/tsc' --noEmit
$ eslint --cache "{src,src-docs}/**/*.{ts,tsx,js}" --max-warnings 0
$ sass-lint -v --max-warnings 0
:sparkles: Done in 8.29s.
yarn run v1.22.19
$ cross-env NODE_ENV=test jest --config ./scripts/jest/config.json --findRelatedTests
Usage: jest [--config=<pathToConfigFile>] [TestPathPattern]
Options:
...<Set of Options>
Documentation: https://jestjs.io/
The --findRelatedTests option requires file paths to be specified.
Example usage: jest --findRelatedTests ./src/source.js ./src/index.js.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
node:child_process:960
throw err;
^
Error: Command failed: yarn test-unit --findRelatedTests
at checkExecSyncError (node:child_process:885:11)
at execSync (node:child_process:957:15)
at Object.<anonymous> (/Users/samuel/Project Code/OpenSearch_Initiative_Programme/oui/scripts/test-staged.js:20:1)
at Module._compile (node:internal/modules/cjs/loader:1254:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1308:10)
at Module.load (node:internal/modules/cjs/loader:1117:32)
at Module._load (node:internal/modules/cjs/loader:958:12)
at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
at node:internal/main/run_main_module:23:47 {
status: 1,
signal: null,
output: [ null, null, null ],
pid: 91528,
stdout: null,
stderr: null
}
Node.js v18.16.0
pre-commit:
pre-commit: We've failed to pass the specified git pre-commit hooks as the `test-staged`
pre-commit: hook returned an exit code (1). If you're feeling adventurous you can
pre-commit: skip the git pre-commit hooks by adding the following flags to your commit:
pre-commit:
pre-commit: git commit -n (or --no-verify)
pre-commit:
pre-commit: This is ill-advised since the commit is broken.
pre-commit:
Host/Environment (please complete the following information):
- OUI Version: 1.3.0
- OS: macOS Ventura 13.5.2
Most likely fix is just gating the execSync behind a "if the list isn't empty"
Also, would be nice to add D to the diff filter: https://git-scm.com/docs/git-diff#Documentation/git-diff.txt---diff-filterACDMRTUXB82308203
I would like to work on this. Thanks!