bpo-32138: Skip on Android test_faulthandler tests that raise SIGSEGV#4604
Conversation
Remove the test.support.requires_android_level decorator.
| except AttributeError: | ||
| # sys.getandroidapilevel() is only available on Android | ||
| is_android = False | ||
| is_android = True if hasattr(sys, 'getandroidapilevel') else False |
There was a problem hiding this comment.
Why not just "hasattr(sys, 'getandroidapilevel')"??
| else: | ||
| return unittest.skip("resource {0!r} is not enabled".format(resource)) | ||
|
|
||
| def requires_android_level(level, reason): |
There was a problem hiding this comment.
I suggest to keep it. I'm in touch with a developer who wants to support older Android versions, and in the future, we may have to use it again for features only working on newer Android versions.
There was a problem hiding this comment.
I prefer not to clutter test.support with dead code and it is easy to restore requires_android_level with git show <sha1> | git apply -R.
| 'access violation') | ||
|
|
||
| @requires_raise | ||
| @unittest.skipIf(is_android, 'raising SIGSEGV on Android is unreliable') |
There was a problem hiding this comment.
Please write a function like the current requires_raise() for Android, since you copied/pasted this line many times. Something like:
def skip_on_android():
return unittest.skipIf(is_android, 'raising SIGSEGV on Android is unreliable')
Maybe "skip_segfault_on_android"?
Add a comment on this function mentionning "bpo-32138" and explain that raise(SIGSEGV) doesn't crash on Android (but does nothing if I understood correctly).
Then just use @skip_on_android.
|
When you're done making the requested changes, leave the comment: |
|
I have made the requested changes; please review again. |
|
Thanks for making the requested changes! @vstinner: please review the changes made to this pull request. |
| return '^' + regex + '$' | ||
|
|
||
| def skip_segfault_on_android(test): | ||
| # Issue #32138: Raising SIGSEGV on Android may not cause a crash. |
|
Thanks for the update, the PR is now much better! |
4 participants
Remove the test.support.requires_android_level decorator.
https://bugs.python.org/issue32138