Ui tester api updates4 - Implement locating textbox in RangeEditor and performing KeySequence #1171

aaronayres35 · 2020-08-28T13:22:55Z

Implements the sixth bullet on issue #1149 (Implement locating the textbox in RangeEditor and then perform KeySequence event on it.)

…and added delay)

…wrapper (and a couple simple fixes)

…click

…e new UITester

… more detailed documentation for it

traitsui/qt4/range_editor.py

aaronayres35 · 2020-08-31T13:49:12Z

traitsui/testing/tester/qt4/implementation/range_editor.py

+    LargeRangeSliderEditor,
+    LogRangeSliderEditor,
+    RangeTextEditor,
+    SimpleSliderEditor,


Eventually we will want to add SimpleSpinEditor, but this can be done in a later PR.
For later reference: The SimpleSpinEditor class in Qt has a control that is a QtGui.QSpinBox object, which subclasses QtGui.AbstractSpinBox which has a lineEdit() method which returns the QLineEdit for the spin box (likely we would just need to resolve to this and then the other handlers should work). For wx however, the SimpleSpinEditor has a control of a wx.SpinCtrl, which is supposedly combines a wx.SpinCtrl and wx.TextCtrl in one. However, it doesn't seem to subclass wx.TextCtrl and doesn't appear to support the same methods, so our handlers for KeyClick, KeySequence, etc. may not work correctly.

kitchoi

Thank you! Some comments to do with documentation for developers and error feedbacks for the users. Mostly LGTM!

I think we can normalize the use of \b in KeySequence for Qt and Wx.

This is worth noting: With this PR, the test coverage for traitsui.wx.range_editor went from 25% to 60%, and the test coverage for traitsui.qt.range_editor went from 40% to 65%. 🎉

traitsui/testing/tester/locator.py

traitsui/testing/tester/qt4/default_registry.py

traitsui/testing/tester/qt4/implementation/range_editor.py

traitsui/qt4/range_editor.py

kitchoi · 2020-09-01T11:03:04Z

traitsui/tests/editors/test_range_editor.py

+    # There is a problem with KeySequence on wx.  trying to include the key
+    # '\b' does not succesfully type a backspace.  Instead EmulateKeyPress
+    # seems to literally type "\x08" which leads to errors.


If both the wx and qt widgets respond to the backsapce keys correctly, that is, the behaviour is the same, then the test code should be toolkit agnostic. That's the purpose of the tester API and we should aim to support that.

I'd propose we translate \b to the backspace key for wx TextCtrl such that the test code can use \b in KeySequence. In other words, in key_sequence_text_ctrl, we do this:

if char == "\b": char = "Backspace"

Then we can run check_set_with_text for both qt and wx. What do you think?

That makes sense so me, but would we then need to hard code all other escape characters? for example '\b', '\t', '\n', etc. That may be just what we need to do and that is perfectly fine. I had just left the tests separate for now. so that the issue was known and we could try to address it. It seems very likely to me that this may be the only solution, but I didn't know if there may would be a way to get the escape characters to work directly without explicitly checking and swapping them for the pyface key names.

There are only a few of these characters so I think it is not a big deal: We can maintain this as a separate mapping. Alternatively we can forbid the use of \b in KeySequence and force test code to use KeyClick("Backspace"), but we should choose one route, not both.

KeyClick("Backspace") with an optional argument for the # of backspaces sounds a lot better than manually doing \b.

OK, then we would have to check for \b in the Qt implementation to disallow it. Otherwise test code trying to use \b would not be toolkit agnostic. In either case, these escape characters need to be handled one way or another.

Just to clarify, I think either way is fine.

KeyClick("Backspace") with an optional argument for the # of backspaces sounds a lot better than manually doing \b.

It makes sense to me to support both. My understanding is just that we want the code to be toolkit agnostic. I added kits suggestion for now so that '\b' works on both wx and qt. Maybe in a later pr we can add to the KeyClick functions to take an optional argument to repeat the specified character n times.

traitsui/testing/tester/qt4/located_object_handlers.py

traitsui/tests/editors/test_range_editor_text.py

kitchoi

Now I looked a bit more into the "\b" business, looks like wx is actually happy with it.

traitsui/testing/tester/locator.py

traitsui/testing/tester/qt4/implementation/range_editor.py

traitsui/testing/tester/wx/implementation/range_editor.py

traitsui/testing/tester/wx/helpers.py

kitchoi · 2020-09-01T14:50:38Z

traitsui/testing/tester/wx/helpers.py

@@ -43,7 +43,7 @@ def key_click(widget, key, delay=0):
        else:
            wx.MilliSleep(delay)
            key_event = wx.KeyEvent(wx.wxEVT_CHAR)
-            key_event.SetUnicodeKey(KEY)
+            key_event.SetKeyCode(KEY)


I think this results in another test to fail:

====================================================================== FAIL: test_key_click (traitsui.testing.tester.wx.tests.test_helpers.TestInteractions) ---------------------------------------------------------------------- Traceback (most recent call last): File "/Users/kchoi/Work/ETS/traitsui-clone/traitsui/testing/tester/wx/tests/test_helpers.py", line 96, in test_key_click self.assertEqual(textbox.Value, "A") AssertionError: 'a' != 'A' - a + A

I think the test failure makes sense because to type "A", one needs to have the cap on or have used the Shift key. Apparently Qt disagrees and would happily insert a capitalized "A" in that case. So the two disagree and we should normalize it. 😂
That's orthogonal though we should open a separate issue to solve.

Once #1176 is merged and if we re-run the CI for this PR we should see this test failure.

Should I modify the test to pass and comment on the discrepancy? I can open a separate issue about it

I realized that the tests with wx and key clicks are actually failing on Windows.... this is getting orthogonal. Let's revert this change to SetKeyCode and modify the tests for RangeEditor here so that it does not rely on backspaces, then open a separate issue to fix the discrepancies.

kitchoi

LGTM!

kitchoi · 2020-09-01T18:08:17Z

Sigh, we did this again... It is hard to see new test failures with wx CI job being allowed to fail. Here are the new errors on Windows:

======================================================================
FAIL: test_large_range_slider_editor_set_with_text (traitsui.tests.editors.test_range_editor.TestRangeEditor)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\appveyor\.edm\envs\traitsui-test-3.6-wx\lib\site-packages\traitsui\tests\editors\test_range_editor.py", line 77, in test_large_range_slider_editor_set_with_text
    return self.check_set_with_text(mode='xslider')
  File "C:\Users\appveyor\.edm\envs\traitsui-test-3.6-wx\lib\site-packages\traitsui\tests\editors\test_range_editor.py", line 70, in check_set_with_text
    self.assertEqual(model.value, 4)
AssertionError: 1 != 4
======================================================================
FAIL: test_log_range_slider_editor_set_with_text (traitsui.tests.editors.test_range_editor.TestRangeEditor)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\appveyor\.edm\envs\traitsui-test-3.6-wx\lib\site-packages\traitsui\tests\editors\test_range_editor.py", line 80, in test_log_range_slider_editor_set_with_text
    return self.check_set_with_text(mode='logslider')
  File "C:\Users\appveyor\.edm\envs\traitsui-test-3.6-wx\lib\site-packages\traitsui\tests\editors\test_range_editor.py", line 70, in check_set_with_text
    self.assertEqual(model.value, 4)
AssertionError: 1 != 4
======================================================================
FAIL: test_range_text_editor_set_with_text (traitsui.tests.editors.test_range_editor.TestRangeEditor)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\appveyor\.edm\envs\traitsui-test-3.6-wx\lib\site-packages\traitsui\tests\editors\test_range_editor.py", line 83, in test_range_text_editor_set_with_text
    return self.check_set_with_text(mode='text')
  File "C:\Users\appveyor\.edm\envs\traitsui-test-3.6-wx\lib\site-packages\traitsui\tests\editors\test_range_editor.py", line 70, in check_set_with_text
    self.assertEqual(model.value, 4)
AssertionError: 1 != 4
======================================================================
FAIL: test_simple_slider_editor_set_with_text (traitsui.tests.editors.test_range_editor.TestRangeEditor)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\appveyor\.edm\envs\traitsui-test-3.6-wx\lib\site-packages\traitsui\tests\editors\test_range_editor.py", line 74, in test_simple_slider_editor_set_with_text
    return self.check_set_with_text(mode='slider')
  File "C:\Users\appveyor\.edm\envs\traitsui-test-3.6-wx\lib\site-packages\traitsui\tests\editors\test_range_editor.py", line 70, in check_set_with_text
    self.assertEqual(model.value, 4)
AssertionError: 1 != 4

aaronayres35 · 2020-09-01T19:08:38Z

Sigh, we did this again... It is hard to see new test failures with wx CI job being allowed to fail. Here are the new errors on Windows:

:/ At least it seems to be the effectively same issue #1177.

aaronayres35 added 30 commits August 19, 2020 15:34

first round of updates for UI Tester (default_registry toolkit setup …

ba2378f

…and added delay)

attempt to handle Null toolkit

a6d28ca

fixing null handling

28d5094

setting up MouseClick for ButtonEditor

684be1c

adding DefaultTarget locator class and logic to handle its use in ui_…

9973d38

…wrapper (and a couple simple fixes)

addressing comments

547b52b

merging changes from ui-tester-api-updates1 after review comments

7fecb2a

this test update was accidentally left out of merge

a6ede42

adding tests (currently broken due to imports)

69d5cd5

adding test for toolkit specific default registries which test mouse …

b97a923

…click

fixing broken tests, and convert TraitsUI test for ButtonEditor to us…

39c034e

…e new UITester

addressing comments

57fcdda

Merge branch 'ui-tester-api-updates1' into ui-tester-api-updates2

dcdfdf8

updating helper functions and adding docstrings

6266a83

Merge branch 'master' into ui-tester-api-updates2

578b280

adding __init__.py files

fe4c597

adding docstring for DefaultTarget locator class

df81380

first steps for KeySequence, KeyClick and DisplayedText for TextEditor

9803375

add QLineEdit, change key press names to click

9077c08

docstring updates and simple name change

400d9d3

fixing typo

33f750f

adding tests for new UIWrapper DefaultTarget logic

51e5b45

flake8 fixes

3149374

rename wobject wx_object

c43c88a

fixing wx test failures

e444628

more flake8 fixes

d4d8012

making suggested changes from comments

848d513

pull new DefaultTarget logic out into its own private method, and add…

9753816

… more detailed documentation for it

fixing failures from last commit

db80b28

first steps towards updating tests

bc0e7f1

aaronayres35 commented Aug 28, 2020

View reviewed changes

traitsui/qt4/range_editor.py Outdated Show resolved Hide resolved

aaronayres35 added 5 commits August 28, 2020 11:02

fixing broken wx tests

a287c52

flake8

7c595ca

removing commented out old code

7847933

missed a flake8

991be83

deleted an old unneeded test

f08e54f

aaronayres35 marked this pull request as ready for review August 28, 2020 17:19

aaronayres35 requested a review from kitchoi August 28, 2020 17:19

adding docstrings and a function rename

ee863e5

aaronayres35 changed the title ~~Ui tester api updates4~~ Ui tester api updates4 - Implement locating textbox in RangeEditor and performing KeySequence Aug 31, 2020

aaronayres35 commented Aug 31, 2020

View reviewed changes

fixing typos and adding a few docstrings

73a97d4

aaronayres35 mentioned this pull request Aug 31, 2020

Ui tester api updates5 - Implement locating a nested UI for a custom style ListEditor #1172

Closed

kitchoi reviewed Sep 1, 2020

View reviewed changes

aaronayres35 added 3 commits September 1, 2020 09:04

making suggested changes

b806e3a

handling '\b' characters on wx

78a7bf0

renaming located_object_handlers as common_ui_targets

c03ef46

aaronayres35 mentioned this pull request Sep 1, 2020

UI Tester update - Implement locating a nested UI for a custom style ListEditor #1175

Merged

kitchoi reviewed Sep 1, 2020

View reviewed changes

more suggested changes

f8b4b9c

kitchoi reviewed Sep 1, 2020

View reviewed changes

revert SetKeyCode change and remove '\b' in test

4f723e1

aaronayres35 mentioned this pull request Sep 1, 2020

UI Tester Key click discrepancy between Qt and Wx #1178

Closed

kitchoi approved these changes Sep 1, 2020

View reviewed changes

aaronayres35 merged commit d0b3ddd into master Sep 1, 2020

aaronayres35 deleted the ui-tester-api-updates4 branch September 1, 2020 16:52

kitchoi mentioned this pull request Sep 2, 2020

Normalize KeySequence support for testing textboxes #1181

Merged

aaronayres35 mentioned this pull request Sep 3, 2020

Expand UI Tester Range Editor implementation #1188

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ui tester api updates4 - Implement locating textbox in RangeEditor and performing KeySequence #1171

Ui tester api updates4 - Implement locating textbox in RangeEditor and performing KeySequence #1171

aaronayres35 commented Aug 28, 2020

aaronayres35 Aug 31, 2020

kitchoi left a comment

kitchoi Sep 1, 2020

aaronayres35 Sep 1, 2020

kitchoi Sep 1, 2020

rahulporuri Sep 1, 2020

kitchoi Sep 1, 2020

kitchoi Sep 1, 2020

aaronayres35 Sep 1, 2020

kitchoi left a comment

kitchoi Sep 1, 2020

kitchoi Sep 1, 2020

aaronayres35 Sep 1, 2020

kitchoi Sep 1, 2020

kitchoi left a comment

kitchoi commented Sep 1, 2020

aaronayres35 commented Sep 1, 2020

Ui tester api updates4 - Implement locating textbox in RangeEditor and performing KeySequence #1171

Ui tester api updates4 - Implement locating textbox in RangeEditor and performing KeySequence #1171

Conversation

aaronayres35 commented Aug 28, 2020

Choose a reason for hiding this comment

kitchoi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kitchoi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kitchoi left a comment

Choose a reason for hiding this comment

kitchoi commented Sep 1, 2020

aaronayres35 commented Sep 1, 2020