Skip to content

bpo-46623: Skip two test_zlib tests on s390x - #31096

Merged
vstinner merged 3 commits into
python:mainfrom
vstinner:test_zlib
Feb 24, 2022
Merged

bpo-46623: Skip two test_zlib tests on s390x#31096
vstinner merged 3 commits into
python:mainfrom
vstinner:test_zlib

Conversation

@vstinner

@vstinnervstinner commented Feb 3, 2022

Copy link
Copy Markdown
Member

Skip test_pair() and test_speech128() of test_zlib on s390x since
they fail if zlib uses the s390x hardware accelerator.

https://bugs.python.org/issue46623

Skip test_pair() and test_speech128() of test_zlib on s390x since
they fail if zlib uses the s390x hardware accelerator.
@encukou

Copy link
Copy Markdown
Member

The tests check round-trip as well as compression, though.
Shouldn't only the compression assertions be skipped?

Comment threadLib/test/test_zlib.py
#
# Make the assumption that s390x always has an accelerator to simplify the
# skip condition. Don't skip on Windows which doesn't have os.uname().
skip_on_s390x = unittest.skipIf(hasattr(os, 'uname') and os.uname().machine == 's390x',

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To implementing Petr's comment suggestion:

Suggested change
skip_on_s390x=unittest.skipIf(hasattr(os, 'uname') andos.uname().machine=='s390x',
defnot_s390x():
returnnot (hasattr(os, 'uname') andos.uname().machine=='s390x')

and replace @skip...s below with if not_s390x: on asserts.

@vstinner

Copy link
Copy Markdown
MemberAuthor

I got access to a s390x machine (RHEL8), sadly it didn't have the zlib hardware optimization and so test_zlib just passed. I agree that it's not ideal to always skip 2 tests on s390x. But the test is run on all other architectures and all other test_zlib tests are still run. An alternative would be to remove the tests which make too many assumptions about compressed data.

For now, I prefer to just skip the tests. If someone knows how to detect the hardware accelerator or wants to enhance the tests, please go ahead and write a PR ;-)

@vstinner
vstinner merged commit 9475dc0 into python:mainFeb 24, 2022
@vstinner
vstinner deleted the test_zlib branch February 24, 2022 23:32
@vstinner

Copy link
Copy Markdown
MemberAuthor

Even if Python 3.9 and 3.10 are also affected, I prefer to not backport the change since it's not ideal.

asvetlov pushed a commit that referenced this pull request Feb 26, 2022
Skip test_pair() and test_speech128() of test_zlib on s390x since
they fail if zlib uses the s390x hardware accelerator.
encukou added a commit to encukou/cpython that referenced this pull request Oct 16, 2024
…, skip checking the compressed bytes (pythonGH-125042)
This backports two commits:
- pythonGH-31096 skipped the tests unconditionally
- pythonGH-125042 skips only the possibly-failing assertion
(cherry picked from commit cc5a225)
encukou added a commit to encukou/cpython that referenced this pull request Oct 16, 2024
…ration, skip checking the compressed bytes (pythonGH-125042)
This backports two commits:
- pythonGH-31096 skipped the tests unconditionally
- pythonGH-125042 skips only the possibly-failing assertion
(cherry picked from commit d522856)
Co-authored-by: Petr Viktorin <encukou@gmail.com>
pablogsal pushed a commit that referenced this pull request Oct 22, 2024
…p checking the compressed bytes (GH-125042) (#125585)
gh-125041: gh-90781: test_zlib: For s390x HW acceleration, skip checking the compressed bytes (GH-125042)
This backports two commits:
- GH-31096 skipped the tests unconditionally
- GH-125042 skips only the possibly-failing assertion
(cherry picked from commit cc5a225)
ambv pushed a commit that referenced this pull request Oct 28, 2024
… checking the compressed bytes (GH-125042) (#125587)
This backports two commits:
- GH-31096 skipped the tests unconditionally
- GH-125042 skips only the possibly-failing assertion
(cherry picked from commit d522856)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testsTests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@vstinner@encukou@terryjreedy@the-knights-who-say-ni@bedevere-bot