I have this code in a controller of an MVC implementation: public void execute()
ID: 639399 • Letter: I
Question
I have this code in a controller of an MVC implementation:
public void execute() {
try {
String path = userSelectsFile();
if ( path == null ) return; //Just returns if the user press "Cancel"
//Load the data
Collection<Info> infos = Infos.fromJSON(getInputReader(path));
//Show the data in the interface
new ShowTransformationsInTree(win(), infos, win().getTree()).execute();
} catch (Exception e) {
new ComplainAction(win(), "Unable to...blah, blah", e).execute();
}
}
To test this, I have to mock the user interface to show file dialog and make getInputReader method an abstract factory method.
Test code:
/**
* Mocks the LoadTransformationsAction so the factory method getReader returns a ByteInputStream
*/
public class MockLoadTransformationsAction extends LoadTransformationsAction {
//CONSTRUCTOR REMOVED FOR CLARITY.....
@Override
protected InputStreamReader getInputReader(String path) {
assertEquals(FAKE_ABSOLUTE_PATH, path);
return new InputStreamReader(
new ByteArrayInputStream(TestHelpers.createDataBytes());
}
}
/**
* Test the proper loading of the data.
*/
@Test
public void testLoadData(@Mocked final FileChooser anyChooser) {
new Expectations() {{
//Mocks the IntelliJ idea API
FileChooser.chooseFile(null, null, null); result = new MyFakeVirtualFile(); //Returns a fixed path
}};
//Loads the data from the JSON file
MainToolWin m = new FakeMainToolWin();
LoadTransformationsAction action = new MockLoadTransformationsAction(
m, new FakeProject(), new MockInputProgram());
action.execute();
//Test that the transformations are in the UI Tree
DefaultTreeModel model = (DefaultTreeModel)m.getTree().getModel();
assertEquals(2, model.getChildCount(model.getRoot()));
assertEquals(3, model.getChildCount(model.getChild(model.getRoot(), 0)));
}
Questions:
Am I doing too much to test too little?
As you can see there are mock and fake objects all the way. Can so much mocking can make my test unreliable?
Is there a better design?
Explanation / Answer
Yes, too many or too complicated mock objects are a bad thing. Yes, there is a better design, and I have been toying with the idea in my mind for years. Unfortunately, I cannot fully explain it here because the paper that I am writing about it is not complete yet. But, in coarse terms, here is what is happening:
You are mixing business logic with GUI logic, and GUIs are generally very hard to test. You are already making some effort to separate the two, as evidenced by your use of a (presumably GUI-agnostic) tree model data structure, but you are not going all the way with this idea, since you are only making this tree available to your business logic via a "main tool window" which is a GUI concept.
The notion of presenting the user with the opportunity to select a file, which they may cancel, is part of your business logic. The fact that you use a GUI to accomplish this is irrelevant: you could achieve the same thing with a command-line interface, asking the user to type the filename, and allowing for an empty filename to mean 'cancel'.
So, if you completely isolated all your business logic from the GUI-related code, then your business logic would be fully testable with no mock objects. (Not just a few simple mock objects, but actually zero mock objects.)
Generally, the presence of a mock object in testing means that the design is not perfect. Quite often an imperfect design which requires mock objects to test is a pragmatic compromise, but you seem to be asking what's best, so, that's what's best.
EDIT:
After discussing it a bit, it turns out that your main concern is the accessing of the data in the file. Here is what I have to say about this:
(And yes, now it seems like this question is more suitable for codereview.stackexchange.com.)
Currently, your business logic needs some "Infos" to work with, but it contains specific hard-coded knowledge of the fact that these "Infos" are coming from JSON; furthermore, it contains specific hard-coded knowledge of the fact that this JSON comes from a file. (By virtue of the 'path' variable.) I think that's a bit problematic. If your business logic needs some "Infos" to work with, it should not be concerned with how they can be obtained. It should just be given those infos from the outside. So, instead of having to mock the creation of a stream, your test code would just be constructing test instances of the "Infos" class to test your business logic with.
Then, you would need a separate set of tests to make sure that your Infos.fromJSON() method works correctly, and these tests would belong to the "Infos" class, not to your controller class. (Maybe you already have them.)
Essentially, what you would be doing with such an approach is that you would be isolating your business logic from the file system, and from text-file-format-choice (JSON) considerations, which is just as crucial as isolating it from the GUI.
If you don't want to do all this, then you have to create precisely that mock object which you have created; this should not come as a surprise, and it is actually not an awful lot of work compared to what you need to accomplish.