fix(test): use path.sep for cross-platform path assertions - #1452
Conversation
The 'should rename the destination directory' test hardcodes '/' as the
path separator in its assertions. On Windows, file paths use '\', so
indexOf('/testforme-') returns -1 and the test fails even though the
upload directory was correctly created.
Replace the hardcoded '/' with path.sep so the assertion holds on both
POSIX and Windows.
Fixes expressjs#1451
Co-Authored-By: EvoX <evox@evomap.ai>
MohammedAlkindi
left a comment
There was a problem hiding this comment.
Ran this on a real Windows 11 box (Node 24.18), since the fork-PR test workflow here is approval-gated and hasn't run:
The only change in the failing set is Functionality > should rename the destination directory to a different directory — exactly the test #1451 reports — which fails on main on the hardcoded '/testforme-' assertion and passes here. No other test changes state in either direction.
The fix itself is right for what the assertion means: disk storage produces ...\testforme-<random> on Windows, so path.sep + 'testforme-' checks the same "directory boundary + prefix" property on both platforms, and indexOf with a literal separator has no escaping pitfalls.
For context on the remaining 11: they are a different Windows breakage — Git rewriting the text fixtures to CRLF on checkout, so the byte-length assertions drift (1803 !== 1778, 128 !== 122). That class is what #1450 addresses; it's untouched by (and doesn't interfere with) this PR.
Summary
Fixes #1451.
The
should rename the destination directory to a different directorytest intest/functionality.jshardcodes/as the path separator in its assertions:On Windows,
pathuses\as the separator, soindexOf('/testforme-')returns-1and the test fails even though the upload directory was correctly created byfs-temp.Fix
Replace the hardcoded
'/'withpath.sepso the assertion holds on both POSIX and Windows:A
require('path')is added at the top of the file. No production code is changed — only the test assertion.Test plan
npx mocha --exit --timeout 30000 test/functionality.json Windows:should rename the destination directory to a different directory✔ passes (was failing before on Windows)should rename the uploaded file✔should ensure all req.files values (single-file per field) point to an array✔should ensure all req.files values (multi-files per field) point to an array✔lib/,storage/,index.js) touchedNotes
test/files/small0.datis a text fixture with LF line endings, but the repo has no.gitattributespinning it to binary/LF. Git's autocrlf on Windows converts LF to CRLF, inflating the file from 1778 to 1803 bytes, which causes the unrelatedshould upload the file to the dest dirtest to fail onassert.strictEqual(util.fileSize(req.file.path), 1778). That is out of scope for Failing test on Windows: functionality.js hardcodes forward slash #1451 (which is specifically about the path separator) and is not addressed by this PR to keep the diff focused.