Skip to content

UI Tester updates - Instance editor implementation#1208

Merged
aaronayres35 merged 10 commits intomasterfrom
ui-tester-updates-InstanceEditor
Sep 15, 2020
Merged

UI Tester updates - Instance editor implementation#1208
aaronayres35 merged 10 commits intomasterfrom
ui-tester-updates-InstanceEditor

Conversation

@aaronayres35
Copy link
Contributor

fixes #1206
Adds a simple implementation for Instance editor, and modifies tests to now use UI Tester. Also adds an additional test to see that the displayed editor updates if a trait changes externally to the editor.

Note: the code for the wx instance editor had to be changed very slightly in order to gain access to the ui of interest. This idea was take from kit's draft branch #1150

Copy link
Contributor

@kitchoi kitchoi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Some comments for discussions, see #1212 which accompanies the review comments.

Comment on lines 37 to 38
editor = instance.locate(locator.NestedUI())
value_txt = editor.find_by_name("value")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suppose we remove press_ok_button (see comment below), could we write value_text = tester.find_by_name(ui, "inst").find_by_name("value") instead?

target : instance of SimpleEditor
"""
if target._dialog_ui is None:
target._button.click()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should move this out to a MouseClick interaction, and only after that the nested UI can be queried.
This will make the test_simple_editor_parent_closed test less awkward... and it can make use of delay?
Apparently the button can be clicked multiple times to launch multiple dialog (all editing the same object).
Extracting the mouse click action to a separate interaction will allow these scenarios to be tested more easily.

This would mean the tests for the simple style and custom style cannot be combined... but I think that's okay, because they do represent different behaviour.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now these two lines can be removed as they are redundant in tests where a mouse click is performed prior to querying for the dialog UI. I am sorry I probably should have done that in #1206 😓 .

If the test code tries to navigate into the dialog UI without performing a mouse click first, the dialog UI is None and we get an error anyway complaining no location solvers are registered on None. It might be nicer to raise early with a different error message but I think it might be a bit overkill.

aaronayres35 and others added 2 commits September 14, 2020 16:36
…1212)

* Move MouseClick to a separate interaction for simple instance editor

* Reuse the wrapper already available

Co-authored-by: aaronayres35 <36972686+aaronayres35@users.noreply.github.com>
Copy link
Contributor

@kitchoi kitchoi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am sorry I omitted to remove the click action in Qt _get_nested_ui_simple in #1206... One last comment.

target : instance of SimpleEditor
"""
if target._dialog_ui is None:
target._button.click()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now these two lines can be removed as they are redundant in tests where a mouse click is performed prior to querying for the dialog UI. I am sorry I probably should have done that in #1206 😓 .

If the test code tries to navigate into the dialog UI without performing a mouse click first, the dialog UI is None and we get an error anyway complaining no location solvers are registered on None. It might be nicer to raise early with a different error message but I think it might be a bit overkill.

Copy link
Contributor

@kitchoi kitchoi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@aaronayres35 aaronayres35 merged commit 294a8f7 into master Sep 15, 2020
@aaronayres35 aaronayres35 deleted the ui-tester-updates-InstanceEditor branch September 15, 2020 17:37
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.

Implement Instance Editor for UI Tester

2 participants