From 5fac1bba14ad5b9e4d2abfa3ca299f88cc1e62bf Mon Sep 17 00:00:00 2001 From: nsfisis Date: Mon, 20 Jul 2026 01:33:14 +0900 Subject: 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 --- .../src/symfony/process/process.rs | 38 ++++++++++++++++++++++ 1 file changed, 38 insertions(+) (limited to 'crates/shirabe-external-packages/src/symfony/process') diff --git a/crates/shirabe-external-packages/src/symfony/process/process.rs b/crates/shirabe-external-packages/src/symfony/process/process.rs index ecd40e7a..5c817c85 100644 --- a/crates/shirabe-external-packages/src/symfony/process/process.rs +++ b/crates/shirabe-external-packages/src/symfony/process/process.rs @@ -29,6 +29,18 @@ enum CommandLine { String(String), } +/// Test-only behaviour for a Process fabricated via [`Process::__mock`]: `getOutput`/ +/// `getErrorOutput`/`getExitCode`/`isSuccessful` return these fixed values instead of reading a +/// real subprocess. Mirrors PHPUnit's `getMockBuilder(Process::class)->disableOriginalConstructor()` +/// mocks used by the Composer test suite (e.g. `ZipDownloaderTest`). Held in [`Process::mock`]; +/// always `None` in production. +#[derive(Debug, Clone)] +pub struct ProcessMock { + pub exit_code: i64, + pub stdout: String, + pub stderr: String, +} + /// Process is a thin wrapper around proc_* functions to easily /// start independent PHP processes. pub struct Process { @@ -59,6 +71,8 @@ pub struct Process { process_pipes: Option>, latest_signal: Option, cached_exit_code: Option, + /// Test-only mock state. `None` in production; set via [`Process::__mock`] in tests. + mock: Option, } impl std::fmt::Debug for Process { @@ -198,9 +212,21 @@ impl Process { process_pipes: None, latest_signal: None, cached_exit_code: None, + mock: None, } } + /// For testing only. Builds an already-terminated mock Process whose getOutput/ + /// getErrorOutput/getExitCode/isSuccessful return the configured values, without spawning a + /// real subprocess. + pub fn __mock(mock: ProcessMock) -> Self { + let mut this = Self::empty(); + this.status = Self::STATUS_TERMINATED.to_string(); + this.exitcode = Some(mock.exit_code); + this.mock = Some(mock); + this + } + pub fn new( command: Vec, cwd: Option, @@ -612,6 +638,10 @@ impl Process { /// Returns the current output of the process (STDOUT). pub fn get_output(&mut self) -> anyhow::Result { + if let Some(mock) = &self.mock { + return Ok(mock.stdout.clone()); + } + self.read_pipes_for_output("getOutput", false)?; Ok( @@ -718,6 +748,10 @@ impl Process { /// Returns the current error output of the process (STDERR). pub fn get_error_output(&mut self) -> anyhow::Result { + if let Some(mock) = &self.mock { + return Ok(mock.stderr.clone()); + } + self.read_pipes_for_output("getErrorOutput", false)?; Ok( @@ -752,6 +786,10 @@ impl Process { /// Returns the exit code returned by the process. pub fn get_exit_code(&mut self) -> Option { + if self.mock.is_some() { + return self.exitcode; + } + self.update_status(false); self.exitcode -- cgit v1.3.1