pyface
pyface copied to clipboard
[Do Not Merge] New GUI Test Tools
This is a proposed new version of the GUI test assistant that is more stand-alone and backend-independent. The biggest changes are to rely more on pyface.timer
(which in turn implies some changes to that API) and to push a lot of the backend dependent behaviour onto the GUI
class (in hopefully a sensible way).
The API is tested out on the Window
class.
Tests need to be written.
Codecov Report
Merging #447 into master will increase coverage by
0.36%
. The diff coverage is68.11%
.
@@ Coverage Diff @@
## master #447 +/- ##
==========================================
+ Coverage 36.63% 36.99% +0.36%
==========================================
Files 481 487 +6
Lines 26424 26722 +298
Branches 3917 3953 +36
==========================================
+ Hits 9680 9886 +206
- Misses 16321 16394 +73
- Partials 423 442 +19
Impacted Files | Coverage Δ | |
---|---|---|
pyface/testing/util.py | 0% <0%> (ø) |
|
pyface/i_gui.py | 80.55% <100%> (+2.43%) |
:arrow_up: |
pyface/qt/__init__.py | 46.93% <100%> (+2.25%) |
:arrow_up: |
pyface/i_window.py | 82.35% <100%> (-15.65%) |
:arrow_down: |
pyface/ui/qt4/confirmation_dialog.py | 100% <100%> (ø) |
:arrow_up: |
pyface/toolkit_utils.py | 100% <100%> (ø) |
|
pyface/ui/wx/window.py | 90.24% <20%> (+10.49%) |
:arrow_up: |
pyface/ui/qt4/window.py | 72.8% <57.14%> (-1.07%) |
:arrow_down: |
pyface/ui/wx/timer/timer.py | 87.5% <62.5%> (+24.34%) |
:arrow_up: |
pyface/ui/wx/gui.py | 66.66% <66.66%> (+18.39%) |
:arrow_up: |
... and 24 more |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 42bdac6...6d23d86. Read the comment docs.
@mdickinson could you please have a look at the proposed API (particularly for GUITestCase
and EventLoopHelper
). The main changes are:
- it is a subclass of
TestCase
rather than a mixin (I could be convinced either way) - most toolkit-dependent event loop stuff is pushed to the GUI class
- it uses
pyface.timer
s rather than working at the toolkit level for timers - no misleading context managers
There are some additional changes that I'd be interested in making (such as renaming event_loop_...
methods to just loop_...
), and there is currently no replacement for the ModalDialogTester
(and probably won't be in this PR). Also there are errors on Windows that need to be wrangled, but I'm not going to invest the effort before getting some feedback that this is a better solution than our current one.
I left a few comments on the code, mostly (but not entirely) at nitpick level.
- it is a subclass of
TestCase
rather than a mixin (I could be convinced either way)
Me too; there are tradeoffs either way, but overall I think I'm happier with the subclass approach. If it makes GuiTestCase
usable with the usual super
pattern in a multiple inheritance situation, so much the better.
Overall the API looks good to me, but I don't think I understand what EventLoopHelper.event_loop_with_timeout
and GuiTestCase.event_loop_with_timeout
are for. From just the name and the signature, I'd assume that these are for cases where you want to say "I've set things up so that the event loop will only run for a short finite time. Now either go and run the event loop until it stops naturally, or if it hasn't stopped after <timeout>
seconds raise a timeout exception." But that doesn't seem to be what the code is doing.
Will there be unit tests for the EventLoopHelper
and the GUITestCase
?
(Edited to change EventLoop
to EventLoopHelper
and fix other minor typos.)
N.B. I didn't really look at the parts of this PR that aren't directly related to the GuiTestCase
.
Yes, event_loop_with_timeout
was problematic, and the docstring got out of sync with the final version. The original intent was as in the current GuiTestAssistant
: run the actual event loop at least repeat
times with a timeout if it takes too long to repeat that many times.
I then went to the start which the current docstring suggests which was "run for timeout
seconds" no matter what due to issues getting it to work on WxPython; but it turned out that the problem was in fact that under WxPython you can't start the event loop until there is at least one window in existence; so I reverted to the original intent and used a different approach for the use case which was causing problems.
But given that we are using it in just one place, it maybe needs a re-think or removal (even the old version is not used much). After all, without the repeat condition, it is just event_loop_with_condition
where the condition always returns False
.