[proxy] github.com← back | site home | direct (HTTPS) ↗ | proxy home | ◑ dark◐ light
/ cpython Public

Conversation

Copy link
Contributor

ZackerySpytz commented Jul 24, 2019

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

https://bugs.python.org/issue35771

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

taleinat left a comment

Choose a reason for hiding this comment

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

If increasing the hover delays from 50ms to 100ms, we should likely also increase the duration in the time.sleep() calls, to make this less fragile. I'd guess that increasing from 0.1 to 0.15 should be fine.

Copy link
Member

I would like a news entry. Others have probably experienced this, and if the problem recurs with a longer delay, I would like to know. I would also like a comment on the first function:
Fragile test -- see issue 35771.

Wanting to keep test_idle running time as short as possible, I dislike adding delay. About 60 tests now take about 10 seconds, an average of 180 milliseconds each. But it is hard to experiment without reproducible failure. Would reducing delays make failure more dependable?

Copy link
Contributor Author

I have updated the PR. The news entry may need to be tweaked.

Copy link
Contributor

taleinat left a comment

Choose a reason for hiding this comment

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

This LGTM.

Copy link
Member

Since this is about a bad test for apparently good code, I like to understand it better before approving. An alternative fix would be to skip the bad tests and add a comment to the code. "# Do not touch without manually testing hovering or temporarily activating the test." Or I would like to add a message to the possible false failing asserts.

Copy link
Contributor

taleinat commented Aug 4, 2019

An alternative fix would be to skip the bad tests and add a comment to the code. "# Do not touch without manually testing hovering or temporarily activating the test." Or I would like to add a message to the possible false failing asserts.

Or replace with an htest.

Copy link
Member

Ah, there already is an htest for both no delay and delayed tip. I just remembered that I have a branch that refactors and add tests for some of the completion code. That includes delay code and tests. I should check what I did there. (And stop delaying until I get 100% coverage ;-).

Copy link
Member

Zachery, there is nothing you need to do. I need to reconsider what auto test we need in addition to the current live test and compare to similar idlelib code and tests.

Copy link
Member

Test completions is a use-gui tests, but in #15121, I mocked the after functions to avoid delays and timing issues. I want to try to do the same here for the same reasons (and close this). The existing htest is an adequate live test. Any corresponding unittest need not test or depend on tk.

Copy link
Contributor Author

For what it's worth, I would appreciate it if this patch was merged. I understand that this may not be a proper long-term fix, but I still frequently see failures in test_tooltip.py when running the full test suite. I see no such failures when this patch is applied.

Copy link
Member

Thanks for the determination of how much more delay seems to be enough. This is closed in favor of #15634.

terryjreedy closed this Sep 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants