Hi,
recently I noticed several people having issues with test failures in GeoServer
that cannot be reproduces on the build server, or, for what it’s worth, on some
devs machines.
Now, barring the usual windows/linux differences, some failures are caused by:
- our base test classes not cleaning up the catalog/config after each test (to save execution time)
- having some tests that change the configuration forgetting to add the corresponding
@Before/@After methods to revert the change - the test method execution order changing based on the platform and JDK version
My first attempt at addressing this has been to try and create a fully randomize test
execution environment, where both test class and test method execution order is random:
https://github.com/aaime/geoserver/commit/982df40103ea21ea45e35cd41c23a6bb4f614b90
The idea was to continue working on it so that the runner stores the order of the last
failed run on the file system, and reproduces it until the test gets fixed.
Now, this path would lead us eventually to full test correctness vs execution order.
The downside is that making commits would be harder, a test that worked on your machine
for three times in a row might not work on the build server… and this is painful.
Another alternative is to go for fully fixed order (both predictable execution order for test classes,
and methods within the classes). Which is something that we get, too.
All we need is to upgrade to JUnit to 4.11. This version has a FixMethodOrder annotation, which
can be used to force an alphabetical ordering.
Unfortunately, using it is going be painful… since FixMethodOrder is not inherited by subclasses,
the jUnit people have a pull request and they are still discussing it:
https://github.com/junit-team/junit/pull/633
However, there is an upside: the annotation is not really needed.
When no method order is specified, JUnit 4.11 uses the “default” method sorter, which compares
the hash codes of the method names: this gives a weird (if you want), but predictable method
execution order.
Here is a patch for to get predictable execution order (it also forces maven to run test classes
in alphabetical order, for total control over the overall order)
https://github.com/aaime/geoserver/commit/2f4b420bbb5365236df6d74eb91c3a8b93b98888
Now, which one is better?
From a theoretical standpoint the fully randomized test approach is better, as it leads us towards
tests that have no unexpected dependencies on execution order.
However, the cost of that approach is high in terms of development and maintenance, since
the tests can fail unpredictably after the commit.
Given that it’s already hard to get any test at all, especially in pull requests, aggravating the
matter does not seem like the best move.
Being practical, I’d prefer to go for the fully predictable approach instead, and spend the extra
time saved from fixing the tests to fix actual bugs in GeoServer.
Unfortunately… jUnit is not ready yet.
Of course, you might have guessed it already, the two solutions have one thing in common:
either way, the build will fail after applying the patches, showing that we do indeed have tests
with execution order assumptions.
I’ve been trying to fix the tests with the fully fixed order, and it has been nice to find
that I get the same errors exactly in maven and in the IDE, and that the errors are reproducible
in JDK 6 and JDK 7 the same way (well, those that do not depend on some specific library
change in the Java API, that is).
Here is a sample patch that fixes the main module to build under the JUnit 4.11 predictable test
order (the security tests seem like a nightmare to cleanup in @After/@Before, so I just
made them run the full setup every time, improvements welcomed):
https://github.com/aaime/geoserver/commit/0452891ffbbe0d157d54fc5e0ade7dcdbb29c4cf
Opinions?
Cheers
Andrea
–
==
Our support, Your Success! Visit http://opensdi.geo-solutions.it for more information.
Ing. Andrea Aime
@geowolf
Technical Lead
GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549
http://www.geo-solutions.it
http://twitter.com/geosolutions_it