diff options
| author | nsfisis <nsfisis@gmail.com> | 2026-08-02 08:55:56 +0900 |
|---|---|---|
| committer | nsfisis <nsfisis@gmail.com> | 2026-08-02 08:55:56 +0900 |
| commit | 2c3373a7466c31f5cce7c2867c00a91afaa8e128 (patch) | |
| tree | 754c7da92def5d14fc425230f9db41c654897b4f /crates/shirabe/tests | |
| parent | 92f5c977b42d1322f3cebb260024179e1285b9d0 (diff) | |
| download | php-shirabe-2c3373a7466c31f5cce7c2867c00a91afaa8e128.tar.gz php-shirabe-2c3373a7466c31f5cce7c2867c00a91afaa8e128.tar.zst php-shirabe-2c3373a7466c31f5cce7c2867c00a91afaa8e128.zip | |
fix(io): propagate ask/select errors instead of panicking
IOInterface::ask/select now return anyhow::Result<PhpMixed>, and
ConsoleIO::ask_question forwards QuestionHelper errors (validator
failures, MissingInputException) instead of collapsing them with
.expect(). In PHP these exceptions propagate from QuestionHelper
through ConsoleIO to the caller, so callers such as UpdateCommand's
interactive package selection must be able to observe them; the
MissingInputException is wrapped with its concrete type preserved so
Application's ExceptionInterface downcast keeps working. All call
sites now propagate with `?` (Perforce::query_p4_user becomes
Result-returning: PHP declares it void but exceptions still escape),
and the previously ignored
test_interactive_mode_throws_if_no_package_entered passes.
ask_confirmation/ask_and_hide_answer still collapse errors; extending
propagation to them is left as TODO(phase-c) pending a decision.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Diffstat (limited to 'crates/shirabe/tests')
| -rw-r--r-- | crates/shirabe/tests/command/update_command_test.rs | 5 | ||||
| -rw-r--r-- | crates/shirabe/tests/common/io_mock.rs | 15 | ||||
| -rw-r--r-- | crates/shirabe/tests/common/io_stub.rs | 8 | ||||
| -rw-r--r-- | crates/shirabe/tests/io/buffer_io_test.rs | 10 | ||||
| -rw-r--r-- | crates/shirabe/tests/io/console_io_test.rs | 6 | ||||
| -rw-r--r-- | crates/shirabe/tests/io/null_io_test.rs | 2 | ||||
| -rw-r--r-- | crates/shirabe/tests/util/perforce_test.rs | 16 |
7 files changed, 33 insertions, 29 deletions
diff --git a/crates/shirabe/tests/command/update_command_test.rs b/crates/shirabe/tests/command/update_command_test.rs index 1b36cadc..3aa42880 100644 --- a/crates/shirabe/tests/command/update_command_test.rs +++ b/crates/shirabe/tests/command/update_command_test.rs @@ -380,11 +380,6 @@ fn test_interactive_mode_throws_if_no_package_to_update() { #[test] #[serial] -#[ignore = "ConsoleIO::ask_question panics via .expect() on any QuestionHelper validator error \ - instead of propagating it, so io.select()'s \"No package named ...\" validation \ - error can't reach app_tester.run() as an Err; fixing this needs IOInterface::ask()/ \ - select() to return anyhow::Result, which ripples through ~18 ask() and ~4 select() \ - call sites (see project_console_io_ask_select_panics memory)"] fn test_interactive_mode_throws_if_no_package_entered() { let composer_json = serde_json::json!({ "repositories": { "packages": { "type": "package", "package": [ diff --git a/crates/shirabe/tests/common/io_mock.rs b/crates/shirabe/tests/common/io_mock.rs index a658c5bc..ad8d72cd 100644 --- a/crates/shirabe/tests/common/io_mock.rs +++ b/crates/shirabe/tests/common/io_mock.rs @@ -224,7 +224,7 @@ impl IOInterfaceImmutable for IOMock { .overwrite_error4(message, newline, size, verbosity) } - fn ask(&self, question: String, default: PhpMixed) -> PhpMixed { + fn ask(&self, question: String, default: PhpMixed) -> anyhow::Result<PhpMixed> { self.inner .ask(format!("{}{}", trim_eol(&question), PHP_EOL), default) } @@ -249,10 +249,13 @@ impl IOInterfaceImmutable for IOMock { fn ask_and_hide_answer(&self, question: String) -> Option<String> { // Do not hide the answer in tests because that blocks on Windows with // hiddeninput.exe, so PHP delegates to `ask` rather than the hidden variant. - let result = self.inner.ask( - format!("{}{}", trim_eol(&question), PHP_EOL), - PhpMixed::Null, - ); + let result = self + .inner + .ask( + format!("{}{}", trim_eol(&question), PHP_EOL), + PhpMixed::Null, + ) + .expect("QuestionHelper::ask raised an error"); result.as_string().map(|s| s.to_string()) } fn select( @@ -263,7 +266,7 @@ impl IOInterfaceImmutable for IOMock { attempts: PhpMixed, error_message: String, multiselect: bool, - ) -> PhpMixed { + ) -> anyhow::Result<PhpMixed> { self.inner.select( format!("{}{}", trim_eol(&question), PHP_EOL), choices, diff --git a/crates/shirabe/tests/common/io_stub.rs b/crates/shirabe/tests/common/io_stub.rs index b897305e..9feeb5a3 100644 --- a/crates/shirabe/tests/common/io_stub.rs +++ b/crates/shirabe/tests/common/io_stub.rs @@ -217,8 +217,8 @@ impl IOInterfaceImmutable for IOStub { ) { } - fn ask(&self, _question: String, default: PhpMixed) -> PhpMixed { - self.ask.clone().unwrap_or(default) + fn ask(&self, _question: String, default: PhpMixed) -> anyhow::Result<PhpMixed> { + Ok(self.ask.clone().unwrap_or(default)) } fn ask_confirmation(&self, _question: String, default: bool) -> bool { self.ask_confirmation.unwrap_or(default) @@ -251,8 +251,8 @@ impl IOInterfaceImmutable for IOStub { _attempts: PhpMixed, _error_message: String, _multiselect: bool, - ) -> PhpMixed { - default + ) -> anyhow::Result<PhpMixed> { + Ok(default) } fn get_authentications( diff --git a/crates/shirabe/tests/io/buffer_io_test.rs b/crates/shirabe/tests/io/buffer_io_test.rs index 5bb18014..c33fd804 100644 --- a/crates/shirabe/tests/io/buffer_io_test.rs +++ b/crates/shirabe/tests/io/buffer_io_test.rs @@ -19,9 +19,11 @@ fn test_set_user_inputs() { assert!(!buffer_io.ask_confirmation("Now please say no!".to_string(), true)); assert_eq!( PhpMixed::String("default".to_string()), - buffer_io.ask( - "Empty string last".to_string(), - PhpMixed::String("default".to_string()) - ) + buffer_io + .ask( + "Empty string last".to_string(), + PhpMixed::String("default".to_string()) + ) + .unwrap() ); } diff --git a/crates/shirabe/tests/io/console_io_test.rs b/crates/shirabe/tests/io/console_io_test.rs index 03307e51..15e7100b 100644 --- a/crates/shirabe/tests/io/console_io_test.rs +++ b/crates/shirabe/tests/io/console_io_test.rs @@ -196,7 +196,9 @@ fn test_ask() { // PHP asserts QuestionHelper::ask receives a Question. Behaviorally, an interactive input whose // stream yields the answer makes ConsoleIO::ask return that answer. let (console_io, _output) = make_console_io_with_answer("answer\n"); - let result = console_io.ask("Why?".to_string(), PhpMixed::String("default".to_string())); + let result = console_io + .ask("Why?".to_string(), PhpMixed::String("default".to_string())) + .unwrap(); assert_eq!(PhpMixed::String("answer".to_string()), result); } @@ -247,7 +249,7 @@ fn test_select() { ); assert_eq!( PhpMixed::List(vec![PhpMixed::String("1".to_string())]), - result + result.unwrap() ); } diff --git a/crates/shirabe/tests/io/null_io_test.rs b/crates/shirabe/tests/io/null_io_test.rs index 1627e91f..ced598b9 100644 --- a/crates/shirabe/tests/io/null_io_test.rs +++ b/crates/shirabe/tests/io/null_io_test.rs @@ -45,6 +45,7 @@ fn test_ask() { assert_eq!( PhpMixed::String("foo".to_string()), io.ask("bar".to_string(), PhpMixed::String("foo".to_string())) + .unwrap() ); } @@ -88,5 +89,6 @@ fn test_select() { "foo".to_string(), true ) + .unwrap() ); } diff --git a/crates/shirabe/tests/util/perforce_test.rs b/crates/shirabe/tests/util/perforce_test.rs index b63a5e0e..293e56d5 100644 --- a/crates/shirabe/tests/util/perforce_test.rs +++ b/crates/shirabe/tests/util/perforce_test.rs @@ -184,7 +184,7 @@ fn test_generate_p4_command() { fn test_query_p4_user_with_user_already_set() { let mut perforce = create_new_perforce_with_windows_flag(true); - perforce.query_p4_user(); + perforce.query_p4_user().unwrap(); assert_eq!(Some(TEST_P4USER.to_string()), perforce.get_user()); } @@ -205,7 +205,7 @@ fn test_query_p4_user_with_user_set_in_p4_variables_with_windows_os() { let mut perforce = create_perforce(true, process, io); perforce.set_user(None); - perforce.query_p4_user(); + perforce.query_p4_user().unwrap(); assert_eq!( Some("TEST_P4VARIABLE_USER".to_string()), perforce.get_user() @@ -229,7 +229,7 @@ fn test_query_p4_user_with_user_set_in_p4_variables_not_windows_os() { let mut perforce = create_perforce(false, process, io); perforce.set_user(None); - perforce.query_p4_user(); + perforce.query_p4_user().unwrap(); assert_eq!( Some("TEST_P4VARIABLE_USER".to_string()), perforce.get_user() @@ -248,7 +248,7 @@ fn test_query_p4_user_queries_for_user() { let mut perforce = create_perforce(true, process, io); perforce.set_user(None); - perforce.query_p4_user(); + perforce.query_p4_user().unwrap(); assert_eq!(Some("TEST_QUERY_USER".to_string()), perforce.get_user()); } @@ -270,7 +270,7 @@ fn test_query_p4_user_stores_response_to_query_for_user_with_windows() { let mut perforce = create_perforce(true, process, io); perforce.set_user(None); - perforce.query_p4_user(); + perforce.query_p4_user().unwrap(); } #[test] @@ -294,7 +294,7 @@ fn test_query_p4_user_stores_response_to_query_for_user_without_windows() { let mut perforce = create_perforce(false, process, io); perforce.set_user(None); - perforce.query_p4_user(); + perforce.query_p4_user().unwrap(); } #[test] @@ -315,7 +315,7 @@ fn test_query_p4_user_escapes_injection_on_windows() { let mut perforce = create_perforce(true, process, io); perforce.set_user(None); - perforce.query_p4_user(); + perforce.query_p4_user().unwrap(); } #[test] @@ -335,7 +335,7 @@ fn test_query_p4_user_escapes_injection_on_unix() { let mut perforce = create_perforce(false, process, io); perforce.set_user(None); - perforce.query_p4_user(); + perforce.query_p4_user().unwrap(); } #[test] |
