aboutsummaryrefslogtreecommitdiffhomepage
path: root/crates/shirabe/tests/downloader
diff options
context:
space:
mode:
authornsfisis <nsfisis@gmail.com>2026-07-20 01:33:14 +0900
committernsfisis <nsfisis@gmail.com>2026-07-20 01:33:14 +0900
commit5fac1bba14ad5b9e4d2abfa3ca299f88cc1e62bf (patch)
tree895d808df734f16b6201e610739a67dfa317d11d /crates/shirabe/tests/downloader
parent9a2cee2532e5d28d5cea5726f35ab3c7046e7c0d (diff)
downloadphp-shirabe-5fac1bba14ad5b9e4d2abfa3ca299f88cc1e62bf.tar.gz
php-shirabe-5fac1bba14ad5b9e4d2abfa3ca299f88cc1e62bf.tar.zst
php-shirabe-5fac1bba14ad5b9e4d2abfa3ca299f88cc1e62bf.zip
fix(process-executor): un-ignore 11 tests by implementing execute_async mock support
ProcessExecutor::execute_async's mock branch was an unimplemented todo!(), blocking every test whose code path calls it (feature-branch git diffing, system-unzip/7z fallback extraction). Implement it by: - Adding Process::__mock (mirroring the existing ZipArchive::__mock pattern) so execute_async can resolve with a fabricated, already- terminated Process instead of spawning a real subprocess. - Extracting the sync mock's expectation-matching logic into a shared ProcessExecutor::mock_match, wrapping the mock state in a RefCell so it works from execute_async's &self/&mut self receivers. - Setting error_output/capture_output from the mock branch, matching PHP's ProcessExecutorMock::executeAsync sharing doExecute with the sync path; execute_async now takes &mut self for this (safe, since the borrow only needs to live through the synchronous setup, not across the .await). - Turning a strict-mode expectation mismatch from a panic!() into a shirabe_php_shim::RuntimeException Err, mirroring PHPUnit's AssertionFailedError extending \RuntimeException: PHP call sites that catch (\RuntimeException $e) around a mocked git/hg/svn call (e.g. Git::get_mirror_default_branch, GitDriver::supports) treat a mismatch as an ordinary recoverable failure, and now so does the port. The RefMut is dropped before firing an expectation's optional callback so a re-entrant callback doesn't panic on double-borrow. Also fixes two real bugs found while porting test_private_repository_ no_interaction: GitHub::authorize_oauth and GitLab::authorize_oauth checked their domains config via PhpMixed::as_array(), which only matches the Array (map) variant, but github-domains/gitlab-domains default to PhpMixed::List, so the check always returned false and OAuth token lookup was silently skipped. Use the in_array shim instead, matching PHP's in_array() semantics. Also fix Git::run_command's "capture credentials from git remote -v" call, which used the panic- swallowing execute_args wrapper instead of a fallible execute(), so a mock mismatch there couldn't reach get_mirror_default_branch's catch. Un-ignores: - zip_downloader_test::test_system_unzip_only_{good,failed} - zip_downloader_test::test_non_windows_fallback_{good,failed} - event_dispatcher_test::test_dispatcher_outputs_error_on_failed_command - root_package_loader_test::test_feature_branch_pretty_version - version_guesser_test::test_guess_version_reads_and_respects_non_feature_branches_configuration_for_arbitrary_naming{,_regex} - version_guesser_test::test_remote_branches_are_selected - github_driver_test::test_private_repository_no_interaction (also adds the missing #[serial], since it seeds the shared Git::VERSION static that vcs_repository_test::test_load_versions depends on for real) - init_command_test::test_get_git_config, made deterministic by pointing HOME at a throwaway dir with its own .gitconfig instead of depending on the host's global git config Deduplicates the GitVersionGuard/RestoreEnv test-drop-guard idioms into tests/common/test_case.rs instead of reimplementing them per file. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Diffstat (limited to 'crates/shirabe/tests/downloader')
-rw-r--r--crates/shirabe/tests/downloader/zip_downloader_test.rs163
1 files changed, 137 insertions, 26 deletions
diff --git a/crates/shirabe/tests/downloader/zip_downloader_test.rs b/crates/shirabe/tests/downloader/zip_downloader_test.rs
index 593f3e76..338b2b54 100644
--- a/crates/shirabe/tests/downloader/zip_downloader_test.rs
+++ b/crates/shirabe/tests/downloader/zip_downloader_test.rs
@@ -14,6 +14,7 @@ use shirabe::util::HttpDownloader;
use shirabe::util::ProcessExecutor;
use shirabe::util::filesystem::Filesystem;
use shirabe::util::r#loop::Loop;
+use shirabe::util::process_executor::MockHandler;
use shirabe_php_shim::{PhpMixed, ZipArchive, ZipArchiveMock};
use shirabe_semver::VersionParser;
use tempfile::TempDir;
@@ -87,10 +88,17 @@ impl Drop for TearDown {
}
fn make_downloader(set_up: &SetUp) -> ZipDownloader {
- let filesystem = std::rc::Rc::new(std::cell::RefCell::new(Filesystem::new(None)));
let process = std::rc::Rc::new(std::cell::RefCell::new(ProcessExecutor::new(Some(
set_up.io.clone(),
))));
+ make_downloader_with_process(set_up, process)
+}
+
+fn make_downloader_with_process(
+ set_up: &SetUp,
+ process: std::rc::Rc<std::cell::RefCell<ProcessExecutor>>,
+) -> ZipDownloader {
+ let filesystem = std::rc::Rc::new(std::cell::RefCell::new(Filesystem::new(None)));
ZipDownloader::new(
set_up.io.clone(),
set_up.config.clone(),
@@ -102,11 +110,6 @@ fn make_downloader(set_up: &SetUp) -> ZipDownloader {
)
}
-// The system-unzip / non-windows-fallback paths route through ProcessExecutor::execute_async, whose
-// mock branch is an unimplemented todo!() (no Process mock seam exists in the external-packages
-// crate). The PHP tests below mock Process/ProcessExecutor::executeAsync, which is not reproducible
-// here, so they remain ignored.
-//
// testErrorMessages drives a real HttpDownloader + Loop, but RemoteFilesystem::get_remote_contents
// is a phase-c stub returning None, so the file:// dist download fails before the ZipArchive path.
@@ -266,38 +269,146 @@ fn test_zip_archive_only_good() {
result.expect("extract should succeed");
}
-#[ignore = "routes through ProcessExecutor::execute_async whose mock branch is todo!() (no Process mock seam in external-packages)"]
+// setPrivateProperty('unzipCommands', [['unzip', 'unzip -qq %s -d %s']]) in PHP: a single
+// two-element commandSpec (executable name, then one literal arg string that contains %s
+// placeholders rather than %file%/%path%, so it is passed through to executeAsync verbatim). The
+// PHPUnit test fully replaces $processExecutor, so the exact command content is never asserted on;
+// this only needs to be non-empty so extractWithSystemUnzip proceeds past the "no commands"
+// short-circuit into ZipDownloader::extract_with_zip_archive.
+fn unzip_command_spec() -> Vec<Vec<String>> {
+ vec![vec!["unzip".to_string(), "unzip -qq %s -d %s".to_string()]]
+}
+
#[test]
+#[serial]
fn test_system_unzip_only_failed() {
- let _ = set_up();
- // TODO(phase-d): routes through ProcessExecutor::execute_async, whose mock branch is
- // todo!() (no Process mock seam exists in the external-packages crate).
- todo!()
+ let set_up = set_up();
+ let _tear_down = TearDown::new(set_up.test_dir.path().to_path_buf());
+
+ ZipDownloader::__set_is_windows(Some(false));
+ ZipDownloader::__set_has_zip_archive(Some(false));
+ ZipDownloader::__set_unzip_commands(Some(unzip_command_spec()));
+
+ let process = std::rc::Rc::new(std::cell::RefCell::new(ProcessExecutor::new(None)));
+ process.borrow_mut().__expects(
+ vec![],
+ false,
+ MockHandler {
+ r#return: 1,
+ stdout: String::new(),
+ stderr: "output".to_string(),
+ },
+ );
+ let downloader = make_downloader_with_process(&set_up, process);
+
+ let filename = set_up.filename.to_string_lossy().into_owned();
+ let result = run(downloader.extract(set_up.package.clone(), &filename, "vendor/dir"));
+
+ let e = result.expect_err("expected RuntimeException");
+ assert!(
+ e.to_string()
+ .contains("Failed to extract test/pkg: (1) unzip"),
+ "got: {e}"
+ );
}
-#[ignore = "routes through ProcessExecutor::execute_async whose mock branch is todo!() (no Process mock seam in external-packages)"]
#[test]
+#[serial]
fn test_system_unzip_only_good() {
- let _ = set_up();
- // TODO(phase-d): routes through ProcessExecutor::execute_async, whose mock branch is
- // todo!() (no Process mock seam exists in the external-packages crate).
- todo!()
+ let set_up = set_up();
+ let _tear_down = TearDown::new(set_up.test_dir.path().to_path_buf());
+
+ ZipDownloader::__set_is_windows(Some(false));
+ ZipDownloader::__set_has_zip_archive(Some(false));
+ ZipDownloader::__set_unzip_commands(Some(unzip_command_spec()));
+
+ let process = std::rc::Rc::new(std::cell::RefCell::new(ProcessExecutor::new(None)));
+ process.borrow_mut().__expects(
+ vec![],
+ false,
+ MockHandler {
+ r#return: 0,
+ stdout: String::new(),
+ stderr: "output".to_string(),
+ },
+ );
+ let downloader = make_downloader_with_process(&set_up, process);
+
+ let filename = set_up.filename.to_string_lossy().into_owned();
+ let result = run(downloader.extract(set_up.package.clone(), &filename, "vendor/dir"));
+
+ result.expect("extract should succeed");
}
-#[ignore = "routes through ProcessExecutor::execute_async whose mock branch is todo!() (no Process mock seam in external-packages)"]
#[test]
+#[serial]
fn test_non_windows_fallback_good() {
- let _ = set_up();
- // TODO(phase-d): routes through ProcessExecutor::execute_async, whose mock branch is
- // todo!() (no Process mock seam exists in the external-packages crate).
- todo!()
+ let set_up = set_up();
+ let _tear_down = TearDown::new(set_up.test_dir.path().to_path_buf());
+
+ ZipDownloader::__set_is_windows(Some(false));
+ ZipDownloader::__set_has_zip_archive(Some(true));
+ ZipDownloader::__set_unzip_commands(Some(unzip_command_spec()));
+
+ let process = std::rc::Rc::new(std::cell::RefCell::new(ProcessExecutor::new(None)));
+ process.borrow_mut().__expects(
+ vec![],
+ false,
+ MockHandler {
+ r#return: 1,
+ stdout: String::new(),
+ stderr: "output".to_string(),
+ },
+ );
+ let downloader = make_downloader_with_process(&set_up, process);
+ let zip_archive = ZipArchive::__mock(ZipArchiveMock {
+ open: Ok(()),
+ count: 0,
+ extract_to: Ok(true),
+ });
+ downloader.__set_zip_archive_object(Some(zip_archive));
+
+ let filename = set_up.filename.to_string_lossy().into_owned();
+ let result = run(downloader.extract(set_up.package.clone(), &filename, "vendor/dir"));
+
+ result.expect("extract should succeed");
}
-#[ignore = "routes through ProcessExecutor::execute_async whose mock branch is todo!() (no Process mock seam in external-packages)"]
#[test]
+#[serial]
fn test_non_windows_fallback_failed() {
- let _ = set_up();
- // TODO(phase-d): routes through ProcessExecutor::execute_async, whose mock branch is
- // todo!() (no Process mock seam exists in the external-packages crate).
- todo!()
+ let set_up = set_up();
+ let _tear_down = TearDown::new(set_up.test_dir.path().to_path_buf());
+
+ ZipDownloader::__set_is_windows(Some(false));
+ ZipDownloader::__set_has_zip_archive(Some(true));
+ ZipDownloader::__set_unzip_commands(Some(unzip_command_spec()));
+
+ let process = std::rc::Rc::new(std::cell::RefCell::new(ProcessExecutor::new(None)));
+ process.borrow_mut().__expects(
+ vec![],
+ false,
+ MockHandler {
+ r#return: 1,
+ stdout: String::new(),
+ stderr: "output".to_string(),
+ },
+ );
+ let downloader = make_downloader_with_process(&set_up, process);
+ let zip_archive = ZipArchive::__mock(ZipArchiveMock {
+ open: Ok(()),
+ count: 0,
+ extract_to: Ok(false),
+ });
+ downloader.__set_zip_archive_object(Some(zip_archive));
+
+ let filename = set_up.filename.to_string_lossy().into_owned();
+ let result = run(downloader.extract(set_up.package.clone(), &filename, "vendor/dir"));
+
+ let e = result.expect_err("expected RuntimeException");
+ assert!(
+ e.to_string()
+ .contains("There was an error extracting the ZIP file"),
+ "got: {e}"
+ );
}