-
Notifications
You must be signed in to change notification settings - Fork 43
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
feat: Improve error reporting in test case execution #111
feat: Improve error reporting in test case execution #111
Conversation
Codecov Report
@@ Coverage Diff @@
## master #111 +/- ##
==========================================
+ Coverage 93.09% 94.89% +1.79%
==========================================
Files 69 68 -1
Lines 4955 4954 -1
==========================================
+ Hits 4613 4701 +88
+ Misses 342 253 -89
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
2700f2c
to
c50d089
Compare
@@ -252,6 +252,9 @@ def execute( | |||
log.exception("Keyboard Interrupt detected") | |||
exit_code = ExitCode.ONE_OR_MORE_TESTS_RAISED_UNEXPECTED_EXCEPTION | |||
except Exception: | |||
log.exception(f'Issue detected in the test-suite: {config["test_suite_list"]}!') | |||
log.exception( |
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.
I don't get this, the principle of log.exception is to log the message at error level AND to print out the exception stack, so even without this change you'd see all of the errors
src/pykiso/exceptions.py
Outdated
|
||
|
||
class PykisoError(Exception): | ||
"""Pykiso specific exception used as basis for all others.""" | ||
|
||
def __str__(self): | ||
logging.exception(self.message) |
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.
As discussed don't mix up logging and exceptions, just add the log.exception when the exceptions are caught
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.
Ok, right just wanted to display the full message, as I didn't know it will be anyway
6a85519
to
fd7ab03
Compare
9d5b462
to
40e022c
Compare
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.
Besides the above question, it looks good :)
40e022c
to
a00490e
Compare
Co-authored-by: Julien THOMAS <external.julien.thomas@de.bosch.com>
Co-authored-by: Julien THOMAS <external.julien.thomas@de.bosch.com>
Co-authored-by: Julien THOMAS <external.julien.thomas@de.bosch.com>
Co-authored-by: Julien THOMAS <external.julien.thomas@de.bosch.com>
No description provided.