From 320399bc08525b2709b4de26270a6453b0089d76 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Sun, 25 Oct 2026 09:41:27 +0100 Subject: [PATCH 1/2] Run the tests skipped on Windows in Windows CI Windows CI ran only the GenType compiler unit tests and skipped the runtime docstrings, analysis, tools and gentype tests and the unicode and duplicated_symlinked_packages build tests. They now run there. The async overlap unit test waited for its domains by spinning, which starved the spawning domain on few cores; its domains now sleep. The analysis harness writes its source copy in binary mode, the analysis and duplicate-package tests normalize Windows path separators, the tools test passes cmd.exe a backslash PPX path, and CI checks out symlinks and installs make. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Christoph Knittel --- .github/workflows/ci.yml | 27 +++++++++----- analysis/src/cli.ml | 5 ++- changes/windows-ci-tests.internal.md | 1 + lib_dev/paths.js | 7 ++-- scripts/test.js | 36 +++---------------- tests/analysis_tests/tests/test.sh | 4 +++ .../duplicated_symlinked_packages/input.js | 18 ++++++++-- tests/build_tests/unicode/input.js | 5 --- .../ounit_compiler_request_tests.ml | 6 ++-- tests/tools_tests/test.sh | 9 ++++- 10 files changed, 63 insertions(+), 55 deletions(-) create mode 100644 changes/windows-ci-tests.internal.md diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 630f6b7ff9c..59a7cb695b1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -91,6 +91,7 @@ jobs: git config --system core.autocrlf false git config --system core.eol lf git config --system core.longpaths true + git config --system core.symlinks true - name: Checkout uses: actions/checkout@v7 @@ -205,20 +206,28 @@ jobs: - name: Check for diffs in tests folder run: git diff --ignore-cr-at-eol --exit-code tests + # On Windows, after running setup-ocaml (if it wasn't cached yet or the cache couldn't be restored), + # Cygwin bash is used instead of Git Bash for Windows, breaking the test scripts below. + # So we need to adjust the path to bring back Git Bash for Windows. + - name: "Windows: Use Git Bash" + if: ${{ runner.os == 'Windows' }} + run: echo "C:\Program Files\Git\bin" >> $GITHUB_PATH + shell: bash + + # Git Bash does not include make. + - name: "Windows: Install make" + if: ${{ runner.os == 'Windows' }} + run: command -v make || choco install make --no-progress + shell: bash + + # Linux runs these tests in the coverage workflow. - name: Run analysis / tools tests - if: runner.os != 'Windows' && runner.os != 'Linux' + if: runner.os != 'Linux' run: opam exec -- make -C tests/analysis_tests test && make -C tests/tools_tests test + shell: bash - name: Run gentype tests - if: runner.os != 'Windows' run: make -C tests/gentype_tests/typescript-react-example clean test - - # On Windows, after running setup-ocaml (if it wasn't cached yet or the cache couldn't be restored), - # Cygwin bash is used instead of Git Bash for Windows, breaking the rewatch tests. - # So we need to adjust the path to bring back Git Bash for Windows. - - name: Rewatch tests need Git Bash for Windows - if: ${{ runner.os == 'Windows' }} - run: echo "C:\Program Files\Git\bin" >> $GITHUB_PATH shell: bash - name: Run OCaml rewatch parity tests diff --git a/analysis/src/cli.ml b/analysis/src/cli.ml index 2c6f843f5a0..18299e95d07 100644 --- a/analysis/src/cli.ml +++ b/analysis/src/cli.ml @@ -209,8 +209,11 @@ let test ~state ~path = let lines = text |> String.split_on_char '\n' in let process_line i line = let create_current_file () = + (* Binary mode keeps the copy's LF line endings on Windows, so dumped + source text matches the original file. *) let current_file, cout = - Filename.open_temp_file "def" ("txt." ^ Filename.extension path) + Filename.open_temp_file ~mode:[Open_binary] "def" + ("txt." ^ Filename.extension path) in let remove_line_comment l = let len = String.length l in diff --git a/changes/windows-ci-tests.internal.md b/changes/windows-ci-tests.internal.md new file mode 100644 index 00000000000..0ec69221109 --- /dev/null +++ b/changes/windows-ci-tests.internal.md @@ -0,0 +1 @@ +- Run the full compiler unit tests, runtime docstring tests, analysis, tools and gentype tests, and the `unicode` and `duplicated_symlinked_packages` build tests on Windows CI. diff --git a/lib_dev/paths.js b/lib_dev/paths.js index b0539d09e5f..b974e6febf9 100644 --- a/lib_dev/paths.js +++ b/lib_dev/paths.js @@ -72,6 +72,9 @@ export const compilerBinDir = path.resolve( ); /** - * path: `/_build/install/default/bin/ounit_tests` + * path: `/_build/install/default/bin/ounit_tests[.exe]` */ -export const ounitTestBin = path.join(compilerBinDir, "ounit_tests"); +export const ounitTestBin = path.join( + compilerBinDir, + process.platform === "win32" ? "ounit_tests.exe" : "ounit_tests", +); diff --git a/scripts/test.js b/scripts/test.js index 79ca4569583..bd76a7479a3 100644 --- a/scripts/test.js +++ b/scripts/test.js @@ -64,35 +64,9 @@ if (formatTest) { } if (ounitTest) { - if (process.platform === "win32") { - // GenType's drive-root regression needs the native Windows Filename module. - const binary = path.join( - projectDir, - "_build/default/tests/ounit_tests/ounit_tests_main.exe", - ); - const { stdout } = await execBin(binary, ["-list-test"], { - shell: false, - throwOnFail: true, - }); - const tests = stdout - .split(/\r?\n/) - .filter(test => test.includes(":gentype:")); - if (tests.length === 0) { - throw new Error("No GenType unit tests found"); - } - await execBin( - binary, - tests.flatMap(test => ["-only-test", test]), - { - stdio: "inherit", - shell: false, - }, - ); - } else { - await execBin(ounitTestBin, [], { - stdio: "inherit", - }); - } + await execBin(ounitTestBin, [], { + stdio: "inherit", + }); } if (mochaTest) { @@ -197,9 +171,7 @@ if (buildTest) { } if (runtimeDocstrings) { - if (process.platform === "win32") { - console.log(`Skipping docstrings tests on ${process.platform}`); - } else if (process.platform === "darwin" && os.release().startsWith("22")) { + if (process.platform === "darwin" && os.release().startsWith("22")) { // Workaround for intermittent hangs in CI console.log("Skipping docstrings tests on macOS 13"); } else { diff --git a/tests/analysis_tests/tests/test.sh b/tests/analysis_tests/tests/test.sh index 993782b8678..592d3a6d472 100755 --- a/tests/analysis_tests/tests/test.sh +++ b/tests/analysis_tests/tests/test.sh @@ -4,6 +4,8 @@ for file in src/*.{res,resi}; do # CI. We use LF, and the CI OCaml fork prints CRLF. Convert. if [ "$RUNNER_OS" == "Windows" ]; then perl -pi -e 's/\r\n/\n/g' -- $output + # Completion data holds a native path. Use the snapshots' separator. + perl -pi -e 's{(?<="filePath": ")[^"]*}{$& =~ s#\\\\#/#gr}ge' -- $output fi done @@ -13,6 +15,8 @@ for file in not_compiled/*.{res,resi}; do # CI. We use LF, and the CI OCaml fork prints CRLF. Convert. if [ "$RUNNER_OS" == "Windows" ]; then perl -pi -e 's/\r\n/\n/g' -- $output + # Completion data holds a native path. Use the snapshots' separator. + perl -pi -e 's{(?<="filePath": ")[^"]*}{$& =~ s#\\\\#/#gr}ge' -- $output fi done diff --git a/tests/build_tests/duplicated_symlinked_packages/input.js b/tests/build_tests/duplicated_symlinked_packages/input.js index 419ac248b13..e41287dcba3 100644 --- a/tests/build_tests/duplicated_symlinked_packages/input.js +++ b/tests/build_tests/duplicated_symlinked_packages/input.js @@ -1,6 +1,8 @@ // @ts-check import * as assert from "node:assert"; +import * as fs from "node:fs/promises"; +import * as path from "node:path"; import { runtimePath } from "#cli/runtime"; import { setup } from "#dev/process"; @@ -9,13 +11,23 @@ process.env.RESCRIPT_RUNTIME = runtimePath; const { execBuild, execClean } = setup(import.meta.dirname); -if (process.platform === "win32") { - console.log("Skipping test on Windows"); +// Without core.symlinks, Git on Windows checks the links out as plain files. +// CI enables it, so only local checkouts may skip. +const link = await fs.lstat( + path.join(import.meta.dirname, "node_modules", "a"), +); +if (!link.isSymbolicLink()) { + if (process.env.CI) { + assert.fail("node_modules/a was not checked out as a symlink"); + } + console.log("Skipping test: symlinks are not checked out as symlinks"); process.exit(0); } await execClean(); -const { stderr } = await execBuild(); +const { stderr: rawStderr } = await execBuild(); +// Windows joins the package paths with backslashes. +const stderr = rawStderr.replaceAll("\\", "/"); // c reaches a and b through node_modules symlinks and counts once; z has two // copies, node_modules/z and a/node_modules/z, the one duplicate to report. diff --git a/tests/build_tests/unicode/input.js b/tests/build_tests/unicode/input.js index 7c68350b899..65693376620 100644 --- a/tests/build_tests/unicode/input.js +++ b/tests/build_tests/unicode/input.js @@ -6,11 +6,6 @@ import { setup } from "#dev/process"; const { execBuildOrThrow, execClean } = setup(import.meta.dirname); -if (process.platform === "win32") { - console.log("Skipping test on Windows"); - process.exit(0); -} - await execBuildOrThrow(); await fs.access( diff --git a/tests/ounit_tests/ounit_compiler_request_tests.ml b/tests/ounit_tests/ounit_compiler_request_tests.ml index 8768304d41c..e797dd7404d 100644 --- a/tests/ounit_tests/ounit_compiler_request_tests.ml +++ b/tests/ounit_tests/ounit_compiler_request_tests.ml @@ -1604,9 +1604,11 @@ let concurrent_async_context_tests _context = write directory "Sync.res" sync_source; Domain.spawn (fun () -> ignore (Atomic.fetch_and_add ready 1); - let deadline = Unix.gettimeofday () +. 5. in + (* Sleep rather than spin: each spawn stops the world, and spinning + domains outnumbering the cores starve the domain that spawns. *) + let deadline = Unix.gettimeofday () +. 30. in while Atomic.get ready < 8 && Unix.gettimeofday () < deadline do - Domain.cpu_relax () + Unix.sleepf 0.001 done; check (Atomic.get ready = 8) "async requests did not overlap"; for _ = 1 to 3 do diff --git a/tests/tools_tests/test.sh b/tests/tools_tests/test.sh index 0a735d74b0f..928e575663f 100755 --- a/tests/tools_tests/test.sh +++ b/tests/tools_tests/test.sh @@ -13,9 +13,16 @@ for file in src/*.{res,resi}; do fi done +# bsc runs the PPX command through the system shell, and cmd.exe needs +# backslashes in the program path. +ppx_command="../../_build/install/default/bin/rescript-tools ppx" +if [ "${RUNNER_OS:-}" == "Windows" ]; then + ppx_command='..\..\_build\install\default\bin\rescript-tools.exe ppx' +fi + for file in ppx/*.res; do output="src/expected/$(basename $file).jsout" - ../../cli/bsc.js -ppx "../../_build/install/default/bin/rescript-tools ppx" $file > $output + ../../cli/bsc.js -ppx "$ppx_command" $file > $output if [ "${RUNNER_OS:-}" == "Windows" ]; then perl -pi -e 's/\r\n/\n/g' -- $output fi From 3ca999e2ddb3d6c7cffc1b76e81aa39e2f013832 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Sun, 25 Oct 2026 11:59:09 +0100 Subject: [PATCH 2/2] Run the test scripts through bash in the test Makefiles The analysis and tools Makefiles ran their scripts as ./test.sh. Make runs a recipe line without shell syntax directly rather than through SHELL, and Windows cannot run a shell script as a program, so Windows CI failed with error 193. The recipes now invoke bash, which several shebang-less scripts needed for brace expansion anyway. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Christoph Knittel --- tests/analysis_tests/tests-generic-jsx-transform/Makefile | 2 +- tests/analysis_tests/tests-incremental-typechecking/Makefile | 2 +- tests/analysis_tests/tests-namespaced-references/Makefile | 2 +- .../analysis_tests/tests-reanalyze/deadcode-benchmark/Makefile | 2 +- tests/analysis_tests/tests-reanalyze/deadcode/Makefile | 2 +- tests/analysis_tests/tests-reanalyze/termination/Makefile | 2 +- tests/analysis_tests/tests-sourcedirs-dependency/Makefile | 2 +- tests/analysis_tests/tests/Makefile | 2 +- tests/tools_tests/Makefile | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/analysis_tests/tests-generic-jsx-transform/Makefile b/tests/analysis_tests/tests-generic-jsx-transform/Makefile index 4d26df85bb1..d973307f858 100644 --- a/tests/analysis_tests/tests-generic-jsx-transform/Makefile +++ b/tests/analysis_tests/tests-generic-jsx-transform/Makefile @@ -4,7 +4,7 @@ build: yarn build test: build - ./test.sh + bash ./test.sh clean: yarn clean diff --git a/tests/analysis_tests/tests-incremental-typechecking/Makefile b/tests/analysis_tests/tests-incremental-typechecking/Makefile index 6abbf0dd984..3b3d9b0c665 100644 --- a/tests/analysis_tests/tests-incremental-typechecking/Makefile +++ b/tests/analysis_tests/tests-incremental-typechecking/Makefile @@ -4,7 +4,7 @@ build: yarn build > /dev/null 2>&1 || true test: build - ./test.sh + bash ./test.sh clean: yarn clean diff --git a/tests/analysis_tests/tests-namespaced-references/Makefile b/tests/analysis_tests/tests-namespaced-references/Makefile index 2b663dd3475..dfc867973c7 100644 --- a/tests/analysis_tests/tests-namespaced-references/Makefile +++ b/tests/analysis_tests/tests-namespaced-references/Makefile @@ -10,7 +10,7 @@ build: cp lib/bs/MyNamespace.cmt lib/bs/___incremental/MyNamespace.cmt test: build - ./test.sh + bash ./test.sh clean: yarn clean diff --git a/tests/analysis_tests/tests-reanalyze/deadcode-benchmark/Makefile b/tests/analysis_tests/tests-reanalyze/deadcode-benchmark/Makefile index 85b3f706db2..150435a2295 100644 --- a/tests/analysis_tests/tests-reanalyze/deadcode-benchmark/Makefile +++ b/tests/analysis_tests/tests-reanalyze/deadcode-benchmark/Makefile @@ -5,7 +5,7 @@ COPIES ?= 50 generate: chmod +x generate.sh - ./generate.sh $(COPIES) + bash ./generate.sh $(COPIES) node_modules/.bin/rescript: yarn install diff --git a/tests/analysis_tests/tests-reanalyze/deadcode/Makefile b/tests/analysis_tests/tests-reanalyze/deadcode/Makefile index 2a1c1a4b95a..24381b4f456 100644 --- a/tests/analysis_tests/tests-reanalyze/deadcode/Makefile +++ b/tests/analysis_tests/tests-reanalyze/deadcode/Makefile @@ -4,7 +4,7 @@ build: yarn build test: build node_modules/.bin/rescript - ./test.sh + bash ./test.sh # Order-independence test for reanalyze - not run by default (takes longer) # Verifies that processing files in different orders produces identical results diff --git a/tests/analysis_tests/tests-reanalyze/termination/Makefile b/tests/analysis_tests/tests-reanalyze/termination/Makefile index 18fcea0dc88..03e13de6fc1 100644 --- a/tests/analysis_tests/tests-reanalyze/termination/Makefile +++ b/tests/analysis_tests/tests-reanalyze/termination/Makefile @@ -4,7 +4,7 @@ build: yarn build test: build - ./test.sh + bash ./test.sh clean: yarn clean diff --git a/tests/analysis_tests/tests-sourcedirs-dependency/Makefile b/tests/analysis_tests/tests-sourcedirs-dependency/Makefile index 4d26df85bb1..d973307f858 100644 --- a/tests/analysis_tests/tests-sourcedirs-dependency/Makefile +++ b/tests/analysis_tests/tests-sourcedirs-dependency/Makefile @@ -4,7 +4,7 @@ build: yarn build test: build - ./test.sh + bash ./test.sh clean: yarn clean diff --git a/tests/analysis_tests/tests/Makefile b/tests/analysis_tests/tests/Makefile index 4d26df85bb1..d973307f858 100644 --- a/tests/analysis_tests/tests/Makefile +++ b/tests/analysis_tests/tests/Makefile @@ -4,7 +4,7 @@ build: yarn build test: build - ./test.sh + bash ./test.sh clean: yarn clean diff --git a/tests/tools_tests/Makefile b/tests/tools_tests/Makefile index e8d2c1f3fc5..0105c9f9825 100644 --- a/tests/tools_tests/Makefile +++ b/tests/tools_tests/Makefile @@ -4,7 +4,7 @@ build: yarn build test: build - ./test.sh + bash ./test.sh yarn build clean: