Serialize test suites sharing build artifacts with a per-checkout lock

.gitignore

@@ -108,3 +108,9 @@ package.tgz
 
 # AI Agents
 .claude/settings.local.json
+
+# Kernel lock inode for suites sharing build artifacts; do not delete while in use.
+.rescript-test.lock
+
+# Bytecode from the Python build/test tooling in scripts/
+__pycache__/

AGENTS.md

@@ -160,6 +160,13 @@ also run `make test-syntax`; use `make test-syntax-roundtrip` when parsing or
 printing changes. Other focused suites are `make test-gentype`,
 `make test-analysis`, `make test-tools`, and `make test-rewatch`.
 
+Root `make test*` suite targets and direct `node scripts/test.js` invocations
+use a per-checkout artifact lock (currently a macOS/Linux prototype). Let a
+waiting invocation wait; do not delete `.rescript-test.lock` or bypass the
+`_locked-*` targets. See [test concurrency](CONTRIBUTING.md#test-concurrency)
+for coverage and limitations. Separate worktrees need their own build outputs
+and dependencies to run independently.
+
 ### Testing Requirements
 
 #### When to Add Tests

CONTRIBUTING.md

@@ -247,6 +247,43 @@ To run all tests:
 make test
 ```
 
+#### Test concurrency
+
+The root test-suite targets and `node scripts/test.js` acquire a shared lock
+before any build, cleanup, or test work. In particular, `make test` and
+`make test-analysis` both rebuild or clean Belt's outputs: starting them in
+parallel now makes one print `[test-lock] ... waiting for shared artifacts`
+until the other finishes. `make test-all -j` serializes its suite targets too.
+Nested runners reuse the active lock, and each suite's recursive Make invocation
+runs with `-j1` so nested `clean test` goals remain ordered.
+
+This is a macOS/Linux prototype using Python 3 and `flock`. Windows has no
+`flock`, and CI runs the suites there, so commands on Windows print
+`[test-lock] ... unlocked` and run unprotected: concurrent suites in one
+Windows checkout are not supported. Any other platform missing `flock` fails
+rather than racing silently. The lock is per physical checkout. Separate
+worktrees with independent build outputs and dependencies can run concurrently.
+Direct low-level commands such as `make lib`, `make clean`,
+`yarn workspace ... build`, and subdirectory Make invocations are not
+automatically protected. To coordinate one with a suite:
+
+```sh
+python3 scripts/with_test_lock.py --label manual-build -- make lib
+```
+
+The lock lives in `.rescript-test.lock`, outside directories cleaned by builds.
+Do not delete it while commands are running: replacing the inode would let two
+processes acquire different locks. The OS releases the lock when its command
+(and any processes inheriting its descriptor) exits, even after a crash. Finish
+child commands before leaving the protected command; do not detach test jobs.
+
+`make test` includes the process-level lock checks. To run just these checks
+(contention, nesting, failure, killed owners, and independent checkouts):
+
+```sh
+python3 scripts/test_test_lock.py
+```
+
 **Run Mocha tests only (for our runtime code):**
 
 This will run our `mocha` unit test suite defined in `tests/tests`.

Makefile

@@ -154,36 +154,47 @@ artifacts: lib
 
 # Tests
 
+# Lock before entering prerequisites: locking only the test recipe would leave
+# lib builds and suite cleanup racing. test-all's leaf targets acquire separately,
+# including under make -j. Recursive suites run serially because nested
+# clean/test goals must also remain ordered.
+LOCKED_TEST_TARGETS := test test-analysis test-reanalyze test-tools test-syntax test-syntax-roundtrip test-gentype test-rewatch
+$(LOCKED_TEST_TARGETS):
+	+python3 scripts/with_test_lock.py --label $@ -- $(MAKE) -j1 _locked-$@
+
+.PHONY: $(addprefix _locked-,$(LOCKED_TEST_TARGETS))
+
 bench: compiler
 	$(DUNE_BIN_DIR)/syntax_benchmarks
 
-test: lib
+_locked-test: lib
+	python3 scripts/test_test_lock.py
 	node scripts/test.js -all
 
-test-analysis: lib
+_locked-test-analysis: lib
 	make -C tests/analysis_tests clean test
 
-test-reanalyze: lib
+_locked-test-reanalyze: lib
 	make -C tests/analysis_tests/tests-reanalyze/deadcode test
 
 # Benchmark reanalyze on larger codebase (COPIES=N for more files)
 benchmark-reanalyze: lib
 	make -C tests/analysis_tests/tests-reanalyze/deadcode-benchmark benchmark COPIES=$(or $(COPIES),50)
 
-test-tools: lib
+_locked-test-tools: lib
 	make -C tests/tools_tests clean test
 
-test-syntax: compiler
+_locked-test-syntax: compiler
 	./scripts/test_syntax.sh
 
-test-syntax-roundtrip: compiler
+_locked-test-syntax-roundtrip: compiler
 	ROUNDTRIP_TEST=1 ./scripts/test_syntax.sh
 
-test-gentype: lib
+_locked-test-gentype: lib
 	make -C tests/gentype_tests/typescript-react-example clean test
 	make -C tests/gentype_tests/stdlib-no-shims clean test
 
-test-rewatch: lib
+_locked-test-rewatch: lib
 	./rewatch/tests/suite.sh $(RESCRIPT_EXE)
 
 test-all: test test-gentype test-analysis test-tools test-rewatch

lib_dev/test_lock.js

@@ -0,0 +1,71 @@
+// @ts-check
+
+import { spawn } from "node:child_process";
+import { readFileSync, realpathSync } from "node:fs";
+import { constants } from "node:os";
+import { fileURLToPath } from "node:url";
+
+const root = realpathSync(fileURLToPath(new URL("../", import.meta.url)));
+const wrapper = fileURLToPath(
+  new URL("../scripts/with_test_lock.py", import.meta.url),
+);
+
+/** Acquire the checkout lock before a directly invoked runner does any work. */
+export async function ensureTestLock() {
+  // Windows has no flock. CI runs this runner there, so proceed unlocked
+  // rather than failing; concurrent suites in one checkout stay unsupported.
+  if (process.platform === "win32") return;
+  try {
+    const owner = JSON.parse(process.env.RESCRIPT_TEST_LOCK ?? "null");
+    const recorded = JSON.parse(
+      readFileSync(`${root}/.rescript-test.lock`, "utf8"),
+    );
+    if (
+      owner?.root === root &&
+      owner.pid === recorded.pid &&
+      owner.token === recorded.token
+    ) {
+      process.kill(owner.pid, 0);
+      return;
+    }
+  } catch {
+    // Missing/stale ownership: acquire through the OS lock, never skip it.
+  }
+  const child = spawn(
+    "python3",
+    [
+      wrapper,
+      "--label",
+      "scripts/test.js",
+      "--",
+      process.execPath,
+      ...process.argv.slice(1),
+    ],
+    { stdio: "inherit" },
+  );
+  /** @type {NodeJS.Signals[]} */
+  const signals = ["SIGINT", "SIGTERM", "SIGHUP"];
+  const forwards = signals.map(signal => {
+    const forward = () => {
+      child.kill(signal);
+    };
+    process.on(signal, forward);
+    return { signal, forward };
+  });
+  let status;
+  try {
+    status = await new Promise(resolve => {
+      child.once("error", error => {
+        console.error(`[test-lock] ${error.message}`);
+        resolve(1);
+      });
+      child.once("exit", (code, signal) =>
+        resolve(code ?? (signal ? 128 + constants.signals[signal] : 1)),
+      );
+    });
+  } finally {
+    for (const { signal, forward } of forwards)
+      process.removeListener(signal, forward);
+  }
+  process.exit(status);
+}

scripts/test.js

@@ -10,7 +10,6 @@ import {
   ounitTestBin,
   projectDir,
 } from "#dev/paths";
-
 import {
   execBin,
   execBuild,
@@ -20,6 +19,9 @@ import {
   rescript,
   shell,
 } from "#dev/process";
+import { ensureTestLock } from "#dev/test_lock";
+
+await ensureTestLock();
 
 let ounitTest = false;
 let mochaTest = false;

scripts/test_test_lock.py

@@ -0,0 +1,159 @@
+"""Process-level tests for the shared-artifact lock prototype."""
+
+import json
+import os
+from pathlib import Path
+import selectors
+import signal
+import subprocess
+import sys
+import tempfile
+import unittest
+
+WRAPPER = Path(__file__).resolve().with_name("with_test_lock.py")
+HOLD = """
+import pathlib, sys, time
+print('START ' + sys.argv[1], flush=True)
+while not pathlib.Path(sys.argv[2]).exists():
+    time.sleep(0.02)
+print('END ' + sys.argv[1], flush=True)
+"""
+
+
+@unittest.skipUnless(os.name == "posix", "prototype uses POSIX flock")
+class TestArtifactLock(unittest.TestCase):
+    def setUp(self):
+        self.temp = tempfile.TemporaryDirectory(prefix="rescript-lock-test-")
+        self.addCleanup(self.temp.cleanup)
+        self.root = Path(self.temp.name)
+        self.children = []
+        self.addCleanup(self.stop_children)
+
+    def stop_children(self):
+        for child in self.children:
+            if child.poll() is None:
+                child.kill()
+            child.wait(timeout=5)
+            child.stdout.close()
+            child.stderr.close()
+
+    def command(self, root, label, *command):
+        return [sys.executable, str(WRAPPER), "--root", str(root),
+                "--label", label, "--", *command]
+
+    def start(self, root, label, *command, env=None):
+        child = subprocess.Popen(self.command(root, label, *command),
+                                 stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+                                 text=True, env=env)
+        self.children.append(child)
+        return child
+
+    def line(self, stream):
+        with selectors.DefaultSelector() as selector:
+            selector.register(stream, selectors.EVENT_READ)
+            self.assertTrue(selector.select(timeout=5), "timed out waiting for output")
+        return stream.readline().strip()
+
+    def hold(self, root, label):
+        release = root / (label + ".release")
+        child = self.start(root, label, sys.executable, "-c", HOLD, label, str(release))
+        return child, release
+
+    def test_competing_suites_wait_for_entire_command(self):
+        first, release_first = self.hold(self.root, "test")
+        self.assertIn("acquired", self.line(first.stderr))
+        self.assertEqual(self.line(first.stdout), "START test")
+        second, release_second = self.hold(self.root, "test-analysis")
+        self.assertIn("waiting for shared artifacts", self.line(second.stderr))
+        self.assertIsNone(second.poll())
+        release_first.touch()
+        self.assertEqual(self.line(first.stdout), "END test")
+        self.assertEqual(first.wait(timeout=5), 0)
+        self.assertIn("acquired", self.line(second.stderr))
+        self.assertEqual(self.line(second.stdout), "START test-analysis")
+        release_second.touch()
+        self.assertEqual(self.line(second.stdout), "END test-analysis")
+        self.assertEqual(second.wait(timeout=5), 0)
+
+    def test_node_command_retains_the_kernel_lock(self):
+        release = self.root / "node.release"
+        code = ("console.log('START node'); setInterval(() => {"
+                "if (require('node:fs').existsSync(process.argv[1])) process.exit(0);"
+                "}, 20)")
+        first = self.start(self.root, "node", "node", "--input-type=commonjs",
+                           "-e", code, str(release))
+        self.assertEqual(self.line(first.stdout), "START node")
+        second = self.start(self.root, "next", sys.executable, "-c", "pass")
+        self.assertIn("waiting", self.line(second.stderr))
+        release.touch()
+        self.assertEqual(first.wait(timeout=5), 0)
+        _, err = second.communicate(timeout=5)
+        self.assertEqual(second.returncode, 0, err)
+
+    def test_nested_command_does_not_deadlock(self):
+        nested = self.command(self.root, "nested", sys.executable, "-c", "print('nested ok')")
+        code = "import subprocess,sys; sys.exit(subprocess.call(" + repr(nested) + "))"
+        child = self.start(self.root, "outer", sys.executable, "-c", code)
+        out, err = child.communicate(timeout=5)
+        self.assertEqual(child.returncode, 0, err)
+        self.assertEqual(out.strip(), "nested ok")
+        self.assertEqual(err.count("acquired"), 1)
+
+    def test_failure_releases_lock_and_preserves_exit_status(self):
+        child = self.start(self.root, "fails", sys.executable, "-c", "raise SystemExit(7)")
+        child.communicate(timeout=5)
+        self.assertEqual(child.returncode, 7)
+        successor = self.start(self.root, "next", sys.executable, "-c", "pass")
+        _, err = successor.communicate(timeout=5)
+        self.assertEqual(successor.returncode, 0, err)
+        self.assertNotIn("waiting", err)
+
+    def test_killed_owner_releases_kernel_lock(self):
+        first, _ = self.hold(self.root, "killed")
+        self.assertIn("acquired", self.line(first.stderr))
+        self.assertEqual(self.line(first.stdout), "START killed")
+        second = self.start(self.root, "next", sys.executable, "-c", "pass")
+        self.assertIn("waiting", self.line(second.stderr))
+        first.kill()
+        self.assertEqual(first.wait(timeout=5), -signal.SIGKILL)
+        _, err = second.communicate(timeout=5)
+        self.assertEqual(second.returncode, 0, err)
+        self.assertIn("acquired", err)
+
+    def test_separate_checkouts_do_not_block_each_other(self):
+        first, release_first = self.hold(self.root, "first")
+        self.assertEqual(self.line(first.stdout), "START first")
+        other_root = self.root / "other"
+        other_root.mkdir()
+        second, release_second = self.hold(other_root, "second")
+        self.assertIn("acquired", self.line(second.stderr))
+        self.assertEqual(self.line(second.stdout), "START second")
+        self.assertIsNone(first.poll())
+        release_first.touch()
+        release_second.touch()
+
+    def test_missing_flock_fails_instead_of_running_unlocked(self):
+        shadow = Path(tempfile.mkdtemp(dir=self.root))
+        (shadow / "fcntl.py").write_text('raise ImportError("no flock here")')
+        env = {**os.environ, "PYTHONPATH": str(shadow)}
+        child = self.start(self.root, "no-flock", sys.executable, "-c", "print('ran')",
+                           env=env)
+        out, err = child.communicate(timeout=5)
+        self.assertEqual(child.returncode, 2, err)
+        self.assertNotIn("ran", out)
+        self.assertIn("requires POSIX flock", err)
+
+    def test_stale_environment_marker_does_not_skip_lock(self):
+        first, release_first = self.hold(self.root, "owner")
+        self.assertEqual(self.line(first.stdout), "START owner")
+        stale = {"root": str(self.root), "pid": os.getpid(), "token": "old"}
+        env = {**os.environ, "RESCRIPT_TEST_LOCK": json.dumps(stale)}
+        second = self.start(self.root, "stale", sys.executable, "-c", "pass", env=env)
+        self.assertIn("waiting", self.line(second.stderr))
+        release_first.touch()
+        _, err = second.communicate(timeout=5)
+        self.assertEqual(second.returncode, 0, err)
+
+
+if __name__ == "__main__":
+    unittest.main(verbosity=2)