bpo-32682: test_zlib improve version parsing by pmp-p · Pull Request #5347 · python/cpython

pmp-p · 2018-01-26T16:02:48Z

fix odd version number parsing of zlib found at least on some android versions.

https://bugs.python.org/issue32682

the-knights-who-say-ni · 2018-01-26T16:02:50Z

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

Thanks again to your contribution and we look forward to looking at it!

vstinner · 2018-01-26T16:24:07Z

Lib/test/test_zlib.py

        # wbits=0 only supported since zlib v1.2.3.5
        # Register "1.2.3" as "1.2.3.0"
-        v = (zlib.ZLIB_RUNTIME_VERSION + ".0").split(".", 4)
+        if zlib.ZLIB_RUNTIME_VERSION.find('-')>=0:


"-" in zlib.ZLIB_RUNTIME_VERSION would be simpler

vstinner · 2018-01-26T16:26:14Z

Lib/test/test_zlib.py

+            v = list(map( int , zlib.ZLIB_RUNTIME_VERSION.split('-',1)[0].split('.')[:3] ))
+            v.append(0)
+        else:
+            v = (zlib.ZLIB_RUNTIME_VERSION + ".0").split(".", 4)


You should factorize the code between the if/else block. Something like:

v = zlib.ZLIB_RUNTIME_VERSION
if "-" in v:
v = v.split('-',1)[0]
v = (v + ".0").split(".", 4)

bedevere-bot · 2018-01-26T16:26:20Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

pmp-p

maybe this way

pmp-p · 2018-01-26T18:03:04Z

or this way ? ( sorry learning interface :p )

pmp-p

maybe a line too long

pmp-p · 2018-01-26T18:50:13Z

I have made the requested changes; please review again

bedevere-bot · 2018-01-26T18:50:15Z

Thanks for making the requested changes!

@vstinner: please review the changes made to this pull request.

zware

Just a couple of minor fixes needed.

zware · 2018-02-01T14:42:59Z

Lib/test/test_zlib.py

+        supports_wbits_0 = ( v[0] > 1 ) or ( v[0] == 1 ) \
+                and ( v[1] > 2 ) or ( v[1] == 2 ) \
+                and ( v[2] > 3 ) or ( v[2] == 3 ) \
+                and ( v[3] >= 5 )


Why not supports_wbits_0 = v >= (1, 2, 3, 5)?

zware · 2018-02-01T15:34:48Z

Lib/test/test_zlib.py

+        elif not v[-1].isnumeric():
+            v[-1] = '0'
+
+        v = tuple( map( int, v ) )


No need for the extra spaces around ( or ); v = tuple(map(int, v)). (See PEP 8.)

zware · 2018-02-01T15:41:18Z

Lib/test/test_zlib.py

+                and ( v[1] > 2 ) or ( v[1] == 2 ) \
+                and ( v[2] > 3 ) or ( v[2] == 3 ) \
+                and ( v[3] >= 5 )
+        del v


No need for del v.

bedevere-bot · 2018-02-04T20:15:28Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

pmp-p · 2018-02-05T17:40:40Z

I have made the requested changes; please review again

bedevere-bot · 2018-02-05T17:40:42Z

Thanks for making the requested changes!

@vstinner, @zware: please review the changes made to this pull request.

zware

LGTM.

miss-islington · 2018-02-19T03:45:13Z

Thanks @pmp-p for the PR, and @zware for merging it 🌮🎉.. I'm working now to backport this PR to: 2.7, 3.6, 3.7.
🐍🍒⛏🤖

(cherry picked from commit 4c7108a) Co-authored-by: pmp-p <[email protected]>

bedevere-bot · 2018-02-19T03:46:22Z

GH-5751 is a backport of this pull request to the 3.7 branch.

miss-islington · 2018-02-19T03:47:15Z

Sorry, @pmp-p and @zware, I could not cleanly backport this to 2.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 4c7108a77144493d0aa6fc0105b67d3797e143f5 2.7

(cherry picked from commit 4c7108a) Co-authored-by: pmp-p <[email protected]>

bedevere-bot · 2018-02-19T03:48:19Z

GH-5752 is a backport of this pull request to the 3.6 branch.

zware · 2018-02-19T03:53:51Z

@serhiy-storchaka This doesn't appear to need to go back to 2.7; would you mind taking care of that one way or the other? I would vote for the simple solution of just removing the label :)

(cherry picked from commit 4c7108a) Co-authored-by: pmp-p <[email protected]>

issue 32682: test_zlib improve version parsing

c144513

bedevere-bot added the awaiting review label Jan 26, 2018

the-knights-who-say-ni added the CLA not signed label Jan 26, 2018

pmp-p changed the title ~~issue 32682: test_zlib improve version parsing~~ # 32682: test_zlib improve version parsing Jan 26, 2018

vstinner requested changes Jan 26, 2018

View reviewed changes

bedevere-bot removed the awaiting review label Jan 26, 2018

bedevere-bot added the awaiting changes label Jan 26, 2018

simplify as requested

e88a712

the-knights-who-say-ni added CLA signed and removed CLA not signed labels Jan 26, 2018

Update test_zlib.py

a2a9571

pmp-p changed the title ~~# 32682: test_zlib improve version parsing~~ bpo-32682: test_zlib improve version parsing Jan 26, 2018

Update test_zlib.py

c6d42d6

pmp-p commented Jan 26, 2018

View reviewed changes

Update test_zlib.py

bcb3c78

pmp-p commented Jan 26, 2018

View reviewed changes

out of space

1c7d536

bedevere-bot added awaiting change review and removed awaiting changes labels Jan 26, 2018

zware requested changes Feb 4, 2018

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting change review labels Feb 4, 2018

zware added the skip news label Feb 4, 2018

Update test_zlib.py

8255937

bedevere-bot added awaiting change review and removed awaiting changes labels Feb 5, 2018

Update test_zlib.py

2e9307d

zware approved these changes Feb 18, 2018

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Feb 18, 2018

serhiy-storchaka approved these changes Feb 18, 2018

View reviewed changes

serhiy-storchaka added needs backport to 3.6 tests Tests in the Lib/test dir labels Feb 18, 2018

zware merged commit 4c7108a into python:master Feb 19, 2018

bedevere-bot removed the awaiting merge label Feb 19, 2018

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 19, 2018

bpo-32682: Improve libz version parsing in test_zilb (pythonGH-5347)

65fefc1

(cherry picked from commit 4c7108a) Co-authored-by: pmp-p <[email protected]>

bedevere-bot removed the needs backport to 3.7 label Feb 19, 2018

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 19, 2018

bpo-32682: Improve libz version parsing in test_zilb (pythonGH-5347)

983c84b

(cherry picked from commit 4c7108a) Co-authored-by: pmp-p <[email protected]>

bedevere-bot removed the needs backport to 3.6 label Feb 19, 2018

pmp-p deleted the patch-3 branch February 19, 2018 03:56

miss-islington added a commit that referenced this pull request Feb 19, 2018

bpo-32682: Improve libz version parsing in test_zilb (GH-5347)

0cd3581

(cherry picked from commit 4c7108a) Co-authored-by: pmp-p <[email protected]>

miss-islington added a commit that referenced this pull request Feb 19, 2018

bpo-32682: Improve libz version parsing in test_zilb (GH-5347)

b0fd935

(cherry picked from commit 4c7108a) Co-authored-by: pmp-p <[email protected]>

serhiy-storchaka removed the needs backport to 2.7 label Feb 19, 2018

pmp-p mannequin mentioned this pull request Apr 10, 2022

test_zlib improve version parsing #76863

Closed

Uh oh!

Conversation

pmp-p commented Jan 26, 2018 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

the-knights-who-say-ni commented Jan 26, 2018

Uh oh!

vstinner Jan 26, 2018

Choose a reason for hiding this comment

Uh oh!

vstinner Jan 26, 2018

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Jan 26, 2018

Uh oh!

pmp-p left a comment

Choose a reason for hiding this comment

Uh oh!

pmp-p commented Jan 26, 2018

Uh oh!

pmp-p left a comment

Choose a reason for hiding this comment

Uh oh!

pmp-p commented Jan 26, 2018

Uh oh!

bedevere-bot commented Jan 26, 2018

Uh oh!

zware left a comment

Choose a reason for hiding this comment

Uh oh!

zware Feb 1, 2018

Choose a reason for hiding this comment

Uh oh!

zware Feb 1, 2018

Choose a reason for hiding this comment

Uh oh!

zware Feb 1, 2018

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Feb 4, 2018

Uh oh!

pmp-p commented Feb 5, 2018

Uh oh!

bedevere-bot commented Feb 5, 2018

Uh oh!

zware left a comment

Choose a reason for hiding this comment

Uh oh!

miss-islington commented Feb 19, 2018

Uh oh!

bedevere-bot commented Feb 19, 2018

Uh oh!

miss-islington commented Feb 19, 2018

Uh oh!

bedevere-bot commented Feb 19, 2018

Uh oh!

zware commented Feb 19, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

pmp-p commented Jan 26, 2018 •

edited by bedevere-bot

Loading