Skip to content
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

Support testing with simple FileEditor (without dialog) #1571

Merged
merged 6 commits into from
Apr 6, 2021

Conversation

kitchoi
Copy link
Contributor

@kitchoi kitchoi commented Apr 6, 2021

This PR adds testing support for the simple FileEditor via its text widget.

  • DisplayedText, KeyClick, KeySequence can now be used for testing with a simple FileEditor
  • A test for the FileEditor demo example is added, and an entry is added to the documentation.

This is motivated by an observation of a similar extension being made in a downstream project. One will extend using the _file_name attribute on the editor, acknowledging that such extension relies on implementation details of the editor. If the testing support is implemented in traitsui, then the support can be maintained along with those implementation details.

In the context of testing an application, it is often sufficient to set a file path via the text widget instead of via the file modal dialog.

This goes some way towards solving #1339. It also extends TraitsUI's own test coverage for the simple FileEditor.

Observations:

  • The smoke tests for wx FileEditor used to fail, but not any more since Simple FileEditor cannot be launched in Wx #889 was solved.
  • The entries trait on the FileEditor factory is only supported by wx. If set on wx, the simple FileEditor will have a combo box instead of a text widget. The simple FileEditor on wx is effectively two editors in one. That branch is not tested here.
  • On the demo, if one sets an existing file path for the simple FileEditor, the custom FileEditor will normalize the file path. So if one sets a relative path to the simple FileEditor, they will get an absolute path back unless the custom FileEditor is removed. That's because in the demo, multiple editors are tied to the same trait.
  • The test for the FileEditor demo fails on wx due to Wx Event Handler clean-up issues #752. The presence of the custom FileEditor caused it. If the custom FileEditor was removed, the test will pass for wx.

@kitchoi
Copy link
Contributor Author

kitchoi commented Apr 6, 2021

Note: One can apply more or less the same change for the simple DirectoryEditor (not covered here).

Copy link
Contributor

@aaronayres35 aaronayres35 left a comment

Choose a reason for hiding this comment

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

LGTM

Just a couple comments about skipping tests on wx


class TestFileEditorDemo(unittest.TestCase):

def test_run_demo(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

You say in the PR description: "The test for the FileEditor demo fails on wx due to #752". Should this have an @requires_toolkit(['ToolkitName.qt']) in that case?
/ a comment regarding the failure being linked to having a custom editor included in the demo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Yes I should add the decorator.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I should add the decorator.

On second thought, this is a demo test so I'd want to avoid importing anything from traitsui.tests._tools.
Will go with a comment instead :/

traitsui/tests/editors/test_file_editor.py Show resolved Hide resolved
@kitchoi kitchoi merged commit 60c78f9 into master Apr 6, 2021
@kitchoi kitchoi deleted the tst-support-file-editor branch April 6, 2021 18:25
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.

None yet

2 participants