Fix built-in method raise exception (#118) #121
Conversation
tests/test_objstr.py
Outdated
|
|
||
| t1 = b"" | ||
| s = objstr(t1, print_methods=True, honor_existing=False) | ||
| is_python_le_3_13 = sys.version_info <= (3, 13) |
There was a problem hiding this comment.
You don't need a separate variable here. The expression is very clear. Also don't need to test 3.13+, just confirm we get the special case for 3.13- would be fine.
There was a problem hiding this comment.
OK, I will delete this variable. You mentioned in the #118 that the new version will fix this problem. I understand that <signature unknown> should not appear in the new version, so I added the version judgment.
There was a problem hiding this comment.
I didn't find out which specific version it was fixed in, so I gave up trying to determine the version. :(
There was a problem hiding this comment.
Just test that on the version that it did not work before, we can print this warning, instead of fail.
tests/test_objstr.py
Outdated
| def custom_capitalize(self): | ||
| print("custom capitalize") | ||
|
|
||
| t2 = ObjTest({"hex": custom_hex, "capitalize": custom_capitalize}) |
There was a problem hiding this comment.
I don't quite understand what we are testing here. This seems like a normal test case that should be covered by print_methods tests. I think simply keeping the test above would work. A regression test is to confirm something that does not work before works after the fix. We don't test something that does not change behavior with the change.
There was a problem hiding this comment.
Ok, I will delete this case. I have some misunderstanding here.
|
|
||
| t1 = b"" | ||
| s = objstr(t1, print_methods=True, honor_existing=False) | ||
| self.assertIn("<signature unknown>", s) |
There was a problem hiding this comment.
Is this passing in the latest version? Let's only test this for certain versions. You can add a skipIf or skipUnless. Did you say 3.13 has a different behavior?
There was a problem hiding this comment.
OK, everythins is ok! It has been tested successfully in the latest version.
https://github.com/Lonely-Dream/objprint/actions/runs/13811688833
The previous version did not pass the test.
https://github.com/Lonely-Dream/objprint/actions/runs/13811661055
There was a problem hiding this comment.
Interesting, so for all exsiting versions, the signature of the methods are not available.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #121 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 5 5
Lines 374 378 +4
=========================================
+ Hits 374 378 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
针对 #118 中提出的问题尝试做了改善，避免因inspect.signature问题，导致自己库抛出异常。
同步增加了相应的测试用例。