[issue19696] Merge all (non-syntactic) import-related tests into test_importlib

[Brett Cannon]

Change by Brett Cannon :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19696] Merge all (non-syntactic) import-related tests into test_importlib

[Kyle Stanley]

Kyle Stanley  added the comment:

The latest PR-14655 moved the last file, "threaded_import_hangers.py" into the 
"test_importlib" directory. As far as I can tell, there's nothing else which 
needs to be moved there, so the issue can be closed.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19696] Merge all (non-syntactic) import-related tests into test_importlib

[miss-islington]

miss-islington  added the comment:


New changeset a65c977552507dd19d2cc073fc91ae22cc66bbff by Miss Islington (bot) 
(Kyle Stanley) in branch 'master':
bpo-19696: Move threaded_import_hangers (GH-14655)
https://github.com/python/cpython/commit/a65c977552507dd19d2cc073fc91ae22cc66bbff


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19696] Merge all (non-syntactic) import-related tests into test_importlib

[Kyle Stanley]

Kyle Stanley  added the comment:

Typo: The PR in the above comment should be 14655, not 14642.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19696] Merge all (non-syntactic) import-related tests into test_importlib

[Kyle Stanley]

Kyle Stanley  added the comment:

Opened a new PR for moving "threaded_import_hangers" to "test_importlib/" 
(PR-14642). This should be the final file in "tests/" to move into 
"test_importlib/", so the issue can be closed if this PR is merged. I'll create 
a new issue for modernizing the tests once this issue is closed.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19696] Merge all (non-syntactic) import-related tests into test_importlib

[Kyle Stanley]

Change by Kyle Stanley :


--
pull_requests: +14462
pull_request: https://github.com/python/cpython/pull/14655

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19696] Merge all (non-syntactic) import-related tests into test_importlib

[Kyle Stanley]

Change by Kyle Stanley :


--
pull_requests: +14454
pull_request: https://github.com/python/cpython/pull/14642

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19696] Merge all (non-syntactic) import-related tests into test_importlib

[Kyle Stanley]

Kyle Stanley  added the comment:

Typo in previous comment: "test_threaded_imports.py" should be 
"test_threaded_import.py".

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19696] Merge all (non-syntactic) import-related tests into test_importlib

[Kyle Stanley]

Kyle Stanley  added the comment:

In order to avoid the merge conflicts, I'm going to move 
test_threaded_imports.py and threaded_import_hangers.py in separate PRs. Here's 
the PR for moving test_threaded_imports.py 
https://github.com/python/cpython/pull/14582.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19696] Merge all (non-syntactic) import-related tests into test_importlib

[Kyle Stanley]

Change by Kyle Stanley :


--
pull_requests: +14401
pull_request: https://github.com/python/cpython/pull/14582

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19696] Merge all (non-syntactic) import-related tests into test_importlib

[miss-islington]

miss-islington  added the comment:


New changeset 56ec4f1fdedd5b38deb06d94d51dd1a540262e90 by Miss Islington (bot) 
(Kyle Stanley) in branch 'master':
bpo-19696: Replace deprecated method in "test_import_pkg.py" (GH-14466)
https://github.com/python/cpython/commit/56ec4f1fdedd5b38deb06d94d51dd1a540262e90


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19696] Merge all (non-syntactic) import-related tests into test_importlib

[Kyle Stanley]

Kyle Stanley  added the comment:

Created a new PR for moving the last two files "test_threaded_import.py" and 
"threaded_import_hangers.py" to the directory "test_importlib". 
(https://github.com/python/cpython/pull/14537)

There were some issues with automatically merging the changes, was the conflict 
because I attempted to move two files in a single PR? The previous one (which 
was just moving a single file) did not have any issues with merge conflicts.

This should be the final PR required for this issue, afterwards I'll look into 
creating a new issue for modernizing the tests.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19696] Merge all (non-syntactic) import-related tests into test_importlib

[Kyle Stanley]

Change by Kyle Stanley :


--
pull_requests: +14350
pull_request: https://github.com/python/cpython/pull/14537

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19696] Merge all (non-syntactic) import-related tests into test_importlib

[aeros167]

aeros167  added the comment:

Created a new PR replacing the deprecated method "random.choose" with 
"random.choice" in "test_pkg_import.py" 
(https://github.com/python/cpython/pull/14466).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19696] Merge all (non-syntactic) import-related tests into test_importlib

[aeros167]

Change by aeros167 :


--
pull_requests: +14283
pull_request: https://github.com/python/cpython/pull/14466

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19696] Merge all (non-syntactic) import-related tests into test_importlib

[aeros167]

aeros167  added the comment:

> Thanks for the PR, aeros167! BTW, if you want to open a new issue and 
> modernize the tests to use importlib directly that would be great!

Sounds good, I'll definitely look into doing that after finishing up this 
issue. Was waiting the previous PR to be merged before replacing the deprecated 
method "random.choose" with "random.choice" in "test_pkg_import.py" and then 
also moving the other two into test_importlib. 

Thank you very much for the timely feedback. This has been a great experience 
for learning to contribute to larger open source projects. It's been an 
aspiration of mine to give back to Python, as it was my first serious 
programming language 5 years ago. This may be an incredibly minor contribution 
in the grand scheme of things, but my goal is for it to be the first of many (:

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19696] Merge all (non-syntactic) import-related tests into test_importlib

[Brett Cannon]

Brett Cannon  added the comment:

Thanks for the PR, aeros167! BTW, if you want to open a new issue and modernize 
the tests to use importlib directly that would be great!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19696] Merge all (non-syntactic) import-related tests into test_importlib

[miss-islington]

miss-islington  added the comment:


New changeset 80097e089ba22a42d804e65fbbcf35e5e49eed00 by Miss Islington (bot) 
(Kyle Stanley) in branch 'master':
bpo-19696: Moved "test_pkgimport.py" to dir "test_importlib" (GH-14303)
https://github.com/python/cpython/commit/80097e089ba22a42d804e65fbbcf35e5e49eed00


--
nosy: +miss-islington

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19696] Merge all (non-syntactic) import-related tests into test_importlib

[aeros167]

aeros167  added the comment:

Created a PR for moving and renaming "test_pkimport". An initial approval was 
made, now it is awaiting core review 
(https://github.com/python/cpython/pull/14303).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19696] Merge all (non-syntactic) import-related tests into test_importlib

[aeros167]

Change by aeros167 :


--
keywords: +patch
pull_requests: +14127
stage: needs patch -> patch review
pull_request: https://github.com/python/cpython/pull/14303

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19696] Merge all (non-syntactic) import-related tests into test_importlib

[aeros167]

aeros167  added the comment:

> Yep, just make sure the tests still pass before and after the change. :)

Sounds good, the first thing I had done before proposing the change was testing 
it in an IDE and using some logging to ensure that random.choose and 
random.choice were providing the same functionality. So hopefully the PR tests 
will pass as well. 

> Separate are easier to review.

Alright, I'll do each of the file moves in individual PRs and then a separate 
one for changing random.choice to random.choose. 

Thanks!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19696] Merge all (non-syntactic) import-related tests into test_importlib

[Brett Cannon]

Brett Cannon  added the comment:

> Would it be appropriate to change the "choose" method to "choice"? 

Yep, just make sure the tests still pass before and after the change. :)

> should the name of "test_pkgimport" instead be "test_pkg_import"?

Since things are moving it's fine to rename the file.

> would it be best to perform all of these changes within separate pull 
> requests, or are they minor enough to be combined into one?

Separate are easier to review.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19696] Merge all (non-syntactic) import-related tests into test_importlib

[aeros167]

aeros167  added the comment:

Decided to start working on this issue, seems like a solid starting point. 
After submitting a minor doc contribution, I wanted to also attempt to 
contribute to some of the easier enhancement requests. 

>From my current assessment, it appears that "test_pkgimport", 
>"test_threaded_import", and "threaded_import_hangers" all would appropriately 
>be categorized in test_importlib. Pkgimport attempts to build the package, 
>perform basic filesystem tests, and ensure that run time errors are correctly 
>functioning. "threaded_import" attempts to simultaneously important multiple 
>modules by assigning each one to different threads. The purpose of 
>"threaded_import_hangers" is specifically dependent on "threaded_import", so 
>those two should be grouped together.

While going through test_pkgimport I noticed that it uses the method 
"random.choose" on lines 17 and 63. However, the standard seems to 
"random.choice". I could not find any specific mention of random.choose on the 
python docs, so this appears to be a deprecated method. As far as I'm aware 
"random.choice" follows current standard and is used far more frequently within 
the CPython repository. Would it be appropriate to change the "choose" method 
to "choice"? 

Additionally, as far as naming convention goes, should the name of 
"test_pkgimport" instead be "test_pkg_import"? This is entirely semantics, but 
it makes for better consistency with the other file names and is a bit more 
readable. Also, would it be best to perform all of these changes within 
separate pull requests, or are they minor enough to be combined into one?

--
nosy: +aeros167

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19696] Merge all (non-syntactic) import-related tests into test_importlib

[Brett Cannon]

Brett Cannon  added the comment:

Anything zipimport-related should stay separate as zipimport is not a part of 
importlib. The other three will have to be looked at to see what exactly they 
are testing to know whether they relate to importlib and the implementation of 
import or not (my guess is yes).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19696] Merge all (non-syntactic) import-related tests into test_importlib

[Gil Forcada Codinachs]

Gil Forcada Codinachs  added the comment:

At least test_namespace_pkgs is already moved, see 
https://bugs.python.org/issue21097

Should then this issue be closed?

I see that there are a few other tests that have import on its name and are 
outside either test_import or test_importlib:

- test_pkgimport.py
- test_threaded_import.py
- test_zipimport.py
- test_zipimport_support.py
- threaded_import_hangers.py

Should those be actually moved to test_importlib?

--
nosy: +gforcada

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19696] Merge all (non-syntactic) import-related tests into test_importlib

[Arfrever Frehtes Taifersar Arahesis]

Changes by Arfrever Frehtes Taifersar Arahesis arfrever@gmail.com:


--
nosy: +Arfrever

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19696
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19696] Merge all (non-syntactic) import-related tests into test_importlib

[Brett Cannon]

New submission from Brett Cannon:

E.g. test_namespace_pkgs should be under test_importlib and so should 
test_namespace_pkgs. test_import can conceivably stay out if it's updated to 
only contain syntactic tests for the import statement.

This is so that it's easier to run import-related tests when making changes to 
importlib (otherwise one has to run the whole test suite or memorize the name 
of every test suite related to import/importlib).

--
components: Tests
keywords: easy
messages: 203792
nosy: brett.cannon
priority: low
severity: normal
stage: needs patch
status: open
title: Merge all (non-syntactic) import-related tests into test_importlib
type: enhancement
versions: Python 3.5

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19696
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19696] Merge all (non-syntactic) import-related tests into test_importlib

[Eric Snow]

Eric Snow added the comment:

+1

--
nosy: +eric.snow

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19696
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com