Repository navigation
fix(tests): Windows paths + CI + timeout fix - #5323
Conversation
Jocs
left a comment
There was a problem hiding this comment.
Thanks for tracking this down. The Windows failure in move-image-to-folder.spec.ts is real: swapping in path.win32 makes 5 of its 7 tests fail locally. I think the fix should go the other way, though.
Root cause: the spec stubs window.path with Node's path, but the preload exposes pathe (packages/desktop/src/preload/index.ts), which always uses /. On Windows, Node's path is win32, so the spec tests a setup the app never runs. Here is the same Windows input through both:
preload (pathe): "assets/7037807….png" ← what a Windows user actually gets
spec stub (win32): "assets\\7037807….png" ← what this PR now asserts on Windows
With assets${path.sep}, the Windows test accepts a backslash reference. The app never produces one, and it isn't a portable Markdown reference, which is what these tests are meant to guard. os.tmpdir() isn't needed either: fileUtils is fully mocked, so these paths are only strings.
Suggested change: stub with the library the preload uses, as resolveLinkHref.spec.ts already does. The original literals and 'assets/' assertions can stay as they were:
-import path from 'path'
+import path from 'pathe'Also update the header comment that says "the real node path". With this change, all 7 tests pass on POSIX and with path.win32, and vue-tsc passes.
The CI matrix and the pdf timeout bump look good to me. Please keep those.
The first dynamic import pays the module-transform cost, which exceeds the 5s default under full-suite parallel load. A timeout here also lets this test's 'delete window.marktext' fall through into the next test and fail it as collateral.
386fde9 to
e2e1323
Compare
e2e1323 to
454bfde
Compare
Jocs
left a comment
There was a problem hiding this comment.
Thanks for turning it around — this is exactly it.
pathe in the stub means the spec now exercises the same path implementation the preload actually exposes, so the 'assets/' assertions hold on both platforms and keep guarding the portable reference. Verified locally: 7/7 in move-image-to-folder.spec.ts, 25/25 across both touched specs, and CI is green on the new test (windows-latest) job.
The matrix and the pdf timeout bump are keepers. Merging.
Follow-up to #5323, which added a `windows-latest` leg to the unit-test matrix. GitHub's Windows runner images set `core.autocrlf=true` globally and this repo declared no attributes, so as of that PR the two legs no longer test the same bytes: every text file arrives LF on Linux and CRLF on Windows. Several specs read tracked source files and parse them as text. source-code-image-action.spec.ts and search-prefill.spec.ts read a .vue, compile it, then split the output on '\n' to strip import lines; empty-state-button-contrast.spec.ts regex-matches the components' scoped CSS and the theme stylesheets to pull custom-property names. They pass on Windows today only because those particular patterns happen to tolerate a trailing '\r'. The next assertion that does not — an `m`-flagged `$`, which will not match before a `\r\n`, a `toBe` on a split line, a snippet holding a literal '\n', a snapshot — goes red on Windows alone, with a cause that is invisible in a diff and unreproducible on macOS or Linux. test/unit/markdown.ts already carries an explicit `.replace(/(?:\r\n|\n)/g, '\n')` from the last time this cost someone an afternoon; that fix was local to one helper and left the class open. `* text=auto eol=lf` normalizes on commit and checks out LF everywhere, closing the class in one line, and keeps a Windows contributor's editor from turning a whole file into a diff on save. Nothing in the tree needs CRLF: there are no .bat, .cmd or .ps1 files, and the single NSIS include, packages/desktop/build/windows/installer.nsh, reads LF fine. Two vendored Open Sans licence texts had been committed with CRLF, so their blobs no longer matched what git would store for the same working-tree content; left alone they surface as spurious modifications the first time git compares by content rather than by stat cache. `git add --renormalize` fixes both, and the change is CR-at-EOL only — `git diff --ignore-cr-at-eol` across it is empty, no blob in the tree now contains a CR at end of line, and the working tree is clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up to #5323, which added a `windows-latest` leg to the unit-test matrix. The job sets no `timeout-minutes` and so inherits GitHub's 360-minute default, unlike muya-spec.yml (15) and muya-e2e.yml (20); it was simply never given one back when it was a single 56-second Linux job. It now also runs on Windows, the platform where a worker that never exits is most plausible — an unreleased file watcher or an EBUSY rename, the same fs semantics that already force `it.skipIf(process.platform === 'win32')` in write-file-atomic.spec.ts. One hang would leave `test (windows-latest)` pending for six hours while `fail-fast: false` lets the Linux leg report green beside it, which reads as a slow queue rather than a failure; it would hold a runner from a concurrency pool shared with every other PR, and `cancel-in-progress` frees it only once someone pushes to the same branch again. Ten minutes turns that into a clear red check, and is four times the slowest run observed so far (2m25s on windows-latest, 56s on ubuntu-latest). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Summary
On Windows tests were failing due to hardcoded linux paths (e.g. separator /, , temp folder location). I used llm to convert literals to os specific paths.
Also increased a timeout for loading pdf (util/pdf import) test which was taking 10s on my laptop with tests running in parallel.
Type of change
Notes for reviewers [optional]
All tests pass on Windows.
By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.