FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-35771: IDLE: Fix and optimize flaky tool-tip hover delay tests by taleinat · Pull Request #15634 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

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
ConversationCommits (6)ChecksFiles changed
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 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 added tests Tests in the Lib/test dir needs backport to 3.7 labels Sep 1, 2019
taleinat requested a review from terryjreedy September 1, 2019 07:26

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 approved these changes Sep 3, 2019
View reviewed changes

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

Comment thread Lib/idlelib/idle_test/test_tooltip.py Outdated
@@ -1,27 +1,31 @@
from functools import wraps

terryjreedy Sep 3, 2019

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This first few changes are irrelevant to the issue and are a regression from the 'fail fast (as soon as possible)' design used in IDLE tests. If importing the tested module fails, doing anything else is useless. If the whole module requires gui and that fails (as is does for a majority of buildbots) doing anything else is useless. I have changed back to this.

I am also adding module docstring with coverage.

Comment thread Lib/idlelib/idle_test/test_tooltip.py Outdated
global root
root.update()


terryjreedy Sep 3, 2019

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

idlelib is not consitently double spaced around functions. To me, it is less needed than for classes. And PEP8 has this, which I think applies here: "Blank lines may be omitted between a bunch of related one-liners".

That aside, root_update just adds call overhead. I am replacing it.

Comment thread Lib/idlelib/idle_test/test_tooltip.py
self.top, self.button = _make_top_and_button(self)

def is_tipwindow_shown(self, tooltip):
return tooltip.tipwindow and tooltip.tipwindow.winfo_viewable()

terryjreedy Sep 3, 2019

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I might have used an even shorten name ('tip_shown'), but even as is, this is a definite improvement in readability.

Comment thread Lib/idlelib/idle_test/test_tooltip.py

def test_dont_show_on_mouse_leave_before_delay(self):
tooltip = Hovertip(self.button, 'ToolTip text', hover_delay=50)
time.sleep(0.15)

terryjreedy Sep 3, 2019

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

One sleep of .15 (to Windows accuracy of around .017) is faster than 2 sleeps of .10 (ditto).

Comment thread Misc/NEWS.d/next/IDLE/2019-09-01-10-22-55.bpo-35771.tdbmbP.rst Outdated
@@ -0,0 +1,2 @@
Increase the ``hover_delay`` in IDLE's test_tooltip, and run both tests
waiting for a delay to expire simultaneously, to reduce total run time.

terryjreedy Sep 3, 2019

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I changed this to what users are more likely to want to know. "To avoid occasional spurious test_idle failures on slower machines, increase the hover_delay in test_tooltip."

terryjreedy merged commit 132acab into python:master Sep 3, 2019

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 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 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
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 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 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
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 mentioned this pull request Sep 3, 2019
Closed
miss-islington added a commit that referenced this pull request Sep 3, 2019
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
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
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
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
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

terryjreedy terryjreedy approved these changes

Assignees

terryjreedy

Labels

tests Tests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL