integration-tests: Remove ignore_status() that hides failures#203
Conversation
The integration tests were using ignore_status() in ways that could hide actual test failures; clean up the suite. Assisted-by: OpenCode (Claude claude-opus-4-5-20250114) Signed-off-by: Colin Walters <walters@verbum.org>
There was a problem hiding this comment.
Code Review
This pull request significantly improves the robustness and readability of the integration tests by refactoring command execution and resource cleanup. The removal of the custom CapturedOutput struct and the ignore_status() calls, in favor of xshell's read()? and run()? methods, simplifies error handling and ensures that command failures are explicitly propagated as test failures. The widespread adoption of scopeguard::defer! macros is a particularly strong improvement, guaranteeing that test resources are consistently cleaned up, even in the event of early exits or panics. This greatly enhances test reliability and reduces the likelihood of resource leaks. Additionally, replacing arbitrary std::thread::sleep calls with the --ssh-wait flag in relevant tests makes the waiting logic more precise and efficient. Overall, these changes contribute to a more maintainable and dependable test suite.
The integration tests were using ignore_status() in ways that could hide actual test failures; clean up the suite.
Assisted-by: OpenCode (Claude claude-opus-4-5-20250114)