Skip to content

bpo-19696: Move threaded import tests to dir test_importlib/ #14537

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
aeros wants to merge 7 commits into from
Closed

bpo-19696: Move threaded import tests to dir test_importlib/ #14537

aeros wants to merge 7 commits into from

Conversation

@aeros
Copy link
Contributor

@aeros aeros commented Jul 1, 2019 •
edited by bedevere-bot
Loading

This is the final component of issue 19696. This PR includes both test_threaded_import.py and threaded_import_hangers.py because the hangers are required for the threaded import test, so the two files are directly related. I'm not sure as to why the changes can't be automatically merged, the previous file move I had done had no issues with that. Is it due to more than one file being moved in a single PR?

There's a minor typo in one of the commit messages ("theaded" instead of "threaded"), but there were no issues with the file moves. Is it possible to edit the commit messages? If this typo is an issue and there's no way of editing the commit messages, I can create a new branch, redo the changes, and submit another PR.

Since this is just a file move, a "skip news" label would likely be appropriate.

https://bugs.python.org/issue19696

@aeros
Copy link
Contributor Author

aeros commented Jul 2, 2019 •
edited
Loading

I believe I found the cause of the issue, on line 184, import test.threaded_import_hangers probably needs to be changed to import test.test_importlib.threaded_import_hangers to reflect the new location of the file.

Edit: looks like I have to change the other references as well. Hopefully that fixes the issue.

aeros added 2 commits July 1, 2019 20:21
@aeros
Updated threaded_import_hangers import
bba2151 
The import statement needed to be updated to reflect the location change of ``threaded_import_hangers.py``
@aeros
Update other references to "threaded_import_hangers"
eafbd37 
Updated the other references to ``threaded_import_hangers.py`` to reflect the change in location
@aeros
Copy link
Contributor Author

aeros commented Jul 2, 2019 •
edited
Loading

After performing a code search of all references to threaded_import_hangers, it appears there is another reference to it in PCbuild/lib.pyproj. I'll update that reference as well.

Edit: also updated the references to test_pkg_import.py in lib.pyproj, that file was moved and renamed from test_pkgimport.py in a previous PR (component of the same issue).

@aeros
Update reference to threaded_import_hangers.py
37f459e 
Since the location of threaded_import_hangers.py was modified as part of issue 19696, the reference to it needs to be updated.
@aeros aeros requested a review from a team as a code owner July 2, 2019 00:37
aeros added 2 commits July 1, 2019 20:40
@aeros
Update reference to test_threaded_import.py
6ace114 
Since the location of test_threaded_import.py was modified in issue 19696, the reference to it needs to be updated.
@aeros
Updated reference to test_pkgimport.py
cf5e677 
Since the location of test_pkgimport.py was changed and the file was renamed to test_import_pkg.py, the reference to it should be updated. This did not cause any issues in merge which moved the file, but it should updated anyways.
@aeros
Copy link
Contributor Author

aeros commented Jul 2, 2019 •
edited
Loading

@brettcannon Do I need to resubmit the PR for the tests to be ran again, or is there something else that I'm missing? If I need to resubmit the PR, I'll instead break it up into two. test_threaded_import.py was able to be moved without issue, it seems that moving threaded_import_hangers.py has been the primary cause of the conflicts. At this point, I have no idea as to what could be the exact cause of these issues. All of the references have been updated, and as far as I can tell, nothing else should have changed from the location of the file being moved.

@aeros
Copy link
Contributor Author

aeros commented Jul 3, 2019

Closing this PR, I'll create a new one which exclusively moves test_threaded_import.py and attempt to move test_import_hangers.py in a separate one later tonight or tomorrow. Hopefully that will resolve the merge conflicts.

@aeros aeros closed this Jul 3, 2019
@aeros aeros deleted the move-threaded_import branch July 3, 2019 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brettcannon brettcannon Awaiting requested review from brettcannon

@encukou encukou Awaiting requested review from encukou

@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently

@ncoghlan ncoghlan Awaiting requested review from ncoghlan

@warsaw warsaw Awaiting requested review from warsaw

Assignees

No one assigned

Labels

awaiting review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@aeros @the-knights-who-say-ni @bedevere-bot