fixup processing parameters tests #2541
Conversation
Can't set parameters to null (None or '') if the parameter is not optional
michaelkirk
changed the title
| @@ -16,3 +16,6 @@ brew ln libffi --force | |||
|
|
|||
| mkdir -p /Users/travis/Library/Python/2.7/lib/python/site-packages | |||
| echo 'import site; site.addsitedir("/usr/local/lib/python2.7/site-packages")' >> /Users/travis/Library/Python/2.7/lib/python/site-packages/homebrew.pth | |||
|
|
|||
| # Needed for Processing | |||
| pip install psycopg2 numpy | |||
There was a problem hiding this comment.
Isn't that installed via apt already?
| ADD_SUBDIRECTORY(ui) | ||
| IF (ENABLE_TESTS) |
There was a problem hiding this comment.
So you propose to keep plugin tests in their directories?
It's fine for me, just wondering about the reason.
There was a problem hiding this comment.
Right - my reasoning is kind of abstract, so bear with me...
Firstly, from a software patterns POV, "plugins" should be self contained. One small example of this is that moving plugins in and out of core is simpler if the plugin carries and installs it's own tests.
And more abstractly, I've been thinking of plugins as python modules. Even though they aren't all currently built this way, I think we should move towards that. We're using python and the standard unit of application composition in Python is a module. The tests are part of the module and "/tests or /tests.py" is a standard place to store a modules tests.
Finally, I think core plugins should serve as an example for contributed plugins, and they should resemble contributed plugins as much as possible so that in core plugins we're dogfooding the same plugin tooling and practices that we want contributors to use. To my point - part of the plugin code is (or should be) tests, and I hope we can move more contributors to write tests for their plugin by making it easy to run plugin tests in situ. This plugin doesn't solve that, but I think moving Processing's tests out of its module would be a step in the wrong direction.
- Broke into per class testcase - Each method tries to test only one aspect of behavior - Use unittest assertions for better error output - Removed non-existant serialize functionality from tests - Test BooleanParameter
Previously, in order to set ENV variables for our test, we were writing a separate CMake file per test, and setting the env variables in that script. This is no longer necessary since CMake provides `SET_TESTS_PROPERTIES` for this. Also we add plugins to the test's PYTHONPATH
Allowing us to load fake app in our unit tests (tests explode without an app present)
remove irrelevant parameters, boring tests, reorder tests
michaelkirk
force-pushed
the
feature/fixup-processing-tests
branch
from
d97e08b
to
1715064
Compare
|
Rebased against master. Critically, I forgot to include that I have no way of running these tests on windows. Is there a windows dev that could give me feedback? |
|
I find it quite hard to proof-read CMake changes. I normally find it easier to hit and run. Seems you had to fiddle quite a bit with the environment. Concerning the parameters test and the related discussion, are you checking the status quo or did you totally skip these optional / default behavior checks? |
|
re: CMake changes Agreed they are hard to read. Also, I'm new to CMake. =( I didn't intent to make any changes to the environment except to add Everything else in the CMake code was just a refactor, as the old python test cmake command was based on the incorrect/outdated premise that you could not set ENV variables for the test runner environment. My intention was that the new style had less moving part and was more intelligible. If I've failed there, I can make this change to the old style. re: default/required Parameter behavior I'm testing just the status-quo except here: Happy to change the behavior if there is any conclusive decision here: I think these kind of exhaustive test cases can be really helpful with illustrating this kind of edgecase behavior. e.g. compare ParameterBoolean default handling to ParameterExtent default handling |
|
Closing in favor of #2590 |
Some necessary changes to get processing tests to run in CI and extends the coverage of the processing parameter test.
Resulted in a fixed bug around
Nonehandling in the ParameterFileAlso revamped the ADD_PYTHON_TEST cmake macro, which was writtten on the premise that you couldn't modify the test environment without invoking a separate ctest script. That's not true (maybe it used to be). I think it's more straight forward now, but I'm very new to CMake.