-
Notifications
You must be signed in to change notification settings - Fork 2
π Fix assert_exit_code, improve exit code handling for all #27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes critical bugs in exit code handling for test assertion functions (assert_success, assert_failure, and assert_exit_code) and improves their reliability when used with shell option set -e.
Key Changes:
- Fixed incorrect exit code capture logic that was broken in the old implementations (exit codes were being captured from the wrong commands)
- Added proper
set -eoption handling to prevent premature script termination when assertions run commands that may fail - Applied consistent implementation pattern across all three assertion functions
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| libs/core/test/assert_success | Fixed exit code capture and added set -e handling to properly test commands that should succeed |
| libs/core/test/assert_failure | Fixed exit code capture and added set -e handling to properly test commands that should fail |
| libs/core/test/assert_exit_code | Added set -e handling to properly test commands with specific exit codes |
π‘ Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
π‘ Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
π What
Fix exit code and options handling in
core::test::assert_exit_code.Improve exit code and options handling in all similar assert functions.
β Why
As tests are getting added, tests that use
core::test::assert_exit_code, all assert functions need to work reliably.β‘ How to Review
β Testing