Skip to content

Commit d51f81d

Browse files
committed
fix: the e2e tests build the worker they exec
CI went red on the examples move, and the cause is a cargo behaviour the move walked straight into: no ordinary test command reliably produces a *runnable* example. - `cargo test --all-targets`, which is what CI runs, compiles every example as a libtest harness — there `--examples` means test them — so `examples/<name>` never appears and only `examples/<name>-<hash>` does. Exec'ing that answers `running 0 tests` and compares nothing. - a plain `cargo test` does build runnable examples, except the four declared `test = true`: `counted`, `sessionize`, `shards` and `wordcount`, which is four of the five workers these tests exec. - `cargo test --test cat_e2e` builds no examples at all. So `tests/common::example` builds what it needs — one `cargo build --example <name>`, at most once per name per test binary, in whatever profile the test itself was built for. By the time a test runs, the cargo that started it has released the target-directory lock. CI keeps a build step of its own in front of the test run. Not because the tests need it now, but so the nine are built once, before anything runs, rather than by several test binaries discovering the gap at the same moment and queueing on that lock. Verified from a clean target directory under `cargo test --workspace`, `cargo test --workspace --all-targets`, a single `--test` file, and `--release`.
1 parent f50d3b8 commit d51f81d

2 files changed

Lines changed: 88 additions & 25 deletions

File tree

  • .github/workflows
  • crates/ytsaurus-job/tests/common

.github/workflows/ci.yml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,17 @@ jobs:
5757
- name: cargo test (client with every feature)
5858
run: cargo test -p ytsaurus-client --all-features
5959

60+
# The workers are examples of `ytsaurus-job`, and the end-to-end tests
61+
# below exec them. `--all-targets` does not build a runnable one — there
62+
# `--examples` means *test* the examples, so each is compiled as a libtest
63+
# harness and no plain binary is produced — and `tests/common::example`
64+
# answers that by building what it needs. Doing it here as well is not
65+
# redundant: it builds the nine once, before any test runs, instead of
66+
# leaving several test binaries to discover the gap at the same moment and
67+
# queue behind each other on cargo's lock.
68+
- name: Build the worker examples the e2e tests run
69+
run: cargo build -p ytsaurus-job --examples
70+
6071
- name: cargo test
6172
run: cargo test --workspace --all-targets
6273

crates/ytsaurus-job/tests/common/mod.rs

Lines changed: 77 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -167,41 +167,93 @@ impl io::Write for SharedBuffer {
167167
}
168168
}
169169

170-
/// Where cargo put a compiled example, so a test can run one.
170+
/// A runnable copy of one of the `examples/` workers, built if it is not there.
171171
///
172-
/// The workers under `examples/` are the thing these end-to-end tests exist to
173-
/// run: they exec the binary the way a cluster does — rows in on fd 0, tables
174-
/// out on fds 1 and 4 — rather than calling into the library, because what the
175-
/// cluster runs is a process and not a function.
172+
/// These end-to-end tests exec the worker the way a cluster does — rows in on
173+
/// fd 0, tables out on fds 1 and 4 — rather than calling into the library,
174+
/// because what the cluster runs is a process and not a function.
176175
///
177-
/// **Derived from the test binary's own path, because cargo names no variable
178-
/// for it.** `CARGO_BIN_EXE_<name>` exists for `[[bin]]` targets and has no
179-
/// counterpart for examples, so the only thing to go on is that both land in
180-
/// the same profile directory: the test at `<profile>/deps/<name>-<hash>` and
181-
/// the example at `<profile>/examples/<name>`. That holds for `--release` and
182-
/// for a `--target` cross-build too, since the whole tree moves together.
176+
/// **Cargo names no variable for an example's path.** `CARGO_BIN_EXE_<name>`
177+
/// exists for `[[bin]]` targets and has no counterpart here, so the path is
178+
/// derived instead: the test binary sits at `<profile>/deps/<name>-<hash>` and a
179+
/// runnable example at `<profile>/examples/<name>`, and that relationship holds
180+
/// under `--release` and under a `--target` cross-build, since the whole tree
181+
/// moves together.
182+
///
183+
/// **And no ordinary test command reliably builds one**, which is why this
184+
/// builds it rather than asserting:
185+
///
186+
/// - `cargo test --all-targets` compiles every example as a *libtest harness*
187+
/// to `examples/<name>-<hash>`, because there `--examples` means test them.
188+
/// No runnable binary is produced. Exec'ing the harness would answer
189+
/// `running 0 tests` and compare nothing, so a hashed sibling is never used
190+
/// here even when one is sitting right beside the name being looked for.
191+
/// - a plain `cargo test` does build runnable examples — except the four
192+
/// declared `test = true` in `Cargo.toml`, which get the harness treatment
193+
/// for the same reason, and those are `counted`, `sessionize`, `shards` and
194+
/// `wordcount`: four of the five workers these tests exec.
195+
/// - `cargo test --test cat_e2e` builds that test and no examples at all.
196+
///
197+
/// So the fallback is one `cargo build --example <name>`, run at most once per
198+
/// name per test binary. By the time a test runs, the cargo that started it has
199+
/// finished building and released the target-directory lock; a second cargo
200+
/// blocks on that lock only if something else is building, which is correct
201+
/// rather than merely tolerable.
183202
///
184203
/// # Panics
185204
///
186-
/// If the example is not there, which means it was not built. `cargo test` and
187-
/// `cargo test --all-targets` build examples; `cargo test --test wordcount_e2e`
188-
/// on its own does not, and the message says so rather than leaving a reader
189-
/// with `No such file or directory` about a path they never wrote.
205+
/// If the example cannot be built, with cargo's own output.
190206
pub fn example(name: &str) -> std::path::PathBuf {
191-
let mut path = std::env::current_exe().expect("a test knows its own path");
192-
path.pop();
193-
if path.ends_with("deps") {
194-
path.pop();
207+
use std::sync::{Mutex, OnceLock};
208+
209+
// One build per name per process: several tests in one binary ask for the
210+
// same worker, and they run on their own threads.
211+
static BUILT: OnceLock<Mutex<std::collections::HashSet<String>>> = OnceLock::new();
212+
213+
let mut dir = std::env::current_exe().expect("a test knows its own path");
214+
dir.pop();
215+
if dir.ends_with("deps") {
216+
dir.pop();
217+
}
218+
// `<profile>` — `debug` for the dev profile, and its own name otherwise.
219+
let profile = dir
220+
.file_name()
221+
.and_then(|name| name.to_str())
222+
.unwrap_or("debug")
223+
.to_owned();
224+
dir.push("examples");
225+
let path = dir.join(name);
226+
227+
let mut built = BUILT
228+
.get_or_init(|| Mutex::new(std::collections::HashSet::new()))
229+
.lock()
230+
.expect("the build set is only ever locked here");
231+
232+
if !path.is_file() && built.insert(name.to_owned()) {
233+
let cargo = std::env::var("CARGO").unwrap_or_else(|_| "cargo".to_owned());
234+
let mut command = std::process::Command::new(cargo);
235+
command.args(["build", "-p", "ytsaurus-job", "--example", name]);
236+
// `--profile dev` is spelled `debug` in the directory and rejected on
237+
// the command line, so the one case that needs no flag is the one that
238+
// cannot take it.
239+
if profile != "debug" {
240+
command.args(["--profile", &profile]);
241+
}
242+
243+
let out = command
244+
.output()
245+
.unwrap_or_else(|e| panic!("could not run cargo to build the `{name}` example: {e}"));
246+
assert!(
247+
out.status.success(),
248+
"building the `{name}` example failed:\n{}",
249+
String::from_utf8_lossy(&out.stderr)
250+
);
195251
}
196-
path.push("examples");
197-
path.push(name);
252+
drop(built);
198253

199254
assert!(
200255
path.is_file(),
201-
"the `{name}` example is not built at {}.\n\
202-
Run `cargo test -p ytsaurus-job` or `cargo build -p ytsaurus-job \
203-
--examples` first: cargo builds examples for a whole-package test run \
204-
and not for `--test <name>` on its own.",
256+
"no runnable `{name}` example at {} even after building it",
205257
path.display()
206258
);
207259
path

0 commit comments

Comments
 (0)