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.
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
UITester support for TableEditor #1707
UITester support for TableEditor #1707
Changes from all commits
073c9a2
61246c3
2795f7f
5079b8c
450a53f
2fffea6
de1326f
863b60f
d2866cc
86e18ad
e27351e
cdb8743
ef06bec
69c0bba
de73681
7e93c8e
5d17dcf
bd1d58e
0aaf7a3
b25cfd6
f842120
469174c
9fdc526
dc4f4a1
d34f0c2
b83e6b2
5f07956
9b418ff
036dc51
567ac88
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is
data
the right thing here for displayed text? (look at Qt docs forQAbstractItemModel
)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes,
data
withDisplayRole
should provide the text to display.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've just added a
SelectedIndices
query class.For the current implementation, for
TableEditor
,Selected
andSelectedIndices
effectively just serve as proxies to theselected
andselected_indices
traits on the table editor (see code above.)In theory it may be better for these to dive down to the Qt level (?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is one of the weird things with UITester support though IMO. We want to test traitsui editors are working correctly.
So having tests be down at qt level is really what we want. Ie I do something in traitsUI and we verify that does what we expect down in qt land.
However, say I were implementing the handler for the TableEditor to do a
SelectedIndices
query. I would end up basically rewriting:traitsui/traitsui/qt4/table_editor.py
Lines 626 to 643 in 347f0b5
It creates sort of a weird chasing your own tail scenario 🤔
For downstream users they just want to see that when they run their code, ____ is selected. For which this (just using traits on the editor) is perfectly fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UITester
strikes me as being useful for two things:For unit tests of applications you can usually get away with tests at the traits level (does changing this trait in a UI change the corresponding thing in the model? or vice-versa) where you don't really care/can trust that the UI then does the right thing.