◐ Shell
reader mode source ↗
Skip to content

bpo-35771: IDLE: Fix and optimize flaky tool-tip hover delay tests#15634

Merged
terryjreedy merged 6 commits into
python:masterfrom
taleinat:bpo-35771/idle-tooltip-hover-delay-tests
Sep 3, 2019
Merged

bpo-35771: IDLE: Fix and optimize flaky tool-tip hover delay tests#15634
terryjreedy merged 6 commits into
python:masterfrom
taleinat:bpo-35771/idle-tooltip-hover-delay-tests

Conversation

@taleinat

@taleinat taleinat commented Sep 1, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

A 50ms delay will often causes failures on slower machines.

This PR is an alternative to PR GH-14926. In this PR, both of the tests with a delay have been merged into one, to avoid multiple time.sleep() calls, reducing the overall run time.

For reference, the total run time for test_idle on my machine is about 7.5 seconds. 0.15 seconds, which is the delay time added by these tests, is 2% of that.

https://bugs.python.org/issue35771

@taleinat taleinat requested a review from terryjreedy September 1, 2019 07:26
@taleinat

taleinat commented Sep 1, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@ZackerySpytz, your review would be welcome.

@terryjreedy, this is not the approach you recommended in your comment on GH-14926. IMO, in this case, it is better to actually test with Tk rather than with a mock.

terryjreedy
terryjreedy approved these changes Sep 3, 2019
View reviewed changes

@terryjreedy terryjreedy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

An average speedup of .004 seconds and greatly decreased chance of spurious failure is a win. I will merge with my revisions when CI passes.

@terryjreedy terryjreedy merged commit 132acab into python:master Sep 3, 2019
@miss-islington

miss-islington commented Sep 3, 2019

Copy link
Copy Markdown
Contributor

Thanks @taleinat for the PR, and @terryjreedy for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@miss-islington

miss-islington commented Sep 3, 2019

Copy link
Copy Markdown
Contributor

Sorry @taleinat and @terryjreedy, I had trouble checking out the 3.8 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 132acaba5a7f01373ca624b1a5975b190fe866f5 3.8

@bedevere-bot

bedevere-bot commented Sep 3, 2019

Copy link
Copy Markdown

GH-15657 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 3, 2019
@taleinat @miss-islington
bpo-35771: IDLE: Fix flaky tool-tip hover delay tests (pythonGH-15634) …
40cf0b4 
Extending the hover delay in test_tooltip should avoid spurious test_idle failures.
One longer delay instead of two shorter delays results in a net speedup.
(cherry picked from commit 132acab)

Co-authored-by: Tal Einat <taleinat+github@gmail.com>
@miss-islington

miss-islington commented Sep 3, 2019

Copy link
Copy Markdown
Contributor

Thanks @taleinat for the PR, and @terryjreedy for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

@bedevere-bot

bedevere-bot commented Sep 3, 2019

Copy link
Copy Markdown

GH-15658 is a backport of this pull request to the 3.8 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 3, 2019
@taleinat @miss-islington
bpo-35771: IDLE: Fix flaky tool-tip hover delay tests (pythonGH-15634) …
eac8da1 
Extending the hover delay in test_tooltip should avoid spurious test_idle failures.
One longer delay instead of two shorter delays results in a net speedup.
(cherry picked from commit 132acab)

Co-authored-by: Tal Einat <taleinat+github@gmail.com>
@terryjreedy terryjreedy mentioned this pull request Sep 3, 2019
Closed
miss-islington added a commit that referenced this pull request Sep 3, 2019
@miss-islington @taleinat
bpo-35771: IDLE: Fix flaky tool-tip hover delay tests (GH-15634) …
4805805 
Extending the hover delay in test_tooltip should avoid spurious test_idle failures.
One longer delay instead of two shorter delays results in a net speedup.
(cherry picked from commit 132acab)

Co-authored-by: Tal Einat <taleinat+github@gmail.com>
miss-islington added a commit that referenced this pull request Sep 3, 2019
@miss-islington @taleinat
bpo-35771: IDLE: Fix flaky tool-tip hover delay tests (GH-15634) …
1c18aec 
Extending the hover delay in test_tooltip should avoid spurious test_idle failures.
One longer delay instead of two shorter delays results in a net speedup.
(cherry picked from commit 132acab)

Co-authored-by: Tal Einat <taleinat+github@gmail.com>
lisroach pushed a commit to lisroach/cpython that referenced this pull request Sep 10, 2019
@taleinat @lisroach
bpo-35771: IDLE: Fix flaky tool-tip hover delay tests (pythonGH-15634) …
c4e023a 
Extending the hover delay in test_tooltip should avoid spurious test_idle failures.
One longer delay instead of two shorter delays results in a net speedup.
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
@taleinat @DinoV
bpo-35771: IDLE: Fix flaky tool-tip hover delay tests (pythonGH-15634) …
069af45 
Extending the hover delay in test_tooltip should avoid spurious test_idle failures.
One longer delay instead of two shorter delays results in a net speedup.
websurfer5 pushed a commit to websurfer5/cpython that referenced this pull request Jul 20, 2020
@taleinat @websurfer5
bpo-35771: IDLE: Fix flaky tool-tip hover delay tests (pythonGH-15634) …
f2443ea 
Extending the hover delay in test_tooltip should avoid spurious test_idle failures.
One longer delay instead of two shorter delays results in a net speedup.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@terryjreedy terryjreedy terryjreedy approved these changes

Assignees

@terryjreedy terryjreedy

Labels

Tests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@taleinat @miss-islington @bedevere-bot @terryjreedy @the-knights-who-say-ni