Niedzielski has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/370944 )
Change subject: Chore: improve test performance
......................................................................
Chore: improve test performance
The hypothesis for poor performance is numerous shell invocations. This
is a well-known problem in recursive Make so the proposed explanation is
that shelling out in NPM for each script is causing a similar impedance.
Three solutions seemed immediately practical:
- NPM: repeat commands instead of reinvoking NPM.
- Make: has a nice rule syntax.
- Shell: simple and the interpreter NPM is already calling.
The NPM solution is the most straightforward but may cause bugs since
code could fall out of sync and reduces readability. There are numerous
pros and cons to Make vs shell. However, the Make solution would require
rather verbose function definitions to avoid subshells and ultimately
boil down to shell script anyway. Only the shell solution was explored.
This implementation replaces the package.json scripts with a new shell
script, marvin. The shell script's commands are almost one-to-one with
the previous package.json implementation but now have the benefit of
being able to share commands without shelling.
Notes:
- This solution adds a layer of indirection to development that is
undesirable.
- nodemon was used insead of `dev:client& dev:server& wait` to avoid
child process trapping and job crossplatform monitoring concerns.
- /dev/null is monitored instead of [watch:false] as this option doesn't
appear to be exposed on the command line. An alternative would be to
watch the likely nonexistent file, "false".
- A big switch statement is used which is very simple, concise, and
easily extended and maintained.
- Aliases were considered but overriding shell builtins like `test` is
a bad idea and dynamically invoking an alias from a variable
required wrapping it in a function first,
`eval "_$COMMAND() { $COMMAND \"\$@\"; }"` (direct evaluation
requires escaping the parameters via `printf '%q ' "$@"` but that is
a Bashism). Aliases do have the nice property of being easily
queried, `alias|sed "s%=.*%%"`.
- Functions were also considered but since colons in function names
are forbidden in shell, commands must be translated,
`COMMAND="$(echo "$1"|sed 's%:%_%g')"`.
Bug: T172862
Change-Id: Ie4f5369393af3468da1520fd849a40ca5cf98e62
[watch:false]: https://github.com/remy/nodemon/issues/516
---
A marvin
M package.json
M readme.md
3 files changed, 29 insertions(+), 15 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/marvin refs/changes/44/370944/1
diff --git a/marvin b/marvin
new file mode 100755
index 0000000..0d295d0
--- /dev/null
+++ b/marvin
@@ -0,0 +1,24 @@
+#!/usr/bin/env sh
+set -eu
+
+execute() {
+ local command="$1"
+ shift
+
+ case "$command" in
+ start) nodemon -q -w /dev/null -x "\"$0\" dev:client\"" -x "\"$0\"
dev:server";;
+ dev:client) NODE_ENV=development webpack -w;;
+ dev:server) NODE_ENV=development nodemon -w dist/client -w src/server/
src/server/index.js;;
+ prod:build) NODE_ENV=production webpack -p;;
+ format) execute lint --fix "$@";;
+ format:all) execute format '{src,test}/**/*.js';;
+ lint) eslint --cache --max-warnings 0 "$@";;
+ lint:all) execute lint '{src,test}/**/*.js';;
+ test) execute lint:all && mocha '{src,test}/**/*.test.js';;
+ test:watch) nodemon -q -w src -w test -x "\"$0\" test";;
+ precommit) execute test;;
+ *) ! :;;
+ esac
+}
+
+execute "$@"
diff --git a/package.json b/package.json
index f7be2a2..bad01d6 100644
--- a/package.json
+++ b/package.json
@@ -3,18 +3,8 @@
"version": "0.0.0",
"description": "An API driven skin for MediaWiki",
"scripts": {
- "start": "run-p -sn dev:\\*",
- "dev:client": "NODE_ENV=development webpack -w",
- "dev:server": "NODE_ENV=development nodemon -w dist/client -w src/server/
src/server/index.js",
- "prod:build": "NODE_ENV=production webpack -p",
- "format": "npm run lint -s -- --fix",
- "format:all": "npm run format -s -- '{src,test}/**/*.js'",
- "lint": "eslint --cache --max-warnings 0",
- "lint:all": "npm run lint -s -- '{src,test}/**/*.js'",
- "pretest": "npm run lint:all -s",
- "test": "mocha '{src,test}/**/*.test.js'",
- "test:watch": "nodemon -w src -w test -x 'npm test'",
- "precommit": "npm run test -s"
+ "test": "./marvin test",
+ "precommit": "./marvin test"
},
"repository": {
"type": "git",
diff --git a/readme.md b/readme.md
index 1eca95a..356eaff 100644
--- a/readme.md
+++ b/readme.md
@@ -9,9 +9,9 @@
Marvin's requirements are listed in package.json **engines**. Generally the
latest Node.js active LTS version, and npm > 5.X should do it.
-For running the project on development, run `npm install` and then `npm start`.
-That will run the server in development mode with file watching which will auto
-restart the server when files change.
+For running the project on development, run `npm install` and then
+`./marvin start`. That will run the server in development mode with file
+watching which will auto restart the server when files change.
## Documentation
--
To view, visit https://gerrit.wikimedia.org/r/370944
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie4f5369393af3468da1520fd849a40ca5cf98e62
Gerrit-PatchSet: 1
Gerrit-Project: marvin
Gerrit-Branch: master
Gerrit-Owner: Niedzielski <sniedziel...@wikimedia.org>
Gerrit-Reviewer: Sniedzielski <sniedziel...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
