fix(install): every fixture installer gets the fixture's dialect

`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) <noreply@anthropic.com>
This commit is contained in:
thomas
2026-07-30 21:08:01 +08:00
co-authored by Claude Opus 5
parent 2a4b5c7f68
commit a39eebfe1c
+16 -9
View File
@@ -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");