Skip to content

test(footer): increase initial pause in flaky test #6032

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 12, 2025

Conversation

TomJGooding
Copy link
Contributor

@TomJGooding TomJGooding commented Aug 8, 2025

Increase the initial pause in test_footer_bindings and add sanity check to ensure the footer key is actually clicked.

Hopefully actually fixes #6020 this time!

Please review the following checklist.

  • Docstrings on all new or modified functions / classes
  • Updated documentation
  • Updated CHANGELOG.md (where appropriate)

@TomJGooding
Copy link
Contributor Author

Previously this test has only seemed flaky on windows-latest, 3.13, so interesting that it failed on windows-latest, 3.9 this time...

Anyway, this seems to confirm my suspicion that the reason this test sometimes fails is that the footer key is never actually clicked.

            # await pilot.click("Footer", offset=(1, 0))
            footer_key_clicked = await pilot.click("FooterKey")
>           assert footer_key_clicked
E           assert False

tests\footer\test_footer.py:58: AssertionError

I'm guessing there's a short delay before the footer is fully composed. Why the pilot.pause() and wait_for_refresh() don't seem to account for this, I have no idea!

@TomJGooding TomJGooding changed the title test(footer): check footer key clicked in CI test(footer): increase initial pause in flaky test Aug 8, 2025
@TomJGooding TomJGooding marked this pull request as ready for review August 8, 2025 13:29
@davidfokkema
Copy link
Contributor

Previously this test has only seemed flaky on windows-latest, 3.13, so interesting that it failed on windows-latest, 3.9 this time...

These kinds of bugs are very irritating. It might help to resolve the 'why almost always only on 3.13' question if you tried disabling the fail-fast strategy. The workers run in parallel and the first worker with an error cancels the other workers. So the fastest Python 'wins', which is 3.13 (I guess, with the whole make-python-faster-with-each-version-master-plan). If all versions of Python prove to fail on Windows, that's one less mystery. That doesn't necessarily bring you closer to the solution, but I'd be good to know if it is just Windows-only, or really also version-specific.

@TomJGooding
Copy link
Contributor Author

TomJGooding commented Aug 11, 2025

Agreed flaky tests are annoying to debug. You think you've fixed the issue after a few successful CI runs, only to reappear almost immediately after the "fix" is merged!

I've noticed this test failing when windows-latest, 3.13 is last in the queue and all other runs pass, so I don't think it's "fastest wins". Unless I've misunderstood what you mean...

@davidfokkema
Copy link
Contributor

I've noticed this test failing when windows-latest, 3.13 is last in the queue and all other runs pass, so I don't think it's "fastest wins". Unless I've misunderstood what you mean...

It may be last in the queue but if it is faster it may finish first. But don't mind me, I checked the workflow run again and I see that 3.9 up to and including 3.12 actually succeeded, and were not cancelled because of 3.13 giving an error. So it does seem to be version-specific. Maybe something changed in asyncio?

@TomJGooding
Copy link
Contributor Author

Your guess is as good as mine! Honestly I'm not convinced it is worth investigating if increasing the initial pause in this test does prevent this flakiness in CI. I have added a sanity check to ensure the footer key is actually clicked, just in case this turns out not to be the fix after all.

@willmcgugan
Copy link
Member

Most of these issues do turn out to be legitimate when investigated. They tend to be race conditions which don't have any impact outside of CI. So while I would like to fix it properly, the pause will at least unblock us until I get to it...

@willmcgugan
Copy link
Member

Thanks, Tom

@willmcgugan willmcgugan merged commit 3a3d046 into Textualize:main Aug 12, 2025
23 checks passed
@TomJGooding TomJGooding deleted the test-footer-flaky-test-redux branch August 13, 2025 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flaky test_footer_bindings test in CI
3 participants