-
Notifications
You must be signed in to change notification settings - Fork 96
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
Initial UITester API for writing toolkit agnostic tests #1107
Conversation
traitsui/testing/tester/ui_tester.py
Outdated
if registries is None: | ||
self._registries = [ | ||
_CustomActionRegistry(), | ||
] |
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.
We will have a second registry (an instance of EditorActionRegistry) here to support testing with TraitsUI editor.
Although this is intended to become a public API eventually, I am thinking that we should keep this "private" to TraitsUI and test this out with TraitsUI's own tests first. When we confirm the API is mature enough, it can be made public. For example, the |
A couple of comments:
If this is going to be successful, writing tests needs to be really, really easy, and any custom interactors/locators written for tests need to be super-simple so test writers can be confident they do the right thing. I'd suggest:
Where I see this framework as being particularly useful is for people who are writing integration tests for complex UIs. That isn't a common use-case we have in the TraitsUI tests, but it does lead me to think we should be designing this with a view to making it public (even if it is soft-launched). (edited to complete the thought in the second bullet point above) |
A few simple widgets have this sort of methods for programmatically doing these sort of things, that is true. But there are widgets don't have that kind of convenient methods. For example, in order to perform a mouse click on an QAbstractItemView, there is not such traitsui/traitsui/testing/qt4/helpers.py Lines 59 to 87 in 6b0f898
In the context of clicking a cell in a table, the test does not need to care what is in the cell. It just instructs the UI to perform a mouse click on a specific cell:
Likewise there are no click methods for traitsui/traitsui/testing/qt4/default_registry.py Lines 385 to 393 in 6b0f898
Furthermore for testing, there is a distinction between pressing and releasing a key with or without emitting an event indicating value change. For example, to test the
Operations like mouse click or key click are directed at the toolkit controls, but that detail is hidden by the API by design. The practical reason for this is because currently these toolkit controls are all over the place. For example, the QTreeView is found in There are just so much implementation details involved in getting hold of a toolkit control for performing a specific mouse click. The end user should not be bothered by this. The If there was a toolkit agnostic layer that provide access to all these sorts of controls, that would be nice... but would also require quite a lot of efforts. This API hides these details away so that we can do this kind of refactoring later without affecting test code downstream.
We do use Using
The above example shows how we can get hold of a nested UI in a custom editor without exposing implementation details:
The first
Indeed that is why we have On the effort front: We currently don't have these locator code because, sadly, we don't have good tests in TraitsUI. We will be writing these locator code anyway if we are to improve the test coverage in TraitsUI. This is an example of existing test code in TraitsUI on this use case:
In other words, this would be effort we need to spend anyway in order to improve test coverage in TraitsUI. In other other words, I think this kind of locator code would have existed already if TraitsUI had done better testing.
I am not ruling that out, but currently I find that hard to do without concrete use case and user code. It would be easier to do abstraction where there are already code demonstrating use cases. One thing we want to avoid is early abstraction, and worse, early abstraction that is a public API in a library depended upon externally. I think the implementation of |
I had considered giving just the
|
Perhaps a quick clarification: when I say that it should be given a control I mean a control not the control (ie. not necessarily A key entry handler on a text field in a |
Should I rename the package |
I think I read this the first time, I did not quite understand where this was coming from. Now I guess you might be referring to the situation in TraitsUI where, for example, to add a placeholder to the textbox widget in RangeEditor, TextEditor and SearchEditor, you have to do that three times. I think I can show how this is not an overwhelming issue here. The code to do with performing a mouse click / key click on a generic widget can be refactored, so that all you are left with is to delegate the right widget to these generic functions for each editor. For example, to perform a key press on the RangeEditor, I have this implementation: traitsui/traitsui/testing/qt4/default_registry.py Lines 478 to 487 in 6b0f898
(I might change the API of the location object; I don't like using index here) The implementation for This pattern is really useful for situations where the underlying widget are actually different, but conforming to a common API. For example, This is used for traitsui/traitsui/testing/qt4/default_registry.py Lines 196 to 203 in 6b0f898
This is used for TableEditor :traitsui/traitsui/testing/qt4/default_registry.py Lines 224 to 231 in 6b0f898
Notice that the traitsui/traitsui/testing/locator.py Lines 18 to 30 in 6b0f898
The one for traitsui/traitsui/testing/locator.py Lines 2 to 15 in 6b0f898
Many TraitsUI editors are composition of basic widgets, e.g. SetEditor has lots of widgets. The code to do with delegating a toolkit control for a given location and action on a per-editor basis allows us to maximize backward compatibility while also maximize the flexibility to change an editor's composition. If the composition of an editor needs to change, we just need to adjust the location code, downstream projects test code won't need to change. We can also add more location objects to support increased complexity of widget compositions in an editor. This is the main motivation of this testing framework: Maximizing backward and forward compatibility while allowing downstream projects to write test code. |
It's exactly the fact that there are many editors with sub-controls that I'm trying to address. What I'm trying to get at can be seen fairly clearly in your example: traitsui/traitsui/testing/qt4/default_registry.py Lines 478 to 487 in 6b0f898
This piece of code has two parts:
It's good that you have factored out the interaction as a "helper", but let's say you want to write a mouse click interactor. Now you need to duplicate the code which locates the control in the new interactor, and use a different "helper" most likely. Now you could of course factor out the "locate the sub-control" code, but then your interactors just become two-liners of the form "locate the sub-control" followed by "call a helper to do the actual interaction." So where you'll end up is with tests which look like:
I think things would be better if things were written as:
This PR is mostly about the "locate" things part of your testing infrastructure, but the "things" it is locating are the editor level only. I think that the additional code that locates things within an editor should be part of this framework, so for example, when writing tests for a simple range editor, you have a way to specify additional locators for the various sub-widgets that the tests can use (and it may be that simple functions are enough; but there will be some standard ones - such as give me the main control, or give me this cell in an item view). There may be some cases where you need slightly different code between the toolkits for locating sub-controls (particularly for things like table views), but that can be dealt with either by contributing different locator functions, or by normalizing the APIs (eg. this sub-widget is always called So to summarise:
|
I think this is what I have achieved in the user-facing API. :) traitsui/traitsui/examples/demo/Advanced/Auto_editable_readonly_table_cells.py Lines 171 to 187 in 6b0f898
(Again I think I will change the OrderedWidget(0) to something else, maybe WidgetType(SomeEnum.slider) )
Another example, this is how it looks like for a tree editor: traitsui/traitsui/examples/demo/Standard_Editors/TreeEditor_demo.py Lines 163 to 171 in 6b0f898
Just to clarify, this PR has only implemented the part to do with locating the editor, which is the As a preview, the traitsui/traitsui/testing/ui_tester.py Lines 302 to 318 in 6b0f898
If I understand correctly, you would want the There are two reasons why I can't do that: (2) The location may not refer to a toolkit control. It could refer to a nested UI (which is a TraitsUI object). traitsui/traitsui/examples/demo/Advanced/List_editors_demo.py Lines 133 to 136 in 6b0f898
|
So this code: traitsui/traitsui/examples/demo/Advanced/Auto_editable_readonly_table_cells.py Lines 171 to 187 in 6b0f898
looks more-or-less at the right level for how we want to be writing things. My question is looking at that: what sort of object is I think we might also be using the word "interactor" for two different concepts - I think you are using it for the thing that is being acted on, where I'm meaning the code which performs an interaction. So for clarity, I'll try changing how I refer to things: targets: objects which can be searched for, either as intermediate things (like editors or a table widget, perhaps), or as a leaf; and which can be operated upon (exactly what operations depend upon the type of target; but most operations are at the toolkit-level, like mouse-clicks). Targets potentially go all the way down to sub-widget level (eg. cells in a table, or sub-widgets inside toolkit widgets, like the +/- buttons on a spinbox) operation: a piece of code that does something like a mouse click or key entry (might just be a function, but more likely to be a callable object, such as a key event operation which needs to know what key(s) to send); the exact implementation is likely toolkit depenedent target wrappers: a wrapper around a target object which knows how to find targets related to the wrapped target (eg. subwidgets, etc.) and which can has an API to perform an operation in a generic way. So in this nomenclature:
So you would end up writing code like:
The key thing here is that So I think if we can agree on this being the right way to break things down, I think things are OK. BUT! ...in looking at these examples and considering the pain I've been having with Wx tree widgets, I think there are going to be major problems if we are literally talking about testing things like key presses on sliders. This sort of thing can potentially work when you are talking a homogeneous platform like Android, but we are going to run into problems with cross-platform conventions. For example:
In working at this level we are effectively doing integration tests on the toolkit. So I think the sorts of operations that we need to be providing and using in test writing need to be a level higher: not "the user pressed the left arrow key on the slider", but rather "the slider changed its value to x (and we don't care how the user did it, maybe it was from a left click and maybe it was from a drag)". Now on some toolkits that may need to be implemented by a bunch of actions because there is no direct API call, and there are occasional places where TraitsUI does make some promises about what happens on individual keystrokes. But they. should be the exception not the rule. This isn't super-relevant for this PR, since it doesn't really make any claims about what the operations look like, but it is something to keep in mind. |
Yeah I think that makes a lot of sense. Perhaps the
Yes I agree that trying to test things like key presses on Wx are not as trivial as with Qt. While Wx advertises this traitsui/traitsui/testing/wx/default_registry.py Lines 53 to 71 in 6b0f898
It is not beautiful code, but it is good enough for testing the slider part for the RangeEditor. It is firing an event rather than calling the editor |
So basically, I allowed myself to cheat a bit with a wx.ScrollEvent because it is for testing only. |
In terms of naming, "interaction" and "operation" would both be fine for the operation code, avoid "action" because otherwise you will hate life when testing menus and toolbars :) For the target wrappers I'm not keen on "interactor" because they aren't really doing anything, "interactee" might be better, but it's not great; "Target" would be fine, as would something generic like "UIProxy" or "UIWrapper", and so they should have a similarly named method or attribute like "target" or "ui_object", or even if you end up having a class hierarchy you of these you might go for "editor" or "control" or more specific names for the sort of thing that they are. Naming things is hard. In terms of the level of testing, in your example to test scrolling, I think "cheating" with using a |
I pushed some name changes:
There is a larger conceptual change: The
This is because sometimes, we cannot throw away the locator information until we perform a mouse click or a key click. This is the case for TreeEditor and TableEditor. Given a cell or a node index, we need both the index AND the parent widget (e.g. tree widget) in order to perform a mouse click / key click. With that, the Previously the The abstraction of |
Ready for review. |
traitsui/testing/tester/ui_tester.py
Outdated
account = Account(person=Person()) | ||
tester = UITester() | ||
with tester.create_ui(account, dict(view=view)) as ui: | ||
wrapper = tester.find_by_name(ui, "person").find_by_name("name") |
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 will raise on this branch because I moved the logic to the the LocationRegistry and as an effort to reduce the scope, I removed the solver for getting the nested UI in a custom InstanceEditor. I should remove this description.
The implementation can be added back later once we have the toolkit specific default registry set up.
See for example:
traitsui/traitsui/testing/qt4/default_registry.py
Lines 109 to 113 in ceb8fee
registry.register_location_solver( | |
target_class=CustomInstanceEditor, | |
locator_class=locator.DefaultTarget, | |
solver=resolve_location_custom_instance_editor, | |
) |
traitsui/traitsui/testing/qt4/default_registry.py
Lines 48 to 49 in ceb8fee
def resolve_location_custom_instance_editor(wrapper, location): | |
return wrapper.editor._ui |
traitsui/testing/tester/ui_tester.py
Outdated
traitsui UI object is always added. | ||
""" | ||
|
||
def __init__(self, interaction_registries=None, location_registries=None): |
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.
The original EditorActionRegistry is now renamed and split into two separate registries, one responsible for interactions (perform, inspect), another one responsible for locating nested target (locate). I am actually in minds whether to combine them into just one registry again..
I think I am going back on this: I will combine the registries and let composition occur there. It will make this argument list more manageable. In so far, one does need to modify both registries at the same time while adding testing supporting to a particular target.
I will push this change, but that would be it (and will still be ready for review).
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.
Done. Now I have to name the registry differently. Still open to the suggestion of separating them again.
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.
LGTM! Just had a few nitpick typo comments. Also I like wrapping both registries inside the TargetRegistry
traitsui/testing/tester/registry.py
Outdated
support ``UIWrapper.locate``. If the nested UI has other locator solvers, | ||
then it would be possible to do chain location resolutions, like this:: | ||
|
||
container.locate(NestedUI()).locate(Index(1)) |
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 Index defined in this PR?
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.
No it isn't, I will change the Index(1)
to NestedUI()
which is added here.
This PR supersedes #861: This proposes a toolkit-agnostic API for testing GUI with TraitsUI. The motivation remains the same, but the user facing API has changed.
Objectives/Benefits for users
Benefits for TraitsUI
traitsui.qt4.DateEditor.SimpleEditor.update_object
with a QDate object as argument. This will prevent TraitsUI from refactoring its editors, for example, to move the toolkit specific logic out.For the reviewer
This PR provides only the most fundament groundwork for future extensions. I recommend start by reading the documentation of
UITester
.For further context, in this test branch, I tried to test the (extended) API with different editors and with more complex UIs. For example, I was able to test the
List_editors_demo.py
demo example with the following test code:And if I run this, I can programmatically simulate user actions on the GUI (no clicking nor typing on my part!):
.
Likewise I could also test the TreeEditor (see test code here).
Note that in the above example, the
delay
parameter,find_by_id
andlocate
methods are not implemented in this PR. The example is meant to show how the API can be extended and we can add those features in separate PRs. For those who are interested, they are implemented in the test branch (here, here, and here) An example Qt specific implementation for supporting the tested user interactions can be seen here. (warnings: They have not been cleaned up and so the implementation may change when I open a PR for them).Design
ButtonEditor
. This is because testing is an entirely separate use case, and will evolve for different reasons compared to the editors themselves. GUI frameworks do typically provide testing frameworks as separate modules / namespaces. e.g. Qt hasQtTest
. Wx hasUIActionSimulator
.mouse_click
is implemented forButtonEditor
but not forTextEditor
. That is because polymorphism was achieved by subclassing. Here polymorphism is achieved in ways similar tofunctools.singledispatch
: we dispatch implementations by registration and delegation. With that, we can more easily report what actions are supported for a particular editor. This feedback makes the API more user friendly. See theActionNotSupported
error in the PR.(UIWrapper, any action type)
. Future extension to the functionality can then be achieved by adding more methods and attributes on theUIWrapper
object. For example, the extension to supportdelay
andlocate
(in the above example) is achieved by extending theUIWrapper
object without having to modify the call signature.