-
Notifications
You must be signed in to change notification settings - Fork 117
[feat] Add configuration parameter for automatically capturing command errors and exit code in job scripts #1490
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
Codecov Report
@@ Coverage Diff @@
## master #1490 +/- ##
==========================================
+ Coverage 91.65% 91.72% +0.07%
==========================================
Files 83 83
Lines 13012 13029 +17
==========================================
+ Hits 11926 11951 +25
+ Misses 1086 1078 -8
Continue to review full report at Codecov.
|
vkarak
left a comment
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.
We will also need a unit test.
|
Does it make sense to allow the check to bypass this configuration? Something like self.trap_job_errors = FalseThe use case is when one needs to write a check where one expects a command to exit code different than zero, while the default config is set to |
|
@victorusu I would see it as a job parameter rather than a test parameter. I'm generally against bloating either API, though, unless we need to. If that comes as a feature request from a real scenario, then we can discuss it again. |
vkarak
left a comment
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.
lgtm now
Will fix #1196