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-10572: Move test sub-packages to Lib/test by idomic · Pull Request #18524 · python/cpython

Conversation

@idomic

@idomic idomic commented Feb 16, 2020 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

https://bugs.python.org/issue10572

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
idomic and others added 5 commits February 13, 2020 17:56
@idomic
Hard reset + cherry piciking the changes.
13c5b05
@blurb-it @idomic
📜🤖 Added by blurb_it.
fdecdf0
@idomic
Added @vstinner News
29d13c9
@idomic @vstinner
Update Misc/NEWS.d/next/Library/2020-02-11-13-01-38.bpo-38691.oND8Sk.rst
135259f 
Co-Authored-By: Victor Stinner <[email protected]>
@idomic
Merge branch 'master' of https://github.com/python/cpython
794b948
@idomic
idomic requested review from berkerpeksag and rhettinger as code owners February 16, 2020 18:07
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Feb 16, 2020
@bedevere-bot bedevere-bot added the awaiting review label Feb 16, 2020
idomic added 2 commits February 16, 2020 13:13
@idomic
Merge branch 'master' of https://github.com/python/cpython
bfa3cd1
@idomic
[bpo-10572] Moved tests of these modules under Lib/tests/MODULE_NAME_…
36b41eb 
…test:

  tkinter/test
  distutils/tests
  ctypes/tests
  lib2to3/tests
  sqlite3/test
  unittest/test
@idomic
idomic force-pushed the bpo-10572-reArranging_Lib_tests branch from 746c83b to 36b41eb Compare February 16, 2020 18:20
@idomic
[bpo-10572] Changing test references for CI
69ad225
ned-deily
ned-deily requested changes Feb 16, 2020
View reviewed changes

@ned-deily ned-deily left a comment •
edited
Loading

Uh oh!

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

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.

Please make sure that tests still run correctly when running from an installed instance of Python, not just from a development build directory. The various test directories are installed in the main Makefile (Makefile.pre.in) libinstall rule. I think you'll find that many tests are now broken. See the LIBSUBDIRS variable in the Makefile. (I see that the dev guide does not refer to this topic so I've opened a devguide issue about it: python/devguide#573)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot

bedevere-bot commented Feb 16, 2020

Copy link
Copy Markdown

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added awaiting changes and removed awaiting review labels Feb 16, 2020
idomic added 5 commits February 16, 2020 15:52
@idomic
[bpo-10572] Adapting tests imports
8e7db7a
@idomic
Merge branch 'master' of https://github.com/python/cpython
71b4de5
@idomic
Removed residues
4be42f0
@idomic
Merge branch 'master' into bpo-10572-reArranging_Lib_tests
4a3cf9f
@idomic
[bpo-10572] Changed Makefile.pre.in to contain the correct tests path
18c5f36
@idomic

idomic commented Feb 16, 2020

Copy link
Copy Markdown
Contributor Author

Thanks @ned-deily , I've updated the makefile and adding a News file.
I have made the requested changes; please review again
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added awaiting change review and removed awaiting changes labels Feb 16, 2020
@bedevere-bot

bedevere-bot commented Feb 16, 2020

Copy link
Copy Markdown

Thanks for making the requested changes!

@ned-deily: please review the changes made to this pull request.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot
bedevere-bot requested a review from ned-deily February 16, 2020 21:48
blurb-it Bot and others added 3 commits February 16, 2020 21:51
@blurb-it
📜🤖 Added by blurb_it.
60a14df
@idomic
[bpo-10572] Changed Makefile.pre.in to contain the correct tests path
fbd63a8
@idomic
Merge remote-tracking branch 'origin/bpo-10572-reArranging_Lib_tests'…
e3ff1cd 
… into bpo-10572-reArranging_Lib_tests
idomic added 2 commits February 16, 2020 17:48
@idomic
[bpo-10572] Changed Makefile.pre.in to contain the correct tests path
bb4e8b8
@idomic
[bpo-10572] Fixing distutils tests path
65db1d2
@idomic
idomic requested a review from a team as a code owner February 16, 2020 23:10
idomic added 10 commits February 16, 2020 18:14
@idomic
[bpo-10572] Fixing Lib.test.tkinter_test tests path
7cfa472
@idomic
[bpo-10572] Fixing Lib.test.unittest_test + ctypes tests path
fd7ed2b
@idomic
[bpo-10572] Fixing Lib.test.unittest_test + test_lib2to3 tests path
6b32dd1
@idomic
[bpo-10572] Fixing Lib.test.unittest_test + test_lib2to3 tests path
1021fcf
@idomic
[bpo-10572] Fixed hardcoded Lib.test.unittest_test and lib2to3 path
f5431d8
@idomic
[bpo-10572] Fixed hardcoded Lib.test.unittest_test and lib2to3 path
d89915f
@idomic
[bpo-10572] Fixed hardcoded Lib.test.unittest_test and lib2to3 path
a5e9500
@idomic
[bpo-10572] Fixed hardcoded Lib.test.unittest_test and lib2to3 path
19ab4cd
@idomic
[bpo-10572] Fixed hardcoded Lib.test.unittest_test and lib2to3 path
1010154
@idomic
[bpo-10572] Fixed hardcoded Lib.test.unittest_test and lib2to3 path
e65b43c
@idomic

idomic commented Feb 17, 2020

Copy link
Copy Markdown
Contributor Author

Ok I fixed all of the tests expect 1 in lib2to3 in which I'm not really sure what's going on.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
idomic added 2 commits February 16, 2020 22:54
@idomic
[bpo-10572] Fixed hardcoded Lib.test.unittest_test and lib2to3 path
4609963
@idomic
[bpo-10572] Fixed hardcoded Lib.test.unittest_test and lib2to3 path
c552264
@idomic

idomic commented Feb 23, 2020

Copy link
Copy Markdown
Contributor Author

When looking on the test that's failing:

test_write_filtered_python_package (test.test_zipfile.PyZipFileTests) ... File "/Users/runner/runners/2.164.0/work/cpython/cpython/Lib/test/lib2to3_tests/data/bom.py", line 2
print "BOM BOOM!"

It looks like it uses print syntax of python 2.7 which is not relevant anymore, any idea why is it there? I suspect in the master it was just not running and because now it's in the tests folder it's running by default.

I have made the requested changes; please review again
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot

bedevere-bot commented Feb 23, 2020

Copy link
Copy Markdown

Thanks for making the requested changes!

@ned-deily: please review the changes made to this pull request.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@brettcannon
brettcannon self-requested a review February 27, 2020 23:08
@brettcannon

brettcannon commented Mar 5, 2020

Copy link
Copy Markdown
Member

@idomic it looks like it's a test file for the lib2to3 test to use. There's probably a test that reads that file, runs lib2to3 on it, and checks the results. So that file is very likely there on purpose and meant to be Python 2 syntax.
All reactions

Sorry, something went wrong.

Uh oh!

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

brettcannon commented Mar 5, 2020

Copy link
Copy Markdown
Member

@idomic you also have conflicts. It might be easier to do one PR per package so lessen the chances of merge conflicts occurring.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
brettcannon
brettcannon requested changes Mar 13, 2020
View reviewed changes

@brettcannon brettcannon left a comment

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.

The merge conflicts need to be fixed, and I would strongly suggest breaking this PR up into single PRs per package.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot removed the awaiting change review label Mar 13, 2020
@bedevere-bot

bedevere-bot commented Mar 13, 2020

Copy link
Copy Markdown

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added the awaiting changes label Mar 13, 2020
@idomic

idomic commented Mar 15, 2020

Copy link
Copy Markdown
Contributor Author

The merge conflicts need to be fixed, and I would strongly suggest breaking this PR up into single PRs per package.

I'll submit a new PR with 1 package and will tag you and Ned
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@idomic idomic closed this Mar 22, 2020
@voidspace voidspace mentioned this pull request Jun 20, 2022
Move test sub-packages to Lib/test #54781
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brettcannon brettcannon brettcannon requested changes
@berkerpeksag berkerpeksag Awaiting requested review from berkerpeksag
@rhettinger rhettinger Awaiting requested review from rhettinger
@ned-deily ned-deily Awaiting requested review from ned-deily

Uh oh!

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

Assignees

No one assigned

Labels

awaiting changes

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

@idomic @bedevere-bot @brettcannon @ned-deily @the-knights-who-say-ni
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.