id	summary	reporter	owner	description	type	status	priority	milestone	component	version	resolution	keywords	cc
16010	Tests: use JMockit to enable more extensive test coverage	ris	team	"Here's something I've been working on...

There are a number of places in the tests where our lack of ability to mock out deeply embedded code feels like it has stunted the growth of the test suite. An example of this is our avoidance of anything that creates a `Window` because its constructor will raise a `HeadlessException` when the tests are run in headless mode. This also means that anything inheriting from `JDialog` can't really be tested - and that's a shame, because there are an awful lot of dialog-flows in JOSM and from what I can tell, there aren't really test for any of them. On the other hand, the headless restriction has caused the growth in a number of places around the code of ""alternate flows"" for ""test mode"", which usually enable themselves by detecting `isHeadless`. The presence of these alternate flows makes the regular-mode behaviour of the code a bit harder to understand and it's not always particularly easy to piece together where all of these magic only-for-test-mode switches are and what their combined effect is.

Also, headless mode is not totally synonymous with ""this is a test"". Developers run tests in non-headless mode and there (hypothetically at least?) could be a non-test use for some JOSM code in a headless context. On top of that, a test being run in non-headless mode doesn't automatically mean that no window-related mocking is required - generally though `Window`s are ''available'' we often don't ''create'' them as that can add some unwanted complexity. (I refer to this as `Windowless` mode in my code)

I've had a little bit of experience working with mocking libraries in java and so I decided to dig into this path. There are a few different java mocking libraries, and they all seem to work in slightly different ways. I've personally had most success with `JMockit`'s `MockUp` class, which is a class that intercepts things at a class loader level in java. What it essentially allows the user to do is to temporarily, at runtime, **globally**, switch out the implementation of an arbitrary class's methods. The original implementations are replaced automatically at the end of the JUnit test ends.

In the posted patch set, one of the first things I've done is replace a couple of JOSM's `isHeadless()` switches with mock methods that get switched out during `JOSMTestRules` initialization. The advantage of this is that the mock behaviour it creates will apply in headless and non-headless mode. I take advantage of this by re-adding an old bugbear test of mine which previously failed in non-headless mode.

Then I go on and develop a mechanism for allowing `ExtendedDialog`-driven flows to be tested. The nastiest part of this is where I actually patch out the constructor of `java.awt.Window`. It's considered bad practise to mock out code you ""don't own"", but really, the `isHeadless()` check existing in `Window`'s constructor, I don't see any other possible way a `Window`-derived class could ever be constructed in headless mode. Now this does mean that we shouldn't expect these partially uninitialized `Window`s to behave perfectly correctly, but fortunately, often we don't need them to. Most uses of `ExtendedDialog` only interact with the instance on a very simple level, not actually relying on any of its window-like behaviour.

And then to demo this I add a test exercising the `updatePlugins` flow, overriding the ""your JOSM version is too low"" warnings. And I also fix a bug I noticed in `JOSMTestRules` whereby the effects of switching to a new-mock-home-directory per-test never take hold.

There are a couple of downsides to using a mocking library like JMockit, the main one is the the mocking is not thread-safe and so this ''could'' limit our ability to execute tests in parallel in future. However, from what I can see, we are a **long** way off from that ability right now and there are also alternatives to thread-based parallel tests (process based parallel tests perhaps?).

And now I've written an essay it would seem. Will post the patches (based on r13450), but the changes are easily browsable in the following PR: https://github.com/openstreetmap/josm/pull/24

I may think of more things to say, but I've got to go out now."	enhancement	closed	normal	18.07	Unit tests		fixed	mock jmockit window extendeddialog