Add more unit testing for the Input widget
#2737
Merged
Conversation
This file contains 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
Also add a test for clicking outside of the content.
rodrigogiraoserrao
approved these changes
Jun 6, 2023
Co-authored-by: Rodrigo Girão Serrão <[email protected]>
…into extend-input-widget-tests
davep
added a commit
to davep/textual
that referenced
this pull request
Jun 6, 2023
The branch being removed here seems to be trying to handle keyboard bindings before handling the raw keyboard event. However, in the unit tests, even when a binding is pressed, this code doesn't get called. I strongly suspect this is code that predates changes that were made some time ago in respect to the order in which bindings were processed and their relationship to keystrokes. After removing this all tests are passing just fine and hand-testing `Input` (especially in the demo, for example, both non-password and password incarnations) shows no problems either. All evidence suggests that Textualize#2737 was incapable of hitting that branch of code because it just could not be hit these days.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adding more unit tests for the
Inputwidget, trying to cover as much code as possible.This gets the
Inputtesting as close to 100% coverage as possible.get_content_heightisn't being used here (and really I'm not sure what sort of testing -- short of just calling it and seeing it it's equal to 1 -- would be sensible here) and there's handling of bindings in_on_keythat is never done, even if a bound key is pressed.On the latter point see #2742 -- I'm pretty sure that that code can be removed (I strongly suspect it's a hangover from before some changes were made last year to how bindings are processed).
I'll nuke that bit of code once these tests are in.