Document EnableTestAssistant#800
Merged
Merged
Conversation
aaronayres35
changed the title
rahulporuri
approved these changes
Apr 29, 2021
Contributor
rahulporuri
left a comment
rahulporuri
left a comment
There was a problem hiding this comment.
Mostly LGTM as it is.
I too was looking through enable for uses of EnableTestAssistant and the usage i find most compelling is interestingly enough test_test_assistant - you could rewrite those tests to demonstrate EnableTestAssistant instead of testing it IMO.
rahulporuri
approved these changes
Jun 1, 2021
Comment on lines
+26
to
+29
| The following is a Dummy TestCase to showcase some basics of the | ||
| :class:`~.EnableTestAssistant` functionality. It is not testing things | ||
| users would likely want to test, but it showcases the capabilities / usefulness | ||
| of the test assistant. |
Contributor
There was a problem hiding this comment.
I think this paragraph is too verbose and unnecessary. You should be able to replace all of this with just "here is an example" and be done with it
| import unittest | ||
| from unittest import mock | ||
|
|
||
| from enable.component import Component |
Contributor
There was a problem hiding this comment.
can we use enable.api here?
| from unittest import mock | ||
|
|
||
| from enable.component import Component | ||
| from enable.testing import EnableTestAssistant, _MockWindow |
Contributor
There was a problem hiding this comment.
Maybe don't import and use _MockWindow. Let's not unnecessarily advertise private objects to end users in the docs.
Co-authored-by: Poruri Sai Rahul <rporuri@enthought.com>
aaronayres35
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #774
This PR adds a brief section to the documentation describing the
EnableTestAssistant. This is effectively meant as a starting point, and we will likely want to expand this documentation going forward. In any case, just mentioning its existence in the docs is a significant improvement over it being missing entirely.A next step (in a future PR) would be to add an example test using the test assistant to showcase how to use it and some of the useful features. I was going to simply link to an existing test, but none of them felt really suited to be a good example for the documentation IMO. I think it will be useful to write a test specifically for the docs, possibly writing a test for on of the existing examples.