GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||
GeezerVison is showing github.com — gh-54781: Move Lib/tkinter/test/test_ttk/ to Lib/test/test_ttk/ by vstinner · Pull Request #94070 · python/cpython Conversation
|
|
I can still run tests on an newly installed (patched) Python 3.12: |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
For me, it's disturbing when tests don't have the same name than the tested module. asyncio => test_asyncio. tkinter => test_tk ???
I can keep test_ttk_guionly name if you prefer, but for me, tests should use the GUI. Only the special tests test_ttk_textonly should have a special name, no? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
This PR is special. I made the new directory tree "flat". Before: After: |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Windows (x64) job failed but I cannot get the logs. I cancelled the workflow. I still cannot see the logs. Strange. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Oh. Windows build failed with an unrelated error: Error: D:\a\cpython\cpython\Modules\socketmodule.c(7475,5): error C2065: 'HVSOCKET_CONTAINER_PASSTHRU': undeclared identifier [D:\a\cpython\cpython\PCbuild_socket.vcxproj] |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Yeah, seems we're in the middle of the rollout of new CI images, so there's a chance you'll get the older image and not see the error. But you'll need my PR for it to be reliable. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Does python -m test.test_tkinter still work?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
It would work if a __main__ module gets added. But it was test.test_tk before, not test.test_tkinter.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Ah you care about this command? If you do, I can add a __main__.py file.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Yes, please add it. And test different ways of running tests:
python -m test.test_xxx
python -m unittest test.test_xxx
python -m test test_xxx
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I added __main__ sub-modules. Example:
./python -m test.test_ttk -v
./python -m test test_ttk -v -u all
./python -m unittest test.test_ttk -v
The 3 commands end with: Ran 308 tests.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Why not use just loader.discover()?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I copy/paste the code from test_asyncio, it works, so I didn't change it :-)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Does loader.discover() work with zipimport?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Does loader.discover() work with zipimport?
I have no idea. If there is an issue, I suggest to fix all load_tests() functions of Lib/test/ in a separated PR.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Should it be backported? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I would prefer that ttk tests be included under test_tkinter, with names like |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
This patch will require changes to the scripts *nix distributions use to make tkinter (and IDLE) a separate install. I don't know how they would feel about backports. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I rebased the PR to get the fix for the Windows build: #94068 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
This specific PR change the name of two tests, so no, it should not be backported. It also changes the build system (Linux, VS project). Honestly, it sounds risky and I would prefer to not backport it. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Oh. I tried to minimize changes in this PR. But I'm open to reorganizing tests. @serhiy-storchaka: Are you fine with the 2 proposed tests, or do you want to merge them? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Once issue #54781 will be fixed, I will properly document these changes in What's New in Python 3.12. IMO it fits into the Build Changes section. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
I was going to ask about merging them, but I am fine with the current state of this PR. If you are going to merge Maybe do this in a separate PR? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Oh, merging 3 Tkinter tests sound out of my skills. I don't know well this module and I would prefer if someone else do it. Here I'm trying to focus on fixing the 12 years old issue #54781 :-) Perfect is the enemy of good ;-) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@serhiy-storchaka: Does it look good to you like that? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@serhiy-storchaka wrote "I am fine with the current state of this PR". I plan to merge the PR tomorrow if I don't hear back from @serhiy-storchaka. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
* Add Lib/test/test_ttk/__init__.py based on test_ttk_guionly.py. * Remove Lib/test/test_ttk_guionly.py.
Remove Lib/test/test_tk.py.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.