From e97dc6e64c1be4bf78c420b11cec2a18dfd506d4 Mon Sep 17 00:00:00 2001 From: nsfisis Date: Sun, 28 Jun 2026 14:30:41 +0900 Subject: test(tests): use mockall for hand-written interface mocks Replace hand-written mock/stub structs that re-implemented PHPUnit mock-builder behavior (record-and-verify, manual call counters, unreachable!() guards) with mockall::mock! locals across: - package/loader: MockLoader, VersionGuesserMock - command: ArchiveManager/RepositoryManager/EventDispatcher mocks - util: ConfigSource/AuthJson mocks (auth_helper, bitbucket, github, forgejo, gitlab) - repository/vcs: github_driver NullConfigSource - installer: CountingInstaller, RecordingBinaryInstaller, and the DownloadManager mock (formerly common/downloader_stub.rs, now deleted) - downloader: download_manager create_downloader_mock Verification (counts/args) now lives in mockall expectations checked on drop. installation_manager BinaryInstaller is left hand-written because its as_binary_presence_interface seam returns Some(&mut self), which mockall cannot express; io_stub and io_mock are left as-is. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../package/loader/root_package_loader_test.rs | 57 ++++++++--------- .../package/loader/validating_array_loader_test.rs | 74 +++++++++------------- 2 files changed, 56 insertions(+), 75 deletions(-) (limited to 'crates/shirabe/tests/package') diff --git a/crates/shirabe/tests/package/loader/root_package_loader_test.rs b/crates/shirabe/tests/package/loader/root_package_loader_test.rs index 7fa970a..93ee93a 100644 --- a/crates/shirabe/tests/package/loader/root_package_loader_test.rs +++ b/crates/shirabe/tests/package/loader/root_package_loader_test.rs @@ -4,7 +4,7 @@ // ProcessExecutor / VersionGuesser or require constraints whose parsing goes through a // look-around regex the regex crate cannot compile. -use std::cell::{Cell, RefCell}; +use std::cell::RefCell; use std::rc::Rc; use indexmap::IndexMap; @@ -87,25 +87,17 @@ impl Drop for GitVersionGuard { } // A test double for the concrete VersionGuesser, supplied through the VersionGuesserInterface seam. -#[derive(Debug)] -struct VersionGuesserMock { - version_data: VersionData, - guess_version_calls: Rc>, -} - -impl VersionGuesserInterface for VersionGuesserMock { - fn guess_version( - &mut self, - _package_config: &IndexMap, - _path: &str, - ) -> anyhow::Result> { - self.guess_version_calls - .set(self.guess_version_calls.get() + 1); - Ok(Some(self.version_data.clone())) - } - - fn get_root_version_from_env(&self) -> anyhow::Result { - unreachable!("COMPOSER_ROOT_VERSION is not set in this test") +mockall::mock! { + #[derive(Debug)] + pub VersionGuesser {} + impl VersionGuesserInterface for VersionGuesser { + fn guess_version( + &mut self, + package_config: &IndexMap, + path: &str, + ) -> anyhow::Result>; + + fn get_root_version_from_env(&self) -> anyhow::Result; } } @@ -253,17 +245,19 @@ fn test_pretty_version_for_root_package_in_version_branch() { let config = make_config(); let manager = make_manager(&io, &config); - let guess_version_calls = Rc::new(Cell::new(0u32)); - let version_guesser = VersionGuesserMock { - version_data: VersionData { - version: Some("3.0.9999999.9999999-dev".to_string()), - commit: Some("aabbccddee".to_string()), - pretty_version: Some("3.0-dev".to_string()), - feature_version: None, - feature_pretty_version: None, - }, - guess_version_calls: guess_version_calls.clone(), - }; + let mut version_guesser = MockVersionGuesser::new(); + version_guesser + .expect_guess_version() + .times(1..) + .returning(|_, _| { + Ok(Some(VersionData { + version: Some("3.0.9999999.9999999-dev".to_string()), + commit: Some("aabbccddee".to_string()), + pretty_version: Some("3.0-dev".to_string()), + feature_version: None, + feature_pretty_version: None, + })) + }); let mut loader = RootPackageLoader::new( manager, @@ -277,7 +271,6 @@ fn test_pretty_version_for_root_package_in_version_branch() { .load(IndexMap::new(), "Composer\\Package\\RootPackage", None) .unwrap(); - assert!(guess_version_calls.get() >= 1); assert_eq!("3.0-dev", package.as_root().unwrap().get_pretty_version()); } diff --git a/crates/shirabe/tests/package/loader/validating_array_loader_test.rs b/crates/shirabe/tests/package/loader/validating_array_loader_test.rs index 2394af2..aeafefd 100644 --- a/crates/shirabe/tests/package/loader/validating_array_loader_test.rs +++ b/crates/shirabe/tests/package/loader/validating_array_loader_test.rs @@ -1,8 +1,5 @@ //! ref: composer/tests/Composer/Test/Package/Loader/ValidatingArrayLoaderTest.php -use std::cell::RefCell; -use std::rc::Rc; - use indexmap::IndexMap; use shirabe::package::handle::PackageInterfaceHandle; use shirabe::package::loader::{InvalidPackageException, LoaderInterface, ValidatingArrayLoader}; @@ -49,40 +46,29 @@ fn config(entries: Vec<(&str, PhpMixed)>) -> IndexMap { m } -type Calls = Rc>>>; - -/// Mock LoaderInterface recording every `load` invocation, mirroring PHPUnit's -/// `expects($this->once())->method('load')->with(...)`. -#[derive(Debug)] -struct MockLoader { - calls: Calls, -} - -impl MockLoader { - fn new() -> (Self, Calls) { - let calls: Calls = Rc::new(RefCell::new(Vec::new())); - ( - MockLoader { - calls: calls.clone(), - }, - calls, - ) +// PHP mocks `Composer\Package\Loader\LoaderInterface` with getMockBuilder. +mockall::mock! { + #[derive(Debug)] + Loader {} + impl LoaderInterface for Loader { + fn load( + &self, + config: IndexMap, + class: Option, + ) -> anyhow::Result; + fn as_any(&self) -> &dyn std::any::Any; } } -impl LoaderInterface for MockLoader { - fn load( - &self, - config: IndexMap, - _class: Option, - ) -> anyhow::Result { - self.calls.borrow_mut().push(config); - Ok(test_case::get_package("mock/mock", "1.0.0")) - } - - fn as_any(&self) -> &dyn std::any::Any { - self - } +/// Build a mock inner loader whose `load` returns a dummy package, mirroring +/// PHPUnit's `getMockBuilder(LoaderInterface::class)->getMock()` with no +/// configured expectations. +fn mock_loader() -> MockLoader { + let mut loader = MockLoader::new(); + loader + .expect_load() + .returning(|_, _| Ok(test_case::get_package("mock/mock", "1.0.0"))); + loader } fn invalid_naming_error(name: &str) -> Vec { @@ -369,7 +355,7 @@ fn success_provider() -> Vec> { #[test] fn test_load_success() { for cfg in success_provider() { - let (internal_loader, _calls) = MockLoader::new(); + let internal_loader = mock_loader(); let mut loader = ValidatingArrayLoader::new( Box::new(internal_loader), true, @@ -796,7 +782,7 @@ fn error_provider() -> Vec<(IndexMap, Vec)> { #[test] fn test_load_failure_throws_exception() { for (cfg, mut expected_errors) in error_provider() { - let (internal_loader, _calls) = MockLoader::new(); + let internal_loader = mock_loader(); let mut loader = ValidatingArrayLoader::new( Box::new(internal_loader), true, @@ -978,7 +964,7 @@ fn warning_provider() -> Vec<( #[test] fn test_load_warnings() { for (cfg, mut expected_warnings, _must_check, _expected_array) in warning_provider() { - let (internal_loader, _calls) = MockLoader::new(); + let internal_loader = mock_loader(); let mut loader = ValidatingArrayLoader::new( Box::new(internal_loader), true, @@ -1003,9 +989,16 @@ fn test_load_skips_warning_data_when_ignoring_errors() { if !must_check { continue; } - let (internal_loader, calls) = MockLoader::new(); let expected = expected_array.unwrap_or_else(|| config(vec![("name", s("a/b"))])); + // The inner loader is called exactly once with the (post-validation) config. + let mut internal_loader = MockLoader::new(); + internal_loader + .expect_load() + .times(1) + .withf(move |cfg, _class| *cfg == expected) + .returning(|_, _| Ok(test_case::get_package("mock/mock", "1.0.0"))); + let mut loader = ValidatingArrayLoader::new( Box::new(internal_loader), true, @@ -1016,10 +1009,5 @@ fn test_load_skips_warning_data_when_ignoring_errors() { loader .load(cfg, "Composer\\Package\\CompletePackage") .unwrap(); - - // The mock recorded exactly the (post-validation) config passed to the inner loader. - let recorded = calls.borrow(); - assert_eq!(recorded.len(), 1); - assert_eq!(recorded[0], expected); } } -- cgit v1.3.1