test: resolve the workspace root at runtime (archive-shipped binaries) - #401
Conversation
Binaries shipped through cargo nextest archive run on machines other
than the one that compiled them, but the ubiquitous
env!("CARGO_WORKSPACE_DIR") bakes the build machine's absolute path
into every binary. One archive job landing on a runner pool with a
different home (/home/ubuntu vs /home/runner) turned every downstream
artifact write into EACCES and wiped out an entire CI run while
looking exactly like a code regression.
neomacs-infra and neovm-core's test_utils now expose the same three
functions: cargo_workspace_root (the baked constant),
nextest_workspace_root (the runtime NEXTEST_WORKSPACE_ROOT nextest
exports, already adjusted by --workspace-remap), and workspace_root
(the resolver call sites use -- runtime value when present, baked
constant otherwise, so the fallback order is decided exactly once).
The tui harness had already converged on this resolver; the remaining
call sites migrate in a follow-up.
…d crates Follow-through on the resolver: every test that runs from a cargo nextest archive now asks for the workspace of the machine it is running on. neovm-core tests call crate::test_utils::workspace_root, the gui/melpa/neomacs/display/renderer suites call neomacs_infra::workspace_root, perf grows the same pub helper, and xtask's tests reuse its own repository_root. Left baking on purpose: include_bytes!/include_str! (bytes embedded at compile time -- no runtime path exists), the production runtime-root candidates in load/mod.rs and gui_chrome.rs (a fallback chain whose compile-time member is genuinely the build machine, correct for a binary that ships with its checkout), and parity-reference's manifest lookup (a CLI run on the build tree, never archive-shipped). Verified: neovm-core/infra/test-fonts/melpa-support subsets (690), gui harness contracts, and the present contract on Vulkan all green.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The migration leaves multiple orphaned PathBuf/Path imports (and displaces an #[allow(dead_code)] attribute), introducing new unused_imports warnings against the crate's stated lint policy that should be removed.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 14
Open (14)
Move inserted functions below the use block · New Remove unused PathBuf import · New Drop now-unused local PathBuf imports · New Remove unused PathBuf import · New Remove unused PathBuf import · New Remove unused PathBuf import · New Remove unused PathBuf import · New Remove unused PathBuf import · New Remove unused PathBuf import · New Remove unused PathBuf import · New Remove unused PathBuf import · New Narrow Path import after removing PathBuf · New Remove unused PathBuf import · New Restore dead_code attribute on run_neovm_eval · New
What changed in this PR
This PR fixes a CI reliability problem: cargo nextest archive builds test binaries once and runs them on other runners, but env!("CARGO_WORKSPACE_DIR") bakes the build machine's absolute path into each binary. When an archive job landed on a runner with a different home (/home/ubuntu vs /home/runner), downstream artifact writes hit EACCES and wiped a full CI run. The fix introduces a runtime workspace-root resolver (NEXTEST_WORKSPACE_ROOT first, baked constant as fallback) and migrates every archive-shipped crate to use it.
Changes:
- Adds
workspace_root()(pluscargo_workspace_root/nextest_workspace_roothelpers) toneomacs-infra,neovm-core::test_utils,neomacs-perf, andneomacs-test-fonts, centralizing the fallback order. - Migrates ~50 test files across neovm-core, gui-tests, perf, renderer-wgpu, display-runtime, melpa, and neomacs from
env!("CARGO_WORKSPACE_DIR")to the runtime resolver. - Wires
neomacs-infraas a dev-dependency where needed (Cargo.toml + Cargo.lock).
| File | Description |
|---|---|
crates/neomacs-infra/src/lib.rs |
New central resolver; functions placed mid-use-block (readability nit) |
crates/neovm-core/src/test_utils.rs |
New workspace_root/cargo_workspace_root/nextest_workspace_root helpers |
crates/neovm-core/tests/common/mod.rs |
New workspace_root; insertion displaced an #[allow(dead_code)] attribute |
crates/neovm-core/src/emacs_core/**/tests/*.rs |
Migrated call sites; several now-orphaned PathBuf imports |
crates/neomacs/tests/neomacsclient_cli.rs |
Migrated; six local use std::path::PathBuf; now unused |
crates/neomacs-perf/src/{lib,*_test}.rs |
New workspace_root; build_provenance_test.rs PathBuf import now unused |
crates/neomacs-gui-tests/tests/*.rs |
Migrated to neomacs_infra::workspace_root() |
crates/neomacs-{renderer-wgpu,display-runtime,melpa-tests}/** |
Migrated call sites + dev-dep wiring |
crates/neomacs-{test-fonts,melpa-test-support}/src/lib.rs |
Resolver definitions consolidated |
Cargo.lock |
Adds neomacs-infra dev-dependency edges |
The core mechanism is sound: CI runs archived tests with --workspace-remap "$GITHUB_WORKSPACE", and nextest exports NEXTEST_WORKSPACE_ROOT at runtime, so the resolver returns the correct live path. The main issue is hygiene: the mechanical migration removed the last PathBuf/Path usage in a number of files, leaving orphaned imports. neovm-core explicitly sets unused_imports = "warn" and the crate has a documented campaign to eliminate such warnings, so these should be cleaned up. They are warnings only (the test build does not use -D warnings), so they won't fail CI.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| use std::fs; | ||
| use std::os::unix::fs::PermissionsExt; | ||
| /// The workspace root baked in at compile time. |
| #[test] | ||
| fn git_metadata_watch_paths_are_absolute_and_worktree_aware() { | ||
| let workspace_tmp = PathBuf::from(env!("CARGO_WORKSPACE_DIR")).join("tmp"); | ||
| let workspace_tmp = crate::workspace_root().join("tmp"); |
| use std::thread; | ||
|
|
||
| let repo_tmp = PathBuf::from(env!("CARGO_WORKSPACE_DIR")).join("tmp"); | ||
| let repo_tmp = neomacs_infra::workspace_root().join("tmp"); |
|
|
||
| fn gnu_subr_keymap_eval_all(src: &str) -> Vec<String> { | ||
| let project_root = PathBuf::from(env!("CARGO_WORKSPACE_DIR")); | ||
| let project_root = crate::test_utils::workspace_root(); |
| let source = fs::read_to_string(crate::test_utils::workspace_root().join("lisp/select.el")) | ||
| .expect("read select.el"); |
| let source = fs::read_to_string(crate::test_utils::workspace_root().join("lisp/ldefs-boot.el")) | ||
| .expect("read ldefs-boot"); |
| install_bare_elisp_shims(ev); | ||
|
|
||
| let project_root = PathBuf::from(env!("CARGO_WORKSPACE_DIR")); | ||
| let project_root = crate::test_utils::workspace_root(); |
| @@ -12,7 +12,7 @@ use crate::heap_types::LispString; | |||
| use std::path::{Path, PathBuf}; | |||
|
|
||
| fn gnu_subr_sit_for_eval() -> Context { | ||
| let project_root = PathBuf::from(env!("CARGO_WORKSPACE_DIR")); | ||
| let project_root = crate::test_utils::workspace_root(); |
| #[allow(dead_code)] // grandfathered when dead_code lint was enabled; delete or wire up | ||
|
|
||
| /// Runtime workspace root: nextest's NEXTEST_WORKSPACE_ROOT when present, | ||
| /// the compile-time constant otherwise (see neovm-core test_utils). | ||
| pub fn workspace_root() -> std::path::PathBuf { |

Why
cargo nextest archivebuilds binaries once, then CI runs them on other machines.env!("CARGO_WORKSPACE_DIR")bakes the build machine's absolute path into every binary — and when one archive job landed on a runner pool with home/home/ubuntuinstead of/home/runner, every downstream artifact write hit EACCES and wiped out an entire CI run (PR #400's first checks, zero passes, indistinguishable from a code regression).What
One resolver, three named functions (
cargo_workspace_root,nextest_workspace_root,workspace_root) so the fallback order — runtimeNEXTEST_WORKSPACE_ROOTfirst, baked constant only outside nextest — is decided exactly once. Migrated across every archive-shipped crate: neovm-core (28 files), gui-tests (10), melpa, test-fonts, neomacs tests, display-runtime, renderer-wgpu, perf.Left baking on purpose:
include_bytes!/include_str!(no runtime path exists), production runtime-root fallback chains (load/mod.rs,gui_chrome.rs), and parity-reference's manifest lookup (build-tree CLI, never shipped).Verification
cargo check --workspace --tests: 0 errors