Add rush test command #60
Conversation
Changed since introduction of typescript tests which create the directory before.
Avoid warnings about output in stderr
| echo Testing $project | ||
| (cd $project && npm test) || exit 1 | ||
| done | ||
| command: node common/scripts/install-run-rush.js test --verbose |
There was a problem hiding this comment.
This is much nicer. Might even work with Windows!
| @@ -7,7 +7,7 @@ | |||
| ], | |||
| "description": "", | |||
| "scripts": { | |||
| "build": "rm -rf dist && tsc && cp -r src/test/backend/ dist/test/", | |||
| "build": "rm -rf dist && tsc && cp -r src/test/backend dist/test/", | |||
There was a problem hiding this comment.
Don't modern tscs clean up enough to do this nicely and quickly?
There was a problem hiding this comment.
tsc does not clean up, so various confusing files collect up over time.
There was a problem hiding this comment.
But the behaviour this implements is confusing Seems like we're trying to fix microsoft/TypeScript#30602 en passant. It will confuse various tools (such as anything that looks at file mtimes. It discourages tsc -w.
I don't want to second-guess TypeScript.
There was a problem hiding this comment.
It is not an uncommon pattern (e.g. https://github.com/ReactiveX/rxjs-tslint/blob/11d64176b717c5029c212d883bfc2dfc5d14d06d/package.json#L14), I just copied it from somewhere - this is just the most paranoid version of starting from a clean slate. I run tsc -w without cleaning directory during development but I agree it's not the best solution.
| { | ||
| "changes": [ | ||
| { | ||
| "comment": "", |
There was a problem hiding this comment.
A dummy rush change since this isn't really a patch change which requires a release and only the testing infrastructure.
| "commandKind": "bulk", | ||
| "name": "test", | ||
| "summary": "Run all tests", | ||
| "enableParallelism": false |
There was a problem hiding this comment.
Why not? (It would be disappointing if Rush is incapable of re-ordering the output to get this to print errors nicely.)
There was a problem hiding this comment.
It is disappointing, really. I documented this and other reasons why it's serial now.
| "commandKind": "bulk", | ||
| "name": "test", | ||
| "summary": "Run all tests", | ||
| "enableParallelism": false |
| @@ -7,7 +7,7 @@ | |||
| ], | |||
| "description": "", | |||
| "scripts": { | |||
| "build": "rm -rf dist && tsc && cp -r src/test/backend/ dist/test/", | |||
| "build": "rm -rf dist && tsc && cp -r src/test/backend dist/test/", | |||
There was a problem hiding this comment.
why did you make this change? it can cause a dist/test/backend/backend to be created
There was a problem hiding this comment.
This was a bug - the tests did not pass on mac before. See this example - for a cross-platform copying of a directory contents need to cp -a source/. destination, or in this example cp -r src/test/backend/. dist/test/backend - this only writes backend once by omitting the trailing slash which causes the Mac OS behavior.
| @@ -7,7 +7,7 @@ | |||
| ], | |||
| "description": "", | |||
| "scripts": { | |||
| "build": "rm -rf dist && tsc && cp -r src/test/backend/ dist/test/", | |||
| "build": "rm -rf dist && tsc && cp -r src/test/backend dist/test/", | |||
There was a problem hiding this comment.
But the behaviour this implements is confusing Seems like we're trying to fix microsoft/TypeScript#30602 en passant. It will confuse various tools (such as anything that looks at file mtimes. It discourages tsc -w.
I don't want to second-guess TypeScript.
Changes to #51 - replace bash with a rush command.