GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-46623: Skip two test_zlib tests on s390x by vstinner · Pull Request #31096 · python/cpython

Conversation

@vstinner

@vstinner vstinner commented Feb 3, 2022 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Feb 3, 2022
@bedevere-bot bedevere-bot added awaiting core review tests Tests in the Lib/test dir labels Feb 3, 2022
@vstinner
bpo-46623: Skip two test_zlib tests on s390x
f40acc9 
Skip test_pair() and test_speech128() of test_zlib on s390x since
they fail if zlib uses the s390x hardware accelerator.
@encukou

encukou commented Feb 3, 2022

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner
Windows doesn't have os.uname()
370ab52
terryjreedy
terryjreedy reviewed Feb 4, 2022
View reviewed changes
Comment thread Lib/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',

@terryjreedy terryjreedy Feb 4, 2022

Copy link
Copy Markdown
Member

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.

To implementing Petr's comment suggestion:

Suggested change
skip_on_s390x = unittest.skipIf(hasattr(os, 'uname') and os.uname().machine == 's390x',
def not_s390x():
return not (hasattr(os, 'uname') and os.uname().machine == 's390x')

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@vstinner
Fix the comment
2ba7379
@vstinner

vstinner commented Feb 24, 2022

Copy link
Copy Markdown
Member Author

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 ;-)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner
vstinner merged commit 9475dc0 into python:main Feb 24, 2022
@bedevere-bot bedevere-bot removed the awaiting core review label Feb 24, 2022
@vstinner
vstinner deleted the test_zlib branch February 24, 2022 23:32
@vstinner

vstinner commented Feb 24, 2022

Copy link
Copy Markdown
Member Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
asvetlov pushed a commit that referenced this pull request Feb 26, 2022
@vstinner @asvetlov
bpo-46623: Skip two test_zlib tests on s390x (GH-31096)
6c844f1 
Skip test_pair() and test_speech128() of test_zlib on s390x since
they fail if zlib uses the s390x hardware accelerator.
@vstinner vstinner mentioned this pull request Apr 10, 2022
test_zlib: test_pair() and test_speech128() fail with s390x hardware accelerator #90781
Closed
@encukou encukou mentioned this pull request Oct 7, 2024
Re-enable skipped test_zlib tests on s390x hardware acceleration #125041
Closed
encukou added a commit to encukou/cpython that referenced this pull request Oct 16, 2024
@encukou
pythongh-125041: pythongh-90781: test_zlib: For s390x HW acceleration…
d522856 
…, 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 encukou mentioned this pull request Oct 16, 2024
[3.10] gh-125041: gh-90781: test_zlib: For s390x HW acceleration, skip checking the compressed bytes (GH-125042) #125585
Merged
encukou added a commit to encukou/cpython that referenced this pull request Oct 16, 2024
@encukou
[3.9] pythongh-125041: pythongh-90781: test_zlib: For s390x HW accele…
21bf2ba 
…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 <[email protected]>
@encukou encukou mentioned this pull request Oct 16, 2024
[3.9] gh-125041: gh-90781: test_zlib: For s390x HW acceleration, skip checking the compressed bytes (GH-125042) #125587
Merged
pablogsal pushed a commit that referenced this pull request Oct 22, 2024
@encukou
[3.10] gh-125041: gh-90781: test_zlib: For s390x HW acceleration, ski…
3c63d80 
…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
@encukou
[3.9] gh-125041: gh-90781: test_zlib: For s390x HW acceleration, skip…
bbe8756 
… 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 free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@terryjreedy terryjreedy terryjreedy left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

tests Tests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@vstinner @encukou @terryjreedy @the-knights-who-say-ni @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.