From a39eebfe1c09465be5bb576a458c2f5beae68696 Mon Sep 17 00:00:00 2001 From: thomas Date: Thu, 30 Jul 2026 21:08:01 +0800 Subject: [PATCH] fix(install): every fixture installer gets the fixture's dialect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `an_install_reports_both_transfers_to_completion` and `every_report_carries_the_host` built their `Installer` by hand — `with_version` and `with_timeouts`, no `with_dialect`. So they ran at `RemoteProtocol::of_this_build()` while `FakeRemote` answered the `--protocol` probe with the fixture's fixed `c3p4`, and passed only because the real `CONTROL_VERSION` currently *is* 3. Found by bumping `CONTROL_VERSION` to 4 locally: both tests fail with `DialectMismatch { wanted: c4p4, spoke: c3p4 }` — an error about a fake's plumbing, raised in two tests that are about *progress reporting*, at the one moment somebody is busy changing the wire and least able to spare the attention. The fixture's own comment says the dialect is a fixed literal "rather than `RemoteProtocol::of_this_build`", which is exactly right and was exactly what these two bypassed. Both now go through `installer()`, whose `release` parameter is a `&dyn AssetFetcher` so that a chunked fetcher — the only reason either test hand-rolled the builder — is no longer a reason to leave the helper. The helper's doc says what forgetting it costs, since that is the failure mode a reader needs and the compiler cannot give. No behaviour change, and the same 644 tests before and after. Co-Authored-By: Claude Opus 5 (1M context) --- crates/tty7-core/src/daemon/install/tests.rs | 25 +++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/crates/tty7-core/src/daemon/install/tests.rs b/crates/tty7-core/src/daemon/install/tests.rs index 5fd681d8..180cd4a5 100644 --- a/crates/tty7-core/src/daemon/install/tests.rs +++ b/crates/tty7-core/src/daemon/install/tests.rs @@ -457,9 +457,22 @@ impl InstallConfirm for FakeUser { } } +/// The fixture's installer: this file's [`VERSION`], this file's dialect, and +/// timeouts a fake can satisfy. +/// +/// **Every test builds its installer through here**, and the dialect is why. +/// `Installer::new` starts at [`RemoteProtocol::of_this_build`], while +/// [`FakeRemote`] answers with [`ours`] — the fixture's fixed `c3p4`. A test that +/// hand-rolls the builder and forgets [`Installer::with_dialect`] passes only +/// while the real [`CONTROL_VERSION`](crate::daemon::control::CONTROL_VERSION) +/// happens to equal [`CONTROL`], and then fails on the next wire break with a +/// `DialectMismatch` that has nothing to do with whatever that bump changed. +/// Two tests did exactly that, so `release` is a trait object: a chunked or +/// throttled fetcher is a reason to vary the *source*, never a reason to leave +/// this function. fn installer<'a>( remote: &'a FakeRemote, - release: &'a FakeRelease, + release: &'a dyn AssetFetcher, user: &'a FakeUser, host: &str, ) -> Installer<'a> { @@ -1341,10 +1354,7 @@ fn an_install_reports_both_transfers_to_completion() { let reports = Arc::new(Reports::default()); let report = with_install_progress(reports.clone(), || { - Installer::new(&remote, &release, &user, "me@build-box:22") - .with_version(VERSION) - .with_timeouts(Duration::from_millis(200), Duration::from_millis(10)) - .run() + installer(&remote, &release, &user, "me@build-box:22").run() }) .expect("install"); assert!(report.installed, "the fake remote started empty"); @@ -1418,10 +1428,7 @@ fn every_report_carries_the_host() { let reports = Arc::new(Reports::default()); with_install_progress(reports.clone(), || { - Installer::new(&remote, &release, &user, "me@build-box:22") - .with_version(VERSION) - .with_timeouts(Duration::from_millis(200), Duration::from_millis(10)) - .run() + installer(&remote, &release, &user, "me@build-box:22").run() }) .expect("install");