Grids GUI test #231

scottyak · 2021-01-09T00:28:40Z

This is just a skeleton to demonstrate how the UI test would work. More cleanups to follow if folks agree on the general approach.

CLAassistant · 2021-01-09T00:28:46Z

All committers have signed the CLA.

youngmit · 2021-01-09T18:01:51Z

Looking good so far. Thanks for turning the crank on Travis; truly a thankless job :(

scottyak · 2021-01-10T19:06:46Z

Thanks!

Here's the progress I've made so far:

Found a way to include wxpython as a dependency in one of the Travis environments. Building the wheel from source would have taken ~30mins, but thankfully there is a prebuilt wheel for xenial which we can just download, so wxpython only takes ~1 min to install.
Headless UI test works in my local Ubuntu environment -- runs in seconds, and the test passes.
Headless UI test runs in the Travis environment without crashing, but the test fails (even though the code is the same as the one which passed in my local environment.) "Works on my machine", indeed.

So my current problem is getting the test to pass in the Travis environment. This is not easy to do, because there isn't a display that allows me to see what the test is doing to the UI.

I have two approaches in mind, both of which I cannot do by myself:

Video capture the virtual display when the test is running, and store the video in some place I can access.
- How to record the screen:
  - Use PIL, opencv, and maybe FFMPEG to implement screen recording functionality in the GridGuiTestCase class, or
  - Install byzanz, and use before_script and after_script to start and stop the screen recording.
- How to upload the video:
  - There is some documentation here for how to upload artifacts from Travis to S3, but it would require someone from Terrapower to configure S3 access keys, and grant me access to some of the artifacts.
Requesting for remote SSH debugging access, and set up X11 forwarding to see what's going on. Unfortunately I am unable to find much information about how exactly to do this.

I am currently experimenting with the screen recording approach, and testing locally. Will let you know how this goes!

youngmit · 2021-01-12T19:25:27Z

Sounds like you've made lots of progress. I think having tests that can run in any environment is enough of an improvement that it'd a shame to hold this up getting the tests to run under Travis. Maybe for now we can just disable the test under Travis, merge this PR and go from there?

Looks like a good way to disable tests under travis is to set an environment variable in the travis config and look for it in a skipif clause in the test (https://stackoverflow.com/questions/57333637/how-i-can-disable-some-tests-in-travis-ci-for-python)

youngmit · 2021-01-12T19:30:18Z

So I think path forward would be to revert most of the Travis changes, but probably retain them on another branch somewhere; I think we will want to circle back to this, and it looks like you learned a lot. Let's add an env var to disable problematic tests. We can close #133 and open a new issue for getting the tests to work under Travis.

scottyak · 2021-01-12T19:36:35Z

Sounds good to me!

youngmit

It looks like there is still stuff left over from trying to figure out what Travis is up to (sorry, i know its really frustrating to try to figure out what CI systems are up to, especially when pytest is so eager to hide all output from you 😡). So let me know when you think this is good to go and I'll do a full review.

For now, I want to get ahead of some content things earlier rather than later. Mainly, I've got to look into whether we are okay with the little LGPL snippet. I think for that to be there, we would need to have a license notice displayed somewhat prominently that there is LGPL code in ARMI. Not sure if we want to go that route (at least not to accommodate such a little snippet). Also, technically according to the CLA, anything that isn't original work needs to be submitted separately (i think so that we can consider it's copyright holder and license terms separately and make sure we incorporate everything properly). So! if at all possible, it would be nice if the _cleanup() function could be adjusted/adapted enough that we can make a fair-use argument that it is original work. Think that's possible?

scottyak · 2021-01-12T20:22:40Z

Yep, happy to adjust the _cleanup() function. I agree that it's best to avoid having LGPL code in the codebase.

Will update once the PR is in a more ready state for a proper review!

armi/utils/gridEditor.py

scottyak · 2021-01-16T07:41:14Z

Ready for review, PTAL! Thanks!

scottyak · 2021-01-16T20:03:42Z

Oops, not quite ready yet! Will update again when ready.

scottyak · 2021-01-16T22:05:52Z

This is ready now!

scottyak · 2021-01-16T22:46:33Z

(Not sure why test coverage dropped.)

ntouran · 2021-01-17T00:37:13Z

Thanks for all this work. I'm not sure why Travis test results are so detached from this PR (failing unit tests should have come with a prominent check failure, so that's probably a new open issue) but checking into it, it looks like the unguarded import wx in test_gridGui.py is causing an import error in the normal unit test suite. Then most of the other tests didn't run so coverage is artificially low.

Maybe shove the import wx into the environment conditional as well?

scottyak · 2021-01-17T02:12:02Z

Now there appears to be an issue with "pip install black" :(

Will update when I figure out why.

Update: There was a transient issue with the black GitHub action, it should be fine now.

scottyak · 2021-01-17T05:58:06Z

It's ready now!

ntouran · 2021-01-17T18:55:08Z

Whew, nice.

Though our test suite only runs up to Python 3.7, I ran this on my Python 3.8 Linux box and got a failure. Have you seen anything like this? Could indicate that there's some important change in an underlying lib that we don't have expressed in requirements.txt, etc.?

$ pytest --no-xvfb --capture=tee-sys test_gridGui.py
Initializing generic ARMI Framework application

                       ---------------------------------------------------
                      |             _      ____     __  __    ___         |
                      |            / \    |  _ \   |  \/  |  |_ _|        |
                      |           / _ \   | |_) |  | |\/| |   | |         |
                      |          / ___ \  |  _ <   | |  | |   | |         |
                      |         /_/   \_\ |_| \_\  |_|  |_|  |___|        |
                      |         Advanced  Reactor  Modeling Interface     |
                       ---------------------------------------------------
                                      Version 0.1.6     

=============================================================================================== test session starts ===============================================================================================
platform linux -- Python 3.8.6, pytest-5.4.3, py-1.8.1, pluggy-0.13.1
rootdir: /home/nick/code/armi, inifile: pytest.ini
plugins: xvfb-2.0.0, cov-2.9.0, html-2.1.1, xdist-1.32.0, metadata-1.9.0, forked-1.1.3
collected 1 item                                                                                                                                                                                                  

test_gridGui.py 10:44:09 AM: Debug: ClientToScreen cannot work when toplevel window is not shown
110:44:09 AM: Debug: ClientToScreen cannot work when toplevel window is not shown
10:44:09 AM: Debug: ClientToScreen cannot work when toplevel window is not shown
10:44:09 AM: Debug: ClientToScreen cannot work when toplevel window is not shown
F                                                                                                                                                                                           [100%]

==================================================================================================== FAILURES =====================================================================================================
______________________________________________________________________________________________ Test.test_setNumRings ______________________________________________________________________________________________
Traceback (most recent call last):
  File "/home/nick/code/armi/armi/utils/tests/test_gridGui.py", line 159, in test_setNumRings
    self.assertEqual(1, len(gridCellIndices))
  File "/usr/lib/python3.8/unittest/case.py", line 912, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/usr/lib/python3.8/unittest/case.py", line 905, in _baseAssertEqual
    raise self.failureException(msg)
AssertionError: 1 != 196
================================================================================================ warnings summary =================================================================================================
armi/utils/tests/test_gridGui.py::Test::test_setNumRings
armi/utils/tests/test_gridGui.py::Test::test_setNumRings
armi/utils/tests/test_gridGui.py::Test::test_setNumRings
armi/utils/tests/test_gridGui.py::Test::test_setNumRings
  /home/nick/code/armi/armi/utils/gridEditor.py:1245: DeprecationWarning: an integer is required (got type numpy.float64).  Implicit conversion to integers using __int__ is deprecated, and may be removed in a future version of Python.
    return wx.Rect(wx.Point(*topLeft[:2]), wx.Point(*bottomRight[:2]))

armi/utils/tests/test_gridGui.py: 784 tests with warnings
  /home/nick/code/armi/armi/utils/gridEditor.py:163: DeprecationWarning: an integer is required (got type numpy.float64).  Implicit conversion to integers using __int__ is deprecated, and may be removed in a future version of Python.
    return wx.Rect(wx.Point(xmin, ymin), wx.Point(xmax, ymax))

armi/utils/tests/test_gridGui.py: 1568 tests with warnings
  /home/nick/code/armi/armi/utils/gridEditor.py:261: DeprecationWarning: an integer is required (got type numpy.float64).  Implicit conversion to integers using __int__ is deprecated, and may be removed in a future version of Python.
    dc.DrawPolygon(poly)

-- Docs: https://docs.pytest.org/en/latest/warnings.html
============================================================================================ slowest 30 test durations ============================================================================================
1.43s call     armi/utils/tests/test_gridGui.py::Test::test_setNumRings

(0.00 durations hidden.  Use -vv to show these durations.)
============================================================================================= short test summary info =============================================================================================
FAILED test_gridGui.py::Test::test_setNumRings - AssertionError: 1 != 196

scottyak · 2021-01-17T19:34:06Z

That's unfortunate -- looks like it only works on my machine, eh?

This is what I see:

$ pytest -vv --no-xvfb --capture=tee-sys armi/utils/tests/test_gridGui.py 
Initializing generic ARMI Framework application

                       ---------------------------------------------------
                      |             _      ____     __  __    ___         |
                      |            / \    |  _ \   |  \/  |  |_ _|        |
                      |           / _ \   | |_) |  | |\/| |   | |         |
                      |          / ___ \  |  _ <   | |  | |   | |         |
                      |         /_/   \_\ |_| \_\  |_|  |_|  |___|        |
                      |         Advanced  Reactor  Modeling Interface     |
                       ---------------------------------------------------
                                      Version 0.1.6     

============================================================================================= test session starts =============================================================================================
platform linux -- Python 3.8.5, pytest-6.2.1, py-1.10.0, pluggy-0.13.1 -- /home/scott/armi-venv/bin/python3.8
cachedir: .pytest_cache
metadata: {'Python': '3.8.5', 'Platform': 'Linux-5.8.0-38-generic-x86_64-with-glibc2.29', 'Packages': {'pytest': '6.2.1', 'py': '1.10.0', 'pluggy': '0.13.1'}, 'Plugins': {'xvfb': '2.0.0', 'metadata': '1.11.0'}}
rootdir: /home/scott/armi-venv/armi, configfile: pytest.ini
plugins: xvfb-2.0.0, metadata-1.11.0
collected 1 item                                                                                                                                                                                              

armi/utils/tests/test_gridGui.py::Test::test_setNumRings PASSED                                                                                                                                         [100%]

============================================================================================== warnings summary ===============================================================================================
armi/utils/tests/test_gridGui.py: 12 warnings
  /home/scott/armi-venv/armi/armi/utils/gridEditor.py:1245: DeprecationWarning: an integer is required (got type numpy.float64).  Implicit conversion to integers using __int__ is deprecated, and may be removed in a future version of Python.
    return wx.Rect(wx.Point(*topLeft[:2]), wx.Point(*bottomRight[:2]))

armi/utils/tests/test_gridGui.py: 1572 warnings
  /home/scott/armi-venv/armi/armi/utils/gridEditor.py:163: DeprecationWarning: an integer is required (got type numpy.float64).  Implicit conversion to integers using __int__ is deprecated, and may be removed in a future version of Python.
    return wx.Rect(wx.Point(xmin, ymin), wx.Point(xmax, ymax))

armi/utils/tests/test_gridGui.py: 3144 warnings
  /home/scott/armi-venv/armi/armi/utils/gridEditor.py:261: DeprecationWarning: an integer is required (got type numpy.float64).  Implicit conversion to integers using __int__ is deprecated, and may be removed in a future version of Python.
    dc.DrawPolygon(poly)

-- Docs: https://docs.pytest.org/en/stable/warnings.html
============================================================================================ slowest 30 durations =============================================================================================
1.21s call     armi/utils/tests/test_gridGui.py::Test::test_setNumRings
0.00s setup    armi/utils/tests/test_gridGui.py::Test::test_setNumRings
0.00s teardown armi/utils/tests/test_gridGui.py::Test::test_setNumRings
====================================================================================== 1 passed, 4728 warnings in 1.32s =======================================================================================

I'm wondering if it could be some difference between our display settings. Would you mind running the test again without disabling xvfb and pasting the output here? Like this:

$ pytest -vv --capture=tee-sys armi/utils/tests/test_gridGui.py

ntouran · 2021-01-17T23:13:20Z

The previous time I was running through a ssh with X forwarding so I thought that might be an issue and ran again locally with this command. Similarly, I get:

(py38) nick@lemur:~/code/armi$ pytest -vv --capture=tee-sys armi/utils/tests/test_gridGui.py
Initializing generic ARMI Framework application

                       ---------------------------------------------------
                      |             _      ____     __  __    ___         |
                      |            / \    |  _ \   |  \/  |  |_ _|        |
                      |           / _ \   | |_) |  | |\/| |   | |         |
                      |          / ___ \  |  _ <   | |  | |   | |         |
                      |         /_/   \_\ |_| \_\  |_|  |_|  |___|        |
                      |         Advanced  Reactor  Modeling Interface     |
                       ---------------------------------------------------
                                      Version 0.1.6     

=============================================================================================== test session starts ===============================================================================================
platform linux -- Python 3.8.6, pytest-6.2.1, py-1.10.0, pluggy-0.13.1 -- /opt/venvs/py38/bin/python3
cachedir: .pytest_cache
metadata: {'Python': '3.8.6', 'Platform': 'Linux-5.8.0-7630-generic-x86_64-with-glibc2.32', 'Packages': {'pytest': '6.2.1', 'py': '1.10.0', 'pluggy': '0.13.1'}, 'Plugins': {'html': '3.1.1', 'xvfb': '2.0.0', 'xdist': '2.2.0', 'metadata': '1.11.0', 'cov': '2.10.1', 'forked': '1.3.0'}}
rootdir: /home/nick/code/armi, configfile: pytest.ini
plugins: html-3.1.1, xvfb-2.0.0, xdist-2.2.0, metadata-1.11.0, cov-2.10.1, forked-1.3.0
collected 1 item                                                                                                                                                                                                  

armi/utils/tests/test_gridGui.py::Test::test_setNumRings FAILED                                                                                                                                             [100%]

==================================================================================================== FAILURES =====================================================================================================
______________________________________________________________________________________________ Test.test_setNumRings ______________________________________________________________________________________________
Traceback (most recent call last):
  File "/home/nick/code/armi/armi/utils/tests/test_gridGui.py", line 159, in test_setNumRings
    self.assertEqual(1, len(gridCellIndices))
  File "/usr/lib/python3.8/unittest/case.py", line 912, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/usr/lib/python3.8/unittest/case.py", line 905, in _baseAssertEqual
    raise self.failureException(msg)
AssertionError: 1 != 196
================================================================================================ warnings summary =================================================================================================
armi/utils/tests/test_gridGui.py: 12 warnings
  /home/nick/code/armi/armi/utils/gridEditor.py:1245: DeprecationWarning: an integer is required (got type numpy.float64).  Implicit conversion to integers using __int__ is deprecated, and may be removed in a future version of Python.
    return wx.Rect(wx.Point(*topLeft[:2]), wx.Point(*bottomRight[:2]))

armi/utils/tests/test_gridGui.py: 1572 warnings
  /home/nick/code/armi/armi/utils/gridEditor.py:163: DeprecationWarning: an integer is required (got type numpy.float64).  Implicit conversion to integers using __int__ is deprecated, and may be removed in a future version of Python.
    return wx.Rect(wx.Point(xmin, ymin), wx.Point(xmax, ymax))

armi/utils/tests/test_gridGui.py: 3144 warnings
  /home/nick/code/armi/armi/utils/gridEditor.py:261: DeprecationWarning: an integer is required (got type numpy.float64).  Implicit conversion to integers using __int__ is deprecated, and may be removed in a future version of Python.
    dc.DrawPolygon(poly)

-- Docs: https://docs.pytest.org/en/stable/warnings.html
============================================================================================== slowest 30 durations ===============================================================================================
1.07s call     armi/utils/tests/test_gridGui.py::Test::test_setNumRings
0.00s setup    armi/utils/tests/test_gridGui.py::Test::test_setNumRings
0.00s teardown armi/utils/tests/test_gridGui.py::Test::test_setNumRings
============================================================================================= short test summary info =============================================================================================
FAILED armi/utils/tests/test_gridGui.py::Test::test_setNumRings - AssertionError: 1 != 196

youngmit · 2021-01-18T18:29:09Z

@scottyak I am going to try to get these tests working locally myself, and once successful, I think we should merge this. You've done a lot of great work on this, and we can always circle back to this long-tail stuff later. Having any tests at all is going to be a huge get, even if it's not running under CI.

youngmit · 2021-01-18T18:31:30Z

If you don't mind me asking, what sort of environment are you running under locally? Distro, python version, etc? If i get stuck, a pip freeze output might be useful as well.

youngmit

I was able to make this work on my end! I had to make sure that i was running under a pure X11 environment. On my system, the default Gnome DE runs under Wayland. If you think you have this running under Wayland, let me know! Since this might go over some folks' heads, I'm thinking that we may want to make the tests opt-in. I played around with a @pytest.mark.skipif decorator, which seemed to do the trick. I also had to make the tick time a little longer for the assertion to reliably pass; i think it was asserting before the app had a chance to fully respond to the event.

So i think that skipif and a longer tick, and this is good to merge. Thanks so much! I know fighting with Travis is a bit of a slog :(

armi/utils/tests/test_gridGui.py

tox.ini

.coveragerc

armi/utils/tests/test_gridGui.py

scottyak · 2021-01-19T00:33:34Z

If you don't mind me asking, what sort of environment are you running under locally? Distro, python version, etc? If i get stuck, a pip freeze output might be useful as well.

~/armi-venv/armi$ uname -a
Linux scott-thinkpad 5.8.0-38-generic #43~20.04.1-Ubuntu SMP Tue Jan 12 16:39:47 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
~/armi-venv/armi$ python -V
Python 3.8.5
~/armi-venv/armi$ pip freeze
apipkg==1.5
appdirs==1.4.4
-e [email protected]:scottyak/armi.git@17664097b69774d5adefee0259d8500b1cce97e5#egg=armi
attrs==20.3.0
black==20.8b1
click==7.1.2
configparser==5.0.1
coverage==5.3.1
cycler==0.10.0
data-science-types==0.2.22
EasyProcess==0.3
execnet==1.7.1
future==0.18.2
h5py==2.10.0
iniconfig==1.1.1
kiwisolver==1.3.1
matplotlib==3.3.3
mypy==0.790
mypy-extensions==0.4.3
numpy==1.19.3
opencv-python==4.5.1.48
ordered-set==4.0.2
packaging==20.8
pathspec==0.8.1
Pillow==8.0.1
pluggy==0.13.1
psutil==5.8.0
py==1.10.0
pyevtk==1.1.1
Pympler==0.9
pyparsing==2.4.7
pytest==6.2.1
pytest-metadata==1.11.0
pytest-xvfb==2.0.0
python-config==0.1.2
python-dateutil==2.8.1
PyVirtualDisplay==2.0
PyYAML==5.3.1
regex==2020.11.13
ruamel.yaml==0.16.12
ruamel.yaml.clib==0.2.2
scipy==1.6.0
screeninfo==0.6.7
six==1.15.0
tabulate==0.8.7
toml==0.10.2
typed-ast==1.4.2
typing-extensions==3.7.4.3
voluptuous==0.12.1
wxPython==4.1.1
xlrd==2.0.1
yamlize==0.7.0
~/armi-venv/armi$ loginctl list-sessions --no-legend | \
    cut --delimiter=' ' --field=1 | \
    xargs loginctl show-session --property=Type
Type=x11

This commit adds the `GuiTestCase` class, as well as an example UI test. At the start of each test, a new instance of the `GridBlueprintControl` class is created and displayed, and the test author can define the sequence of actions to be simulated using `wx.UIActionSimulator`, and check that the properties of the `GridBlueprintControl` instance are as expected. The test can also be run in headless mode with `pytest-xvfb`. Unfortunately, this test only works in certain environments (we've only gotten it to work locally on Linux with pure X11 environments), so we are disabling the tests for now.

youngmit

Alrighty, this looks good to go! Thanks for doing this :) Really good to have some coverage on this GUI stuff.

scottyak · 2021-01-19T15:32:20Z

And thank you all for being so responsive to comments!

scottyak force-pushed the gui_test branch from 61aa86f to 34be535 Compare January 9, 2021 03:14

youngmit reviewed Jan 12, 2021

View reviewed changes

youngmit reviewed Jan 15, 2021

View reviewed changes

armi/utils/gridEditor.py Outdated Show resolved Hide resolved

scottyak force-pushed the gui_test branch from 62e38ea to fc51a83 Compare January 16, 2021 07:36

youngmit suggested changes Jan 18, 2021

View reviewed changes

armi/utils/tests/test_gridGui.py Show resolved Hide resolved

armi/utils/tests/test_gridGui.py Show resolved Hide resolved

tox.ini Show resolved Hide resolved

.coveragerc Show resolved Hide resolved

armi/utils/tests/test_gridGui.py Outdated Show resolved Hide resolved

scottyak force-pushed the gui_test branch from ae12584 to 51ea251 Compare January 19, 2021 02:26

scottyak requested a review from youngmit January 19, 2021 02:37

youngmit approved these changes Jan 19, 2021

View reviewed changes

youngmit merged commit 79b99d6 into terrapower:master Jan 19, 2021

scottyak deleted the gui_test branch January 23, 2021 18:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Grids GUI test #231

Grids GUI test #231

scottyak commented Jan 9, 2021

CLAassistant commented Jan 9, 2021 •

edited

Loading

youngmit commented Jan 9, 2021

scottyak commented Jan 10, 2021

youngmit commented Jan 12, 2021

youngmit commented Jan 12, 2021

scottyak commented Jan 12, 2021

youngmit left a comment

scottyak commented Jan 12, 2021

scottyak commented Jan 16, 2021

scottyak commented Jan 16, 2021

scottyak commented Jan 16, 2021

scottyak commented Jan 16, 2021

ntouran commented Jan 17, 2021 •

edited

Loading

scottyak commented Jan 17, 2021 •

edited

Loading

scottyak commented Jan 17, 2021

ntouran commented Jan 17, 2021

scottyak commented Jan 17, 2021

ntouran commented Jan 17, 2021

youngmit commented Jan 18, 2021

youngmit commented Jan 18, 2021

youngmit left a comment

scottyak commented Jan 19, 2021 •

edited

Loading

youngmit left a comment

scottyak commented Jan 19, 2021

Grids GUI test #231

Grids GUI test #231

Conversation

scottyak commented Jan 9, 2021

CLAassistant commented Jan 9, 2021 • edited Loading

youngmit commented Jan 9, 2021

scottyak commented Jan 10, 2021

youngmit commented Jan 12, 2021

youngmit commented Jan 12, 2021

scottyak commented Jan 12, 2021

youngmit left a comment

Choose a reason for hiding this comment

scottyak commented Jan 12, 2021

scottyak commented Jan 16, 2021

scottyak commented Jan 16, 2021

scottyak commented Jan 16, 2021

scottyak commented Jan 16, 2021

ntouran commented Jan 17, 2021 • edited Loading

scottyak commented Jan 17, 2021 • edited Loading

scottyak commented Jan 17, 2021

ntouran commented Jan 17, 2021

scottyak commented Jan 17, 2021

ntouran commented Jan 17, 2021

youngmit commented Jan 18, 2021

youngmit commented Jan 18, 2021

youngmit left a comment

Choose a reason for hiding this comment

scottyak commented Jan 19, 2021 • edited Loading

youngmit left a comment

Choose a reason for hiding this comment

scottyak commented Jan 19, 2021

CLAassistant commented Jan 9, 2021 •

edited

Loading

ntouran commented Jan 17, 2021 •

edited

Loading

scottyak commented Jan 17, 2021 •

edited

Loading

scottyak commented Jan 19, 2021 •

edited

Loading