Skip to content
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

Added test for on_gain_focus and on_lose_focus #2158

Closed
wants to merge 1 commit into from

Conversation

dgmouris
Copy link
Contributor

@dgmouris dgmouris commented Oct 19, 2023

Adds a test for on_gain_focus and on_lose_focus

Adds a bit more testing regarding on_lose_focus and on_gain_focus, as I couldn't find them anywhere.

#1601 (perhaps?)

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

@dgmouris
Copy link
Contributor Author

dgmouris commented Oct 19, 2023

Just thought this might be a small help, I came into something on a personal project on losing focus so I thought I'd add a test just for my own peace of mind. If it's not relevant that's fine too!

Don't think there's anything to document here, but I might be wrong.

I'll add the change so towncrier passes shortly.

@freakboy3742
Copy link
Member

Thanks for the suggestion; however in this case, it's already being tested! test_textinput.py imports test_focus from the base property tests, which validates the on_gain_focus and on_lose_focus signals.

This is a general pattern that is followed by a lot of widgets - since the underlying test for focus is fundamentally the same, we can use the same test repeatedly, just replacing the widget that is used as a fixture for that test.

In terms of #1600 and coverage - we're almost there. #2075 is the last PR of the widget audit, and part of that PR is turning on the "fail the test suite if there isn't 100% branch coverage" test condition on both the core and the testbed. As I write this, the core, plus iOS and GTK backends have 100% coverage, macOS backend is currently failing because of 1 line of missing coverage (although this should be corrected in an hour or two).

Closing on the basis that the PR isn't required, but thanks for the effort anyway!

@dgmouris
Copy link
Contributor Author

Thank you for the kind comment, my apologies on this!

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.

2 participants