Python log unit-tests, with hang fix and more - #11237
Conversation
| rs.log_to_callback( rs.log_severity.debug, common.message_counter ) | ||
| test.check_equal( common.n_messages, 0 ) | ||
| # Following will throw a recoverable_exception, which will issue a log by itself! | ||
| test.check_throws( lambda: rs.log( rs.log_severity( 10 ), "10" ), RuntimeError, 'invalid enum value for argument "severity"' ) |
There was a problem hiding this comment.
This cast, and the one in the next line, work on my system: I get a RuntimeError from the C++.
But if you look, our LibCI somehow fails with a TypeError in the Python -- the cast cannot be done in earlier Python versions, it seems...
Any preference on how to deal with this?
There was a problem hiding this comment.
I would make the exception type optional.
Normally I want to check the function throws on ilegal operation, I don't care what kind of exception.
There was a problem hiding this comment.
No, the whole point is to check the C++ error. Saying "this Python code may fail" invalidates the whole test.
I traced it to this:
Python 3.6 fails for the -1 value, but works for, e.g., 10 — both are invalid enum values. It gives:
TypeError: __init__(): incompatible constructor arguments. The following argument types are supported:
1. pyrealsense2.pyrealsense2.log_severity(value: int)
Invoked with: -1
Somehow -1 isn't an int?
Python 3.9 works for both, even with negative values. So they fixed something it seems...
I'll comment out the -1 line and add explanation...
There was a problem hiding this comment.
OK, nut I still think the check_throw parameter should be optional :)
I know this should throw and want to test it, but I dont care about the exception type normally, don't you think?
If I care I can check it , it's optional :)
There was a problem hiding this comment.
Depends how anal you want to be :)
If we were lax like that, I would not have caught the problem with the Python. And that would have been a bug...
I think it's currently generic enough and specific enough, at the same time.
There was a problem hiding this comment.
How is it generic enough?
I know this function should throw but I am not sure what exception type will the python get.
You make me run it, verify, and they specifically write it in the test, sounds like a punishment :)
I just wanted to make sure it throws..
But that's not related to this PR
There was a problem hiding this comment.
I think it should be explicit :)
Started getting this message:
Could not load empty file for logging, please re-check your configurations for level [TRACE]
In the python log tests...
Looking online, I see PRs addressing this. The code tries to create a
file even when the log file is empty. This is a workaround.
NOTE: ELPP hasn't had any commits since Feb'21...
Fix hang on python exit after
log_to_callback().Replace almost all of the C/++ tests with Python.
Fixes some small issue in ELPP.
Fix
pyrs.test.set_env_vars().