Fix uvu test pattern #402
Fix uvu test pattern #402
Conversation
packages/kit/package.json
Outdated
| @@ -50,7 +50,7 @@ | |||
| "format": "prettier --write . --config ../../.prettierrc --ignore-path .gitignore", | |||
| "check-format": "prettier --check . --config ../../.prettierrc --ignore-path .gitignore", | |||
| "prepublishOnly": "npm run build", | |||
| "test": "uvu src \"(spec.mjs|test/index.mjs)\"" | |||
| "test": "uvu src \"(spec\\.mjs|test/index\\.mjs|test\\\\index\\.mjs)\"" | |||
There was a problem hiding this comment.
I wonder if we should make this shorter by doing something like test[/\\\\]index\\.mjs instead of test/index\\.mjs|test\\\\index\\.mjs?
There was a problem hiding this comment.
Oh, yep, that should work as well. I'll update the PR.
There was a problem hiding this comment.
@benmccann Interestingly, it fails on Linux instead! I guess I'll revert to the original.
There was a problem hiding this comment.
If I use [\\\\/] as the character class it passes on Linux. It looks like you have to switch the order of the slash and backslash or it thinks you're trying to escape the ]
There was a problem hiding this comment.
If I swap to [\\\\/], it looks like it reverts back to the bugged behavior on Windows. If I do [\\\\\\\\/] it does the same, so it seems to be collapsing down to [\/] which is equivalent to /.
I think uvu is doing something strange with the regex here.
There was a problem hiding this comment.
Nope, that results in uvu src "(spec\.mjs|test/index[/\\\\].mjs)" but still skips over api/test/index.mjs.
There was a problem hiding this comment.
pnpm test, yarn test, and npm test all produce the same results, so the behavior is at least consistent across them.
There was a problem hiding this comment.
I would say it's better to duplicate that then instead of having to insert some obscure combination of \ and /.
There was a problem hiding this comment.
Wait, nevermind, PEBCAK, I just noticed that I've been replacing the dot before mjs instead of the path separator. "test": "uvu src \"(spec\\.mjs|test[\\\\/]index\\.mjs)\"" does work.
There was a problem hiding this comment.
The initial run of the latest change failed at an unrelated test:
> [email protected] test /home/runner/work/kit/kit/test/apps/basics
> node test
FAIL dev "announces client-side navigation [js]"
Expected values to be deeply equal: (equal)
++Navigated·to·b (Expected)
-- (Actual)
^^^^^^^^^^^^^^
at assert (file:///home/runner/work/kit/kit/node_modules/.pnpm/[email protected]/node_modules/uvu/assert/index.mjs:31:8)
Re-running the Github action successfully passed all tests.
I wonder if a race condition is involved here? There should be no effect on test/apps/basics at all.
This reverts commit 528479e. Cause: Invalid regex on Linux systems.
Fixes #401: incorrect test pattern (especially on Windows)