From 16e856a20307a3ca20524d96ea13348db7f2cffd Mon Sep 17 00:00:00 2001 From: nsfisis Date: Sat, 2 May 2026 17:40:07 +0900 Subject: feat(installer): add trace recorder and topo install order Adds TraceRecorderExecutor (Composer's InstallationManagerMock analog), which records every install/update/uninstall as a string matching Composer's *Operation::__toString output (after strip_tags) - the load-bearing assertion target for in-process fixture tests. Two changes were needed to make the recorder useful: - InstallerExecutor::uninstall_package gains a version parameter, and install_from_lock now looks up both the uninstall and the Update-from-version from installed.json. Previously the Update path passed the new version as a placeholder; the recorder needs the real old version to emit `Upgrading pkg (old => new)`. - compute_operations now topologically sorts the lock contents (deps before dependents) before computing actions, mirroring Composer's Transaction::calculateOperations. Without this, packages would install in alphabetical order and the trace would diverge from Composer's expectation. Also adds crates/mozart/tests/installer_in_process.rs with the in-process harness scaffold: parses the same .test fixtures, builds a tempdir, calls commands::install::run / update::run with an empty RepositorySet (no Packagist) and a TraceRecorderExecutor, then asserts exit code + EXPECT trace. One fixture wired up: suggest_replaced - the original CI failure that motivated this whole DI refactor. It now passes on the in-process path because the empty RepositorySet makes b/b unreachable just like Composer's `'packagist' => false` test config, and the resolver finds c/c (which replaces b/b) via the inline package repo's eager preload. Step F will migrate every fixture currently in installer.rs to the new harness; remaining divergences (alias handling, output ordering, replace trace shape, etc.) will surface as individual follow-ups. All 136 existing spawn-based fixtures + 114 mozart-registry tests + 541 mozart lib tests still green; clippy clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/installer_executor/filesystem.rs | 7 +- .../mozart-registry/src/installer_executor/mod.rs | 14 ++- .../src/installer_executor/trace_recorder.rs | 104 +++++++++++++++++++++ 3 files changed, 123 insertions(+), 2 deletions(-) create mode 100644 crates/mozart-registry/src/installer_executor/trace_recorder.rs (limited to 'crates/mozart-registry/src/installer_executor') diff --git a/crates/mozart-registry/src/installer_executor/filesystem.rs b/crates/mozart-registry/src/installer_executor/filesystem.rs index 82acc42..e36006c 100644 --- a/crates/mozart-registry/src/installer_executor/filesystem.rs +++ b/crates/mozart-registry/src/installer_executor/filesystem.rs @@ -81,7 +81,12 @@ impl InstallerExecutor for FilesystemExecutor { Ok(()) } - fn uninstall_package(&mut self, name: &str, ctx: &ExecuteContext) -> anyhow::Result<()> { + fn uninstall_package( + &mut self, + name: &str, + _version: &str, + ctx: &ExecuteContext, + ) -> anyhow::Result<()> { let pkg_dir = ctx.vendor_dir.join(name); if pkg_dir.exists() { std::fs::remove_dir_all(&pkg_dir)?; diff --git a/crates/mozart-registry/src/installer_executor/mod.rs b/crates/mozart-registry/src/installer_executor/mod.rs index fde4c49..1fab19f 100644 --- a/crates/mozart-registry/src/installer_executor/mod.rs +++ b/crates/mozart-registry/src/installer_executor/mod.rs @@ -18,8 +18,10 @@ use std::path::PathBuf; use crate::lockfile::LockedPackage; pub mod filesystem; +pub mod trace_recorder; pub use filesystem::FilesystemExecutor; +pub use trace_recorder::TraceRecorderExecutor; /// One install or update operation handed to [`InstallerExecutor::install_package`]. #[derive(Debug, Clone, Copy)] @@ -73,7 +75,17 @@ pub trait InstallerExecutor: Send + Sync { ) -> anyhow::Result<()>; /// Perform side effects for one uninstall. - fn uninstall_package(&mut self, name: &str, ctx: &ExecuteContext) -> anyhow::Result<()>; + /// + /// `version` is the previously-installed version (from installed.json), + /// passed so the trace recorder can format Composer's + /// `Uninstalling pkg/name (version)` line. The filesystem implementation + /// ignores it — `name` alone is enough to locate the vendor directory. + fn uninstall_package( + &mut self, + name: &str, + version: &str, + ctx: &ExecuteContext, + ) -> anyhow::Result<()>; /// Hook called once after every uninstall has run. Default no-op. /// Composer cleans up empty namespace directories here; the recorder diff --git a/crates/mozart-registry/src/installer_executor/trace_recorder.rs b/crates/mozart-registry/src/installer_executor/trace_recorder.rs new file mode 100644 index 0000000..bb20eb1 --- /dev/null +++ b/crates/mozart-registry/src/installer_executor/trace_recorder.rs @@ -0,0 +1,104 @@ +//! Recording-only [`InstallerExecutor`] for in-process tests. +//! +//! Mirrors `Composer\Test\Mock\InstallationManagerMock` — every call appends +//! a string to a `Vec` matching Composer's +//! `(string) $operation` output (after `strip_tags`). No filesystem or +//! network I/O happens. The recorded trace is what tests assert against +//! `--EXPECT--` in Composer's `.test` fixture format. +//! +//! Trace line shapes (byte-equivalent to Composer's `*Operation::__toString` +//! after `strip_tags`): +//! +//! - Install: `Installing ()` +//! - Update (upgrade direction): `Upgrading ( => )` +//! - Update (downgrade direction): `Downgrading ( => )` +//! - Uninstall: `Uninstalling ()` + +use mozart_semver::Version; + +use super::{ExecuteContext, InstallerExecutor, PackageOperation}; + +/// Recording-only executor. Construct with [`TraceRecorderExecutor::new`], +/// then read [`TraceRecorderExecutor::trace`] after the run completes. +pub struct TraceRecorderExecutor { + trace: Vec, +} + +impl TraceRecorderExecutor { + pub fn new() -> Self { + Self { trace: Vec::new() } + } + + /// Recorded operation strings, in the order [`InstallerExecutor`] was + /// invoked. Pass this to `assert_eq!` against the fixture's `--EXPECT--` + /// section after splitting on newlines. + pub fn trace(&self) -> &[String] { + &self.trace + } + + /// Take ownership of the recorded trace. Use after the run if the + /// executor is going out of scope. + pub fn into_trace(self) -> Vec { + self.trace + } +} + +impl Default for TraceRecorderExecutor { + fn default() -> Self { + Self::new() + } +} + +#[async_trait::async_trait] +impl InstallerExecutor for TraceRecorderExecutor { + async fn install_package( + &mut self, + op: PackageOperation<'_>, + _ctx: &ExecuteContext, + ) -> anyhow::Result<()> { + match op { + PackageOperation::Install { package } => { + self.trace.push(format!( + "Installing {} ({})", + package.name, package.version + )); + } + PackageOperation::Update { + from_version, + package, + } => { + let action = if is_upgrade(from_version, &package.version) { + "Upgrading" + } else { + "Downgrading" + }; + self.trace.push(format!( + "{} {} ({} => {})", + action, package.name, from_version, package.version + )); + } + } + Ok(()) + } + + fn uninstall_package( + &mut self, + name: &str, + version: &str, + _ctx: &ExecuteContext, + ) -> anyhow::Result<()> { + self.trace + .push(format!("Uninstalling {} ({})", name, version)); + Ok(()) + } +} + +/// Mirrors `Composer\Package\Version\VersionParser::isUpgrade` — returns +/// true when `to` is a strictly higher version than `from`. Both unparseable +/// or both equal means treat as upgrade (Composer's behavior on edge cases). +fn is_upgrade(from: &str, to: &str) -> bool { + match (Version::parse(from), Version::parse(to)) { + (Ok(a), Ok(b)) => b >= a, + _ => true, + } +} -- cgit v1.3.1