Skip to content

bpo-33465: Use an unlikely to be built-in C extension in a test#6797

Merged
warsaw merged 5 commits intopython:masterfrom
warsaw:issue33465
May 15, 2018
Merged

bpo-33465: Use an unlikely to be built-in C extension in a test#6797
warsaw merged 5 commits intopython:masterfrom
warsaw:issue33465

Conversation

@warsaw
Copy link
Copy Markdown
Member

@warsaw warsaw commented May 14, 2018 •
edited by bedevere-bot
Loading

zware
zware reviewed May 14, 2018
View reviewed changes
Comment thread Lib/test/test_import/__init__.py Outdated

def test_from_import_missing_attr_has_name_and_so_path(self):
import select
import _hashlib
Copy link
Copy Markdown
Member

@zware zware May 14, 2018

Choose a reason for hiding this comment

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

It's pretty easy for _hashlib to just not exist, for instance if OpenSSL is not available. How about _testcapi?

Copy link
Copy Markdown
Member Author

@warsaw warsaw May 15, 2018

Choose a reason for hiding this comment

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

Okay, after talking with @doko42 we think _testcapi is a good choice. We need to find one that will always be built and is not a built-in (on Debian).

Copy link
Copy Markdown
Member

@encukou encukou May 15, 2018

Choose a reason for hiding this comment

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

Same in Fedora; _testcapi sounds like a good choice.
(It's always built, as a shared library. Then it's packaged in python3-tests so it's not necessarily installed on all systems.)

zware
zware approved these changes May 15, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@zware zware left a comment

Choose a reason for hiding this comment

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

I wonder if this test should be guarded by @cpython_only? Otherwise, LGTM.

@warsaw warsaw changed the title bpo-33465: Use binascii, as it's less likely to be built-in bpo-33465: Use an unlikely to be built-in C extension in a test May 15, 2018
@warsaw
Copy link
Copy Markdown
Member Author

warsaw commented May 15, 2018

@zware Good point. Done. And thanks for the approval; I'll merge this once CI passes.

@warsaw warsaw merged commit 8709b23 into python:master May 15, 2018
@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented May 15, 2018

@warsaw: Please replace # with GH- in the commit message next time. Thanks!

@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented May 15, 2018

Thanks @warsaw for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@warsaw warsaw deleted the issue33465 branch May 15, 2018 18:41
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 15, 2018
@warsaw @miss-islington
bpo-33465: Use an unlikely to be built-in C extension in a test (pyth…
fd5a14c 
…onGH-6797)

(cherry picked from commit 8709b23)

Co-authored-by: Barry Warsaw <barry@python.org>
@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented May 15, 2018

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

warsaw added a commit that referenced this pull request May 15, 2018
@miss-islington @warsaw
bpo-33465: Use an unlikely to be built-in C extension in a test (GH-6797
2cdb70a 
) (#6869)

(cherry picked from commit 8709b23)

Co-authored-by: Barry Warsaw <barry@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@encukou encukou encukou left review comments

@zware zware zware approved these changes

Assignees

@doko42 doko42

Labels

skip news tests Tests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@warsaw @bedevere-bot @miss-islington @encukou @zware @doko42 @the-knights-who-say-ni