Skip to content

Commit 2075242

Browse files
Martin Taillefer (from Dev Box)Copilot
andcommitted
Fixes
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6a1863fa-92ca-4fd6-9e0c-12ae4ac61826
1 parent fb06d76 commit 2075242

14 files changed

Lines changed: 592 additions & 89 deletions

File tree

crates/cargo-gamma-engine/src/parse/nesting.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ fn identifier_end(text: &str, at: usize) -> Option<usize> {
332332

333333
#[cfg(test)]
334334
mod tests {
335-
use std::fs;
335+
use std::{env, fs};
336336

337337
use camino::Utf8Path;
338338
use walkdir::WalkDir;
@@ -531,8 +531,13 @@ mod tests {
531531
/// The counts added here are proxies, and a proxy that refuses ordinary Rust is worse than the
532532
/// crash it prevents. This is the check that keeps them calibrated against real code.
533533
#[test]
534-
535534
fn this_workspace_is_within_the_limit() {
535+
// Gamma's scratch tree contains instrumented source whose guard expressions are
536+
// intentionally deeper than the source this calibration test is meant to measure.
537+
let _ = env::var_os("CARGO_GAMMA").is_none().then(assert_workspace_is_within_limit);
538+
}
539+
540+
fn assert_workspace_is_within_limit() {
536541
let root = Utf8Path::new(env!("CARGO_MANIFEST_DIR")).join("..");
537542

538543
for entry in WalkDir::new(root.as_std_path()).into_iter().filter_map(Result::ok) {

crates/cargo-gamma-lib/docs/DESIGN.md

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ Each Cargo output stream is retained up to 256 MiB for artifact and diagnostic
6969
processing, and each logical line is bounded at 1 MiB. The buffers grow with
7070
observed output rather than reserving those ceilings for every invocation.
7171

72-
Each failed baseline observation retains a bounded 64 KiB, 200-line tail from
72+
Each failed baseline observation retains a bounded 64 KiB, 2,000-line tail from
7373
stdout and stderr, along with the process exit code or signal when available.
7474
The baseline error identifies the package, target, runner, executable, working
7575
directory, elapsed time, resource failure, and failing or last-observed test.
@@ -81,10 +81,20 @@ safely encoded output tails, to `baseline-failure.json` and writes the ordinary
8181
controls are eligible for diagnostic records, never the inherited process
8282
environment.
8383

84+
Completed runs publish the five ordinary `gamma-report.json`, HTML, SARIF,
85+
performance-advice, and diagnostics artifacts. An early baseline failure
86+
instead publishes `baseline-failure.json` and `gamma-diagnostics.json` before
87+
the scratch workspace is removed. The baseline record uses `schemaVersion: 1`
88+
and records the failure kind and reason; package, target, runner, executable,
89+
and working directory; cargo-gamma's explicit environment overrides; failing
90+
and last-observed tests; termination, elapsed time, budget, peak, and memory
91+
limit; and control-character-encoded stdout and stderr tails with a truncation
92+
flag.
93+
8494
When Cargo's resolved package selection covers the whole workspace, every stage
85-
checks mutation viability with that constant Cargo root set. Packages with no
86-
mutable files may be omitted from preflight, but do not narrow the staged
87-
checks. This keeps dependency feature unification identical across stages
95+
checks mutation viability with that constant Cargo root set, and preflight
96+
validates the same roots even when some packages contain no mutable files. This
97+
keeps dependency feature unification identical across validation and stages
8898
instead of compiling a new dependency variant for each downstream package
8999
selection. Only the current stage's mutants are instrumented; mutants belonging
90100
to other stages are restored before each ordinary, probe, or isolation build.

crates/cargo-gamma-lib/src/commands/run.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -806,7 +806,8 @@ fn measured<H: Host>(host: &mut H, args: &RunArgs, progress_when: When, styler:
806806
if let Some(lock_identity) = cache_lock_identity {
807807
pause_after_cache_adoption(&survey.root, lock_identity);
808808
}
809-
let mut outcome = exec::run_with_locks(&survey, &selection, &config, &mut events, cache_locks);
809+
let mut failed_work = None;
810+
let mut outcome = exec::run_with_locks(&survey, &selection, &config, &mut events, cache_locks, &mut failed_work);
810811

811812
// A phase that failed never got to say what it found, so the line it opened is still waiting
812813
// for an ending. Close it before the error is printed, or the error arrives as the rest of
@@ -821,6 +822,13 @@ fn measured<H: Host>(host: &mut H, args: &RunArgs, progress_when: When, styler:
821822
emit_failure_artifacts(&mut events, args, &survey.skeleton(), failure, &artifact_dir, started, styler);
822823
}
823824

825+
if let Some(mut work) = failed_work
826+
&& let Err(cleanup) = work.teardown()
827+
&& let Err(failure) = &mut outcome
828+
{
829+
failure.append_message(&format!("\nCleanup: {cleanup}"));
830+
}
831+
824832
let exec::Measured {
825833
plan,
826834
built,

crates/cargo-gamma-lib/src/exec/baseline.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -296,8 +296,8 @@ fn baseline_failure_error(
296296
"packageId": binary.package_id,
297297
"target": binary.target,
298298
"runner": runner,
299-
"executable": binary.path,
300-
"workingDirectory": directory,
299+
"executable": binary.path.as_str(),
300+
"workingDirectory": directory.as_str(),
301301
"environmentOverrides": baseline_environment(work),
302302
"test": test,
303303
"lastObservedTest": last_test,
@@ -483,6 +483,8 @@ mod tests {
483483

484484
assert_eq!(artifact.file_name, "baseline-failure.json");
485485
assert_eq!(artifact.value["package"], "subject");
486+
assert_eq!(artifact.value["executable"], binaries[0].path.as_str());
487+
assert_eq!(artifact.value["workingDirectory"], working_directory(&work, &binaries[0]).as_str());
486488
assert_eq!(artifact.value["test"], "a::b");
487489
assert_eq!(artifact.value["termination"]["kind"], "exitCode");
488490
assert_eq!(artifact.value["termination"]["value"], 101);

crates/cargo-gamma-lib/src/exec/build.rs

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -203,10 +203,10 @@ pub(super) struct Abandoned {
203203

204204
/// Drives the build, withdrawing mutants that cannot compile until what is asked for compiles.
205205
///
206-
/// A run converges the workspace one stage at a time and then once as a whole, and every one of
207-
/// those builds shares the same withdrawal set and budget reference. Sharing the withdrawal set is
208-
/// what lets a stage inherit what earlier stages already ruled out: a mutant already known to be
209-
/// unbuildable stays withdrawn for the rest of the run.
206+
/// A run converges the workspace one stage at a time and then once as a whole. The withdrawal set is
207+
/// shared, which lets a stage inherit what earlier stages already ruled out: a mutant already known
208+
/// to be unbuildable stays withdrawn for the rest of the run. Timeout calibration is reset for each
209+
/// build so only comparable Cargo commands and root sets share a reference.
210210
///
211211
/// The round counter is not shared. `--rollback-rounds` caps the rounds one build may spend
212212
/// converging, so it is reset for each build; a cumulative counter would let early stages spend the
@@ -249,11 +249,11 @@ pub(super) struct Converger {
249249
/// wants to know is where the run's build time went, not where one stage's did.
250250
history: Vec<Round>,
251251

252-
/// How long the first build of the run took, which every later budget is scaled from.
252+
/// How long the first ordinary round of the current build took.
253253
///
254-
/// Set once and never reset. A narrowed stage can build a fraction of the workspace, so letting
255-
/// one set this reference would leave every later stage — and the whole-workspace build that
256-
/// follows them — with a budget derived from a build that was never comparable.
254+
/// Reset before each convergence. Subsequent rollback and isolation rounds repeat that build's
255+
/// Cargo command and roots, so they are comparable; a later stage or final test-target build is
256+
/// not.
257257
first_round: Option<Duration>,
258258

259259
/// What the tree already holds, so a round rewrites only the files it changed.
@@ -403,6 +403,13 @@ impl Converger {
403403
withdrawn
404404
}
405405

406+
/// Resets state that describes one Cargo command and root set.
407+
fn begin_convergence(&mut self) {
408+
self.rounds = 0;
409+
self.per_round.clear();
410+
self.first_round = None;
411+
}
412+
406413
/// Instruments the tree and builds it until it compiles, withdrawing whatever stands in the way.
407414
///
408415
/// The scope's roots name the packages Cargo compiles, while its mutants limit what convergence
@@ -427,8 +434,7 @@ impl Converger {
427434
// to answer for, and the withdrawal series the limit error reads has to describe the build
428435
// that failed. The withdrawal set is deliberately left alone — a mutant already known not
429436
// to compile stays withdrawn for the rest of the run.
430-
self.rounds = 0;
431-
self.per_round.clear();
437+
self.begin_convergence();
432438

433439
// Before the first ordinary round, and only ever before it. Whatever the probe withdraws is
434440
// withdrawn by the compiler's own accusation in a real build, so the loop below starts from
@@ -779,11 +785,8 @@ impl Converger {
779785
return Ok(());
780786
};
781787

782-
// Deliberately not allowed to set `first_round`, which every later build timeout is scaled
783-
// from. A probe compiles a fraction of the mutants and usually stops on the first errors,
784-
// so its elapsed time is not what a full round of this build costs — adopting it as the
785-
// reference would set every later budget from a build that was never comparable, and the
786-
// run would start timing out builds that are merely honest about their size.
788+
// A probe compiles a fraction of the mutants and usually stops on the first errors, so it
789+
// cannot calibrate the ordinary rounds that follow.
787790

788791
if outcome.succeeded {
789792
// Every hint was wrong: nothing here is unviable now. Nothing is withdrawn and nothing
@@ -1200,7 +1203,7 @@ impl Converger {
12001203
roots: select,
12011204
mutants: select,
12021205
},
1203-
&["build", "--tests", "--examples", "--keep-going"],
1206+
&["build", "--tests", "--keep-going"],
12041207
limits,
12051208
events,
12061209
)

crates/cargo-gamma-lib/src/exec/build/tests.rs

Lines changed: 56 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,32 @@ fn a_build_past_its_output_limit_fails_without_retaining_the_excess() {
177177
);
178178
}
179179

180+
/// The public defaults, rather than test-only reduced limits, exceed the historical stream and line
181+
/// caps.
182+
#[test]
183+
fn default_output_limits_accept_large_streams_and_lines() {
184+
const OLD_STREAM_LIMIT: usize = 4 * 1024 * 1024;
185+
const LINE_LENGTH: usize = 128 * 1024;
186+
187+
let mut output = Vec::with_capacity(OLD_STREAM_LIMIT + LINE_LENGTH);
188+
while output.len() <= OLD_STREAM_LIMIT {
189+
output.extend(std::iter::repeat_n(b'x', LINE_LENGTH - 1));
190+
output.push(b'\n');
191+
}
192+
193+
let (sender, lines) = mpsc::sync_channel(64);
194+
let pipe = read_pipe(io::Cursor::new(output.clone()), Stream::Prose, &sender)
195+
.expect("spawn reader")
196+
.join()
197+
.expect("reader");
198+
drop(sender);
199+
drop(lines);
200+
201+
assert_eq!(pipe.text, output);
202+
assert!(pipe.complete);
203+
assert!(pipe.within_limits, "the production defaults regressed to their old bounds");
204+
}
205+
180206
/// A small retained cap records truncation while continuing to drain the pipe.
181207
#[test]
182208
fn an_over_limit_pipe_keeps_only_its_configured_prefix() {
@@ -367,17 +393,23 @@ fn a_narrowed_build_that_fails_is_retried_across_the_whole_workspace() {
367393
assert!(build.widened, "the build should have reported that it widened");
368394
}
369395

370-
/// Examples remain part of the compilation oracle even though they are never test binaries.
396+
/// Targets that cargo-gamma will not run do not belong to its compilation oracle.
371397
#[test]
372-
fn the_final_build_still_compiles_examples() {
373-
let (_dir, work) = trivial_workspace("build-example-");
398+
fn the_final_build_does_not_compile_examples_or_benches() {
399+
let (_dir, work) = trivial_workspace("build-non-test-targets-");
374400

375401
fs::create_dir_all(work.root.join("examples").as_std_path()).expect("examples");
376402
fs::write(
377403
work.root.join("examples/broken.rs").as_std_path(),
378404
"fn main() { let _: i32 = \"not an integer\"; }\n",
379405
)
380406
.expect("example");
407+
fs::create_dir_all(work.root.join("benches").as_std_path()).expect("benches");
408+
fs::write(
409+
work.root.join("benches/broken.rs").as_std_path(),
410+
"fn main() { let _: i32 = \"not an integer\"; }\n",
411+
)
412+
.expect("bench");
381413

382414
let mut plan = empty_plan(&work);
383415
let build = Converger::default()
@@ -388,13 +420,13 @@ fn the_final_build_still_compiles_examples() {
388420
BuildLimits::default(),
389421
&mut crate::testing::Recorder::default(),
390422
)
391-
.expect("the build reports the example failure");
423+
.expect("non-test targets are not part of the build");
392424

425+
assert!(build.stuck.is_none(), "examples and benches must not affect the test oracle");
393426
assert!(
394-
build.stuck.is_some(),
395-
"a broken example must not disappear from the compilation oracle"
427+
!build.binaries.is_empty(),
428+
"the test oracle still contains its runnable test binary"
396429
);
397-
assert!(build.binaries.is_empty(), "a failed compilation produces no runnable oracle");
398430
}
399431

400432
/// A workspace of two members, one of which does not compile and is not being mutated.
@@ -1034,6 +1066,23 @@ fn each_build_gets_the_whole_round_budget_rather_than_what_earlier_builds_left()
10341066
assert_eq!(converger.withdrawn(), 2, "withdrawals carry across builds");
10351067
}
10361068

1069+
/// A check-stage duration cannot budget a later code-generating test-target build.
1070+
#[test]
1071+
fn each_build_calibrates_its_own_timeout_reference() {
1072+
let mut converger = Converger {
1073+
rounds: 7,
1074+
per_round: vec![3, 1],
1075+
first_round: Some(Duration::from_millis(1)),
1076+
..Converger::default()
1077+
};
1078+
1079+
converger.begin_convergence();
1080+
1081+
assert_eq!(converger.rounds, 0);
1082+
assert!(converger.per_round.is_empty());
1083+
assert_eq!(converger.first_round, None);
1084+
}
1085+
10371086
/// The limit error reads a series of withdrawal counts and gives falling-or-flat advice from
10381087
/// it, so the series has to describe the build that just failed. Counts left over from earlier
10391088
/// builds would have it describe work the reader is not being told about.

0 commit comments

Comments
 (0)