Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/pty-conformance/src/bin/conformance_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//! comments in `tests/*.rs`), or the reason it is not portable.
//!
//! Usage: `cargo run -p pty-conformance --bin conformance-map [--node <checkout>] [--out <path>]`.
//! Defaults: the checkout in `PTY_NODE_CHECKOUT` (or `/home/myobie/src/github.com/compoundingtech/pty`),
//! Defaults: the checkout named by `PTY_NODE_CHECKOUT` (required; there is no default),
//! output to `docs/conformance.md` at the workspace root.

fn main() {
Expand Down
31 changes: 26 additions & 5 deletions crates/pty-conformance/src/conformance_map_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,11 +274,32 @@ pub fn run(args: Vec<String>) -> Result<(), String> {
}
i += 1;
}
if checkout.is_none() {
let default = PathBuf::from("/home/myobie/src/github.com/compoundingtech/pty");
if default.join("tests").is_dir() {
checkout = Some(default);
}
// No default, and no silent degrade.
//
// This used to fall back to an absolute path on one developer's machine,
// and `node_suites` falls back again to the compiled-in `SUITES` list when
// it has no checkout — so running this with the variable unset produced a
// checked-in document describing a Node tree nobody named, which looked
// exactly as authoritative as a correct one. That is how docs/conformance.md
// came to say `0.12.0+500eab2` while the pinned reference in `node-ref` was
// `86dcc5e`, and a stale map is what a stale comparison is built on.
//
// A test may skip when its reference is missing. A generator may not guess:
// it writes a file that outlives the run and carries no memory of what it
// was made from.
let Some(dir) = checkout.clone() else {
return Err("PTY_NODE_CHECKOUT is not set, and there is no default.\n\
This writes docs/conformance.md from a Node checkout, and a map \
built from an unnamed source is worse than no map.\n\
Set PTY_NODE_CHECKOUT to the commit in crates/pty-conformance/node-ref, \
or pass --checkout <path>."
.to_string());
};
if !dir.join("tests").is_dir() {
return Err(format!(
"PTY_NODE_CHECKOUT is {}, which has no tests/ directory.",
dir.display()
));
}

let mapped = scan_tests(&manifest_dir().join("tests"))?;
Expand Down
16 changes: 13 additions & 3 deletions crates/pty-conformance/tests/completions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,20 @@ fn which(bin: &str) -> Option<PathBuf> {
if out.status.success() && !s.is_empty() { Some(PathBuf::from(s)) } else { None }
}

/// The Node checkout to compare against, or `None`.
///
/// **There is deliberately no fallback path.** This used to default to an
/// absolute path on one developer's machine, which meant that when
/// `PTY_NODE_CHECKOUT` was unset the test did not skip — it silently compared
/// against whatever happened to be at that path. On a machine where that
/// checkout was seven commits behind, it produced a confident failure about a
/// completion entry that was not missing at all. CI never saw it, because the
/// path does not exist on a runner, so there the filter returned `None` and the
/// test skipped.
///
/// A test that cannot find its reference must say so. It must not guess.
fn node_checkout_dir() -> Option<PathBuf> {
node_checkout()
.or_else(|| Some(PathBuf::from("/home/myobie/src/github.com/compoundingtech/pty")))
.filter(|p| p.join("completions").is_dir())
node_checkout().filter(|p| p.join("completions").is_dir())
}

/// node: tests/completions.test.ts:81
Expand Down
Loading