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 — gh-95069: Move tests from `idlelib.idle_test` to `test.test_idle` by sobolevn · Pull Request #115444 · python/cpython

Conversation

@sobolevn

@sobolevn sobolevn commented Feb 14, 2024 •
edited by terryjreedy
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

This work is based on #94145

I've addressed the original feedback.

move unittest-related files in idlelib.idle_test to a new test/test_idle directory and split test/test_idle.py into test_idle/__init__.py and test_idle/__main__.py

Done ✅

Note that I want idle_test and any non-unittest files (at least htest.py and part of README.txt) left as they are.

Done ✅

Among other comments:

  • git should handle the path change smoothly
  • The diff itself is very small

@terryjreedy please, share your feedback, so we can make this transition as convenient for you as possible :)

  • Issue: IDLE: Move unittests to test directory #95069

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@sobolevn
pythongh-95069: Move tests from idlelib.idle_test to test.test_idle
533dd45
@sobolevn sobolevn added the skip news label Feb 14, 2024
@sobolevn
sobolevn requested a review from vstinner February 14, 2024 10:21
@sobolevn
sobolevn requested review from 1st1 and terryjreedy as code owners February 14, 2024 10:21
@bedevere-app bedevere-app Bot added the awaiting core review label Feb 14, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Feb 14, 2024
IDLE: Move unittests to test directory #95069
Open
sobolevn added 3 commits February 14, 2024 14:12
@sobolevn
Fix CI
998374a
@sobolevn
Fix CI
24fd790
@sobolevn
Fix CI
3f42787
@vstinner

vstinner commented Feb 14, 2024

Copy link
Copy Markdown
Member

Well, for these tests, I will rely on @terryjreedy's decision. If he is fine with the overall approach, I can review the implementation.
All reactions

Sorry, something went wrong.

Uh oh!

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

terryjreedy commented Feb 15, 2024

Copy link
Copy Markdown
Member

I am still opposed to the idea (see issue) but will review this to see if it satisfies my objections to the previous proposed implementation.
vstinner reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
merwok
merwok reviewed Feb 15, 2024
View reviewed changes
Comment thread Makefile.pre.in Resolved

Uh oh!

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

sobolevn commented Mar 13, 2024

Copy link
Copy Markdown
Member Author

@terryjreedy did you have a chance to look at it? :)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
terryjreedy
terryjreedy requested changes Mar 18, 2024
View reviewed changes

@terryjreedy terryjreedy 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.

I want to make IDLE's tests as independent of location as sensibly possible, but not move them now. The comments are about what can/should go in a new PR to do the former.

This PR could be then simplified and retested before being closed until needed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
sobolevn reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread Lib/idlelib/autocomplete.py
if __name__ == '__main__':
from unittest import main
main('idlelib.idle_test.test_autocomplete', verbosity=2)
main('test.test_idle.test_autocomplete', verbosity=2)

@terryjreedy terryjreedy Mar 18, 2024

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.

I would add utestdir = 'test.test_idle to util.py and from idlelib.util import utestdir above each main line, which would then become main('test.test_idle.test_autocomplete', verbosity=2). I believe easy enough with sed.

Sorry, something went wrong.

Uh oh!

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

@terryjreedy terryjreedy Apr 18, 2024

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.

Actually, if tests were not installed, this would just crash, and prevent running any htest that follows. Running a file's unittest would have to be made conditional (or the exception caught). This if tests were moved, this should be encapsulated in a util function that would fail gracefully.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/idlelib/colorizer.py
def _color_delegator(parent): # htest #
from tkinter import Toplevel, Text
from idlelib.idle_test.test_colorizer import source
from test.test_idle.test_colorizer import source

@terryjreedy terryjreedy Mar 18, 2024

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.

This would introduce a bug if IDLE were installed and tests were not (which is normal on macOS) and this were run. My thought is to move the (illegal) sample code into a file in idle_test.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_idle/__init__.py
import unittest
import os

from test import support

@terryjreedy terryjreedy Mar 18, 2024

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.

This and the next change are pure style preference, independent of this issue. I don't care either way.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_idle/test_config.py
idle_dir = os.path.dirname(__file__)
else:
idle_dir = os.path.abspath(sys.path[0])
idle_dir = os.path.abspath(os.path.dirname(idlelib.__file__))

@terryjreedy terryjreedy Mar 18, 2024

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.

I believe all these test file changes make them independent of test file location. If so, they can be done now without moving the files. It would be easy to find out. Some of these look like improvements in any case. I would add a note to idle_test.README.txt that tests should not depend on being in idle_test.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot added awaiting changes and removed awaiting core review labels Mar 18, 2024
@bedevere-app

bedevere-app Bot commented Mar 18, 2024

Copy link
Copy Markdown

When you're done making the requested changes, leave the comment: 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.
@github-actions

github-actions Bot commented Apr 15, 2026

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 15, 2026
@ambv

ambv commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

This PR is blocked on its author, has not been touched by a human in more than 2 years, and no longer merges cleanly. Closing to cull the number of open pull requests. Do not reopen this pull request without getting a go-ahead from a triager or core developer on gh-95069.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ambv ambv closed this Oct 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@merwok merwok merwok left review comments
@terryjreedy terryjreedy terryjreedy requested changes
@vstinner vstinner Awaiting requested review from vstinner
@1st1 1st1 Awaiting requested review from 1st1

Uh oh!

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

Assignees

No one assigned

Labels

awaiting changes skip news stale Stale PR or inactive for long period of time.

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

@sobolevn @vstinner @terryjreedy @ambv @merwok
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.