Skip to content

Ui tester updates - add enum editor implementation#1191

Merged
aaronayres35 merged 44 commits intomasterfrom
ui-tester-updates-EnumEditor
Sep 15, 2020
Merged

Ui tester updates - add enum editor implementation#1191
aaronayres35 merged 44 commits intomasterfrom
ui-tester-updates-EnumEditor

Conversation

@aaronayres35
Copy link
Contributor

fixes #1187

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.

Refactoring to a base class makes sense to me!

@aaronayres35 aaronayres35 marked this pull request as ready for review September 4, 2020 15:24
@aaronayres35 aaronayres35 requested a review from kitchoi September 4, 2020 15:35
@aaronayres35
Copy link
Contributor Author

There are new errors on wx in the test suite on OSX which I did not observe on my machine. looking into them now

@aaronayres35
Copy link
Contributor Author

There are new errors on wx in the test suite on OSX which I did not observe on my machine. looking into them now

I believe it is a result of the changes in #1193 and what I mentioned above here: #1191 (comment)

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! The testing support for EnumEditor will be very useful!

Some suggestions... Since I will be away for some time, @rahulporuri would you be able to take over please?

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 new comments.
(Reminder to run flake8 again at the end... I think I am annoyed about this enough I am going to put it up, today!)

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.

Looks great. Thank you very much!
Some minor comments.

@aaronayres35
Copy link
Contributor Author

Looking at the CI I saw a test was failing for windows/wx:

======================================================================
FAIL: test_radio_enum_none_selected (traitsui.tests.editors.test_enum_editor.TestRadioEnumEditor)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\appveyor\.edm\envs\traitsui-test-3.6-wx\lib\site-packages\traitsui\tests\editors\test_enum_editor.py", line 382, in test_radio_enum_none_selected
    self.assertEqual(enum_edit.value, None)
AssertionError: 'one' != None
----------------------------------------------------------------------

It seems that for windows/wx the enum traits value is forced to be 'one' whereas for all other platforms/ toolkit combos this is not the case. I am skipping this test for now on windows/wx, and will merge once CI completes (I would like to have it merged today so that I can use this code in other PRs more easily). However, we will likely either want to rewrite the test, or add a separate test for the wx/windows specific case. I am not sure, @kitchoi lmk what you think is best.

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 Enum Editor for UI Tester

3 participants