From: Steve F. <sm...@us...> - 2002-08-17 12:59:13
|
Update of /cvsroot/mockobjects/no-stone-unturned/doc/xdocs In directory usw-pr-cvs1:/tmp/cvs-serv30252/doc/xdocs Modified Files: how_mocks_happened.xml patterns.xml htmlbook.css Log Message: More rework on how mocks happened. Index: how_mocks_happened.xml =================================================================== RCS file: /cvsroot/mockobjects/no-stone-unturned/doc/xdocs/how_mocks_happened.xml,v retrieving revision 1.6 retrieving revision 1.7 diff -u -r1.6 -r1.7 --- how_mocks_happened.xml 12 Aug 2002 23:10:20 -0000 1.6 +++ how_mocks_happened.xml 17 Aug 2002 12:59:10 -0000 1.7 @@ -42,7 +42,7 @@ robot.setCurrentPosition(POSITION); robot.goTo(POSITION); - assertEquals("Should be same", POSITION, robot.getPosition()); + assertEquals("Should be same", POSITION, robot.getCurrentPosition()); } }</programlisting> @@ -51,7 +51,7 @@ That's an essential requirement, but it's not enough. We can't be sure that the robot hasn't trundled all the way around the warehouse before returning; we need to know that the robot hasn't moved. How about if the robot were to store the route it takes each time we call - <methodname>goto()</methodname>? We could retrieve the route and make sure it's valid. For example: + <methodname>goTo()</methodname>? We could retrieve the route and make sure it's valid. For example: </para> <programlisting> @@ -60,10 +60,10 @@ Robot robot = new Robot(); robot.setCurrentPosition(POSITION); - robot.goto(POSITION); + robot.goTo(POSITION); <emphasis>assertEquals("Should be empty", 0, robot.getRecentMoveRequests().size());</emphasis> - assertEquals("Should be same", POSITION, robot.getPosition()); + assertEquals("Should be same", POSITION, robot.getCurrentPosition()); }</programlisting> <para> @@ -78,9 +78,9 @@ Robot robot = new Robot(); robot.setCurrentPosition(new Position(0, 0)); - robot.goto(DESTINATION); + robot.goTo(DESTINATION); - assertEquals("Should be destination", DESTINATION, robot.getPosition()); + assertEquals("Should be destination", DESTINATION, robot.getCurrentPosition()); List moves = robot.getRecentMoveRequests(); assertEquals("Should be one move", 1, moves.size()); assertEquals("Should be same move", @@ -106,9 +106,9 @@ Robot robot = new Robot(); robot.setCurrentPosition(new Position(0, 0)); - robot.goto(DESTINATION); + robot.goTo(DESTINATION); - assertEquals("Should be destination", DESTINATION, robot.getPosition()); + assertEquals("Should be destination", DESTINATION, robot.getCurrentPosition()); assertEquals("Should be same moves", makeExpectedLongWayMoves(), robot.getRecentMoves()); }</programlisting> @@ -139,20 +139,20 @@ FAILURES!!!</screen> <para> - We will have to step through the code to find the problem because the assertions have + We will have to re-run the code to find the problem because the assertions have been made <emphasis>after</emphasis> the call has finished. It could be worse, the robot is a relatively - simple example. Some of us tried to do this with financial mathematics and it's very painful when a - calculation fails. A test failure usually meant stepping carefully through the code with an open spreadsheet - nearby to check the values. + simple example. Some of us tried to do this with financial mathematics and it was very painful when a + calculation failed. A test failure usually meant carefully stepping through the code with an open spreadsheet + nearby to check the intermediate values. </para> <para> Second, to test this behaviour at all, we had to add some functionality to the real code, - to hold all the <classname>MoveRequest</classname>s since the last <methodname>goto()</methodname>. + to hold all the <classname>MoveRequest</classname>s since the last <methodname>goTo()</methodname>. We don't have any other immediate need for <methodname>getRecentMovesRequests()</methodname> and, what's worse, we've made an implicit promise to other people who work with the Robot code that we will store routes, thus shrinking (ever so slightly) our room to manouver. We can mark the method as - test infrastructure, but that's messy and does tend to get ignored when in the heat of development. + test infrastructure, but that sort of notation is clumsy and tends to be ignored in the heat of development. </para> <para> @@ -166,7 +166,12 @@ Is there a better way? Can we find a technique that's less intrusive, that doesn't require the robot to hang on to unnecessary values? Can we have more helpful test failures that will fail faster, like we know we're supposed to? What would happen if we really believed in the object maxim - <quote>Tell, don't ask</quote>? + <glossterm linkend="telldontask"><quote>Tell, don't ask</quote></glossterm>? That is, what if we make a point + of avoiding getters? + <footnote><para> + John Nolan is credited with asking this question at the right time at a software architecture + meeting in London + </para></footnote> </para> </section> <!-- what's wrong with this --> @@ -177,18 +182,21 @@ Let's stop and think for a moment. Our robot is actually doing two things when we ask it to move through the warehouse; it has to choose a route from its current position to its destination, <emphasis>and</emphasis> it has to move along the route it has chosen. Those activities are - separate, even if they might happen at the same time. If we break out these responsibilities into two - objects, we can also separate the testing that goes with them. We can test that the route planning - object creates a suitable route and that the Robot moving object follows that route correctly. + distinct, even if they might happen at the same time. If we break out these responsibilities into two + objects, we can also separate the testing that goes with them; we can test that the route planning + object creates a suitable route <emphasis>and</emphasis> that the Robot moving object follows + that route correctly. + </para> + <para> + We'll start with the <quote>Robot Moving Object</quote>, what would be a good name for such a component? + How about <emphasis>Motor</emphasis>? If we intercept the requests that the routing object makes to + the <classname>Motor</classname>, we can track where the Robot thinks its going and not have to retain the + route description for our unit tests. For now, we'll assume that the Routing object is embedded somewhere + in the Robot and we'll extract it later. </para> <para> - We'll start with the "Robot Moving Object", what would be a good name for such a component? How about - <emphasis>Motor</emphasis>? If we leave the route planning in the <classname>Robot</classname> object, - we can intercept the requests that the <classname>Robot</classname> makes to its - <classname>Motor</classname>, which means that we would be able to see inside the Robot without holding - on to the route data. First we define an interface for <classname>Motor</classname>. We know that there must be some kind of - request, which we'll call <methodname>move()</methodname>, that takes some kind of move request as a parameter. + action, which we'll call <methodname>move()</methodname>, that takes some kind of move request as a parameter. We don't yet know what's in that request, so we'll define an empty <classname>MoveRequest</classname> interface as a placeholder to get us through the compiler. (Of course, in dynamic languages, such as Smalltalk and Python, we don't even need to do that.) @@ -202,17 +210,17 @@ <para> Now that we've separated out the motor from the robot, where does the <classname>Robot</classname> object get its instance of the <classname>Motor</classname> class? We don't yet know what a real <classname>Motor</classname> - looks like, so we can't just create a default instance. We could, however, pass in an temporary implementation - during the test, so we'll add a <classname>Motor</classname> to the <classname>Robot</classname>'s constructor. + looks like, but we can't create a <classname>Robot</classname> without one. To get around this, we could + put together a temporary implementation for the test and pass it through to the <classname>Robot</classname>'s + constructor. Later on, when we have a real implementation of <classname>Motor</classname>, we can pass one of + those through instead. </para> <para> Now we have to decide what the test implementation will do. In this test, we want to be sure that the <classname>Robot</classname> stays in place, so the test <classname>Motor</classname> should simply fail if it receives any requests to move. We can write this test now, before we know anything else about - the system, which means that we have locked down a little piece of the specification. We can be sure that, - however complex our routing code gets, the <classname>Robot</classname> will not move if asked to go to - its current position. The new version of the test is: + the system. The new version of the test is: </para> <programlisting> @@ -229,11 +237,13 @@ robot.setCurrentPosition(POSITION); robot.goTo(POSITION); - assertEquals("Should be same", POSITION, robot.getPosition()); + assertEquals("Should be same", POSITION, robot.getCurrentPosition()); }</programlisting> <para> - Now if there's a bug in the robot routing code that asks the motor to move, the test will fail at the + We have just locked down a little piece of the specification. We can be sure that, however complex our + routing code gets, the <classname>Robot</classname> will not move if asked to go to its current position. + From now on, if there's a bug in the robot routing code that asks the motor to move, the test will fail at the point that the request is made. The error report might look like: </para> @@ -253,13 +263,13 @@ which gives a much clearer view of where the error became visible. If finding the problem turns out to be harder, we can trap the &junit; <classname>AssertionFailedError</classname> in the development tool to bring up the debugger. Then we can explore the program state at the time of the failure, without - having to step through from the beginning of the test. Of course, this doesn't work in every case, but most of - the time it takes you to straight the heart of the problem. + having to step through from the beginning of the test. Of course, this doesn't work in every case but, more + often than not, it takes you to straight to the source of the problem. </para> </section> <!-- Breaking apart the Robot --> <section> - <title>More complex tests</title> + <title>Testing a single step route</title> <para> Now let's revisit the second test, moving to an adjacent space. We want to ensure that exactly one move request has been made and that it contains the right values. Testing that the request is made at most @@ -282,27 +292,54 @@ Robot robot = new Robot(mockMotor); robot.setCurrentPosition(new Position(0, 0)); - robot.goto(DESTINATION); + robot.goTo(DESTINATION); - assertEquals("Should be destination", DESTINATION, robot.getPosition()); + assertEquals("Should be destination", DESTINATION, robot.getCurrentPosition()); }</programlisting> <para> - The first assertion will fail if the <classname>Robot</classname> passes an incorrect - <classname>MoveRequest</classname> or <constant>null</constant>. The second assertion will fail if the - <classname>Robot</classname> calls <function>move()</function> more than once. This test won't + The first assertion will fail if the <classname>Robot</classname> sends the wrong + <classname>MoveRequest</classname>. The second assertion will fail if the + <classname>Robot</classname> calls <function>move()</function> more than once. Unfortunately, this test won't fail if the <classname>Robot</classname> doesn't call <function>move()</function> at all, as it won't - have tried either assertion. We can only tell whether this failure has happened <emphasis>after</emphasis> - we've finished <function>goto()</function>, so we need to record how many times <function>move()</function> - was called. We can't just push <varname>moveCount</varname> up from the anonymous <classname>Motor</classname> - class to the test method because, unfortunately, Java requires such variables to be declared - <token>final</token> and we can't change the value of a <type>final int</type>. There are two alternatives. + try either assertion. We can only check for this kind of failure <emphasis>after</emphasis> + <function>goTo()</function> has finished, so we need to know how many times <function>move()</function> + was called. Unfortunately, we can't just push <varname>moveCount</varname> up from the anonymous + <classname>Motor</classname> class to the test method because a quirk of Java requires such variables to + be declared <token>final</token> and we can't change the value of a <type>final int</type>. I can think of + two alternatives that would solve our immediate problem. </para> + <para> + First, we could write a named implementation of <classname>Motor</classname> so that the test can see + the <varname>moveCount</varname> field. + </para> + <programlisting> +static class AbstractMotorStub implements Motor { + int moveCount = 0; +} + +public void testMoveOnePoint() { + final Position DESTINATION = new Position(1, 0); + + AbstractMotorStub mockMotor = new AbstractMotorStub() { + public void move(MoveRequest request) { + assertEquals("Should be move", new MoveRequest(1, MoveRequest.SOUTH), request); + moveCount++; + assertEquals("Should be first move", 1, moveCount); + } + }; + + Robot robot = new Robot(mockMotor); + robot.setCurrentPosition(new Position(0, 0)); + robot.goTo(DESTINATION); + + assertEquals("Should be destination", DESTINATION, robot.getCurrentPosition()); + assertEquals("Should be one move", 1, mockMotor.moveCount); +}</programlisting> <para> - First we could use a <glossterm linkend="selfshunt">Self Shunt</glossterm> instead of an anonymous class. This - means that the test class itself implements <classname>Motor</classname> so the test has direct access - to its instance fields. For example: + Second, we could use a <glossterm linkend="selfshunt">Self Shunt</glossterm> instead of an anonymous class. + This means that the test class itself implements <classname>Motor</classname>. </para> <programlisting> @@ -321,15 +358,57 @@ Robot robot = new Robot(this); robot.setCurrentPosition(new Position(0, 0)); - robot.goto(DESTINATION); + robot.goTo(DESTINATION); - assertEquals("Should be destination", DESTINATION, robot.getPosition()); + assertEquals("Should be destination", DESTINATION, robot.getCurrentPosition()); assertEquals("Should be one move", 1, moveCount); } }</programlisting> <para> - Alternatively, we could change the outer level data type, as follows: + Personally, I prefer the first of these two options because I find it easier to see the boundaries + between the stub and the test classes. As you'll see shortly, I think that makes handling sequences of + events easier. That said, there's not a huge difference, there are plenty of people making effective use + of Self Shunt, and it's more important that your team is consistent about its approach. + <tip> + <para> + The most important thing is for the team to have a consistent approach to writing unit tests. + </para> + </tip> + </para> + + </section> <!-- Testing a single step route --> + + <section> + <title>Testing more than one step</title> + <!-- TODO --> <comment>Under development</comment> + + <para> + Now we want to test a route that requires two steps. + </para> + <programlisting> +public void testMoveOnePoint() { + final Position DESTINATION = new Position(1, 0); + + AbstractMotorStub mockMotor = new AbstractMotorStub() { + public void move(MoveRequest request) { + assertEquals("Should be move", new MoveRequest(1, MoveRequest.SOUTH), request); + moveCount++; + assertEquals("Should be first move", 1, moveCount); + } + }; + + Robot robot = new Robot(mockMotor); + robot.setCurrentPosition(new Position(0, 0)); + robot.goTo(DESTINATION); + + assertEquals("Should be destination", DESTINATION, robot.getCurrentPosition()); + assertEquals("Should be one move", 1, mockMotor.moveCount); +}</programlisting> + + + <para> + Third, we could change the way we hold our move requests, we can let the </para> <programlisting> @@ -350,15 +429,14 @@ expectedMoveRequests.add(new MoveRequest(1, MoveRequest.SOUTH)); robot.setCurrentPosition(new Position(0, 0)); - robot.goto(DESTINATION); + robot.goTo(DESTINATION); - assertEquals("Should be destination", DESTINATION, robot.getPosition()); + assertEquals("Should be destination", DESTINATION, robot.getCurrentPosition()); assertEquals("Should be no more moves", 0, expectedMoveRequests.size()); }</programlisting> <para> In this version, I'm holding the details of the expected route in <varname>expectedMoveRequests</varname>. - <!-- TODO --> <comment>Under development</comment> </para> <para> @@ -405,7 +483,7 @@ public void testGotoSamePlace() { robot.goTo(ORIGIN); - assertEquals("Should be same", ORIGIN, robot.getPosition()); + assertEquals("Should be same", ORIGIN, robot.getCurrentPosition()); mockMotor.verify(); } public void testMoveOnePoint() { @@ -413,9 +491,9 @@ mockRobot.addExpectedRequest(new MoveRequest(1, MoveRequest.SOUTH)); - robot.goto(DESTINATION); + robot.goTo(DESTINATION); - assertEquals("Should be destination", DESTINATION, robot.getPosition()); + assertEquals("Should be destination", DESTINATION, robot.getCurrentPosition()); mockMotor.verify(); } public void testMoveALongWay() { @@ -423,9 +501,9 @@ mockMotor.addExpectedRequests(makeExpectedLongWayMoveRequests()); - robot.goto(DESTINATION); + robot.goTo(DESTINATION); - assertEquals("Should be destination", DESTINATION, robot.getPosition()); + assertEquals("Should be destination", DESTINATION, robot.getCurrentPosition()); mockMotor.verify(); } } Index: patterns.xml =================================================================== RCS file: /cvsroot/mockobjects/no-stone-unturned/doc/xdocs/patterns.xml,v retrieving revision 1.1 retrieving revision 1.2 diff -u -r1.1 -r1.2 --- patterns.xml 12 Aug 2002 23:07:58 -0000 1.1 +++ patterns.xml 17 Aug 2002 12:59:10 -0000 1.2 @@ -9,4 +9,13 @@ </glossentry> </glossdiv> <!-- S --> + <glossdiv> + <title>T</title> + + <glossentry id="telldontask"> + <glossterm>Tell, Don't ask</glossterm> + &TODO; + </glossentry> + </glossdiv> <!-- T --> + </glossary> Index: htmlbook.css =================================================================== RCS file: /cvsroot/mockobjects/no-stone-unturned/doc/xdocs/htmlbook.css,v retrieving revision 1.7 retrieving revision 1.8 diff -u -r1.7 -r1.8 --- htmlbook.css 10 Aug 2002 23:43:41 -0000 1.7 +++ htmlbook.css 17 Aug 2002 12:59:10 -0000 1.8 @@ -6,10 +6,13 @@ DIV.section H2 { font-style: italic; font-weight: normal; } DIV.section H3 { font-style: normal; font-size: small; } +DIV.tip H3 { font-style: italic; font-weight: normal; font-size: small; } +PRE SPAN.emphasis { font-weight: bold; } .programlisting { margin-left: 5%; } .screen { margin-left: 5%; } .sidebar { border: double black 1px; font-size: 80%; padding: 4px; text-align: center; margin-left: 80%; } +.tip { border: double black 1px; font-size: 80%; padding: 2px; } .screenshot { margin-left: 20%; } .caption { font-size: 80%; font-style: italic; } .guibutton { font-weight: bold; } |