Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > Java > state of an object in setUp() of junit TestCase

Reply
Thread Tools

state of an object in setUp() of junit TestCase

 
 
Lew
Guest
Posts: n/a
 
      10-30-2010
markspace wrote:
>> I've used instance fields but initialized them each time before a test.
>> I don't know if this is good practice or not but there it is.
>>
>> Partial code:
>>
>> public class BasicIrcParserTest
>> {
>>
>> private Writer writer;
>> private MockCommandList commandList;
>> private ChatScreen screen;
>>
>> @Before
>> public void setUp()
>> {
>> writer = new StringWriter();
>> commandList = new MockCommandList();
>> screen = new ChatScreen()
>> ...
>> }
>>
>> ...


Lew wrote:
> I aver that that is good practice. First, the default value for 'writer'
> will be 'null', so you need to initialize to the non-default values in
> your '@Before'. So you have no choice. Second, you aren't leaving it
> undocumented to those who read your test source - maintainers can
> readily follow the test flow. Third, you provide a hook for future
> initializations as the test class expands.


Oh, and if you refer to using instance variables vs. local ones, given that by
design JUnit creates a new test instance per test there's no risk, and it
allows the clean separation of setup, test and teardown, so that part is good
practice, too. Otherwise you risk setting up resources differently, or
failing to tear down cleanly.

Side note: Arguments an be made either way to treat the word "setup"
(accepted by Thunderbird's spell checker) as a single word or a compound.

--
Lew
 
Reply With Quote
 
 
 
 
Tom Anderson
Guest
Posts: n/a
 
      10-30-2010
On Sat, 30 Oct 2010, Lew wrote:

>> public void stop(){
>> if (isrunning) stopTime=System.currentTimeMillis();

>
> This 'if' block violates the coding conventions.


Why? If you mean because it doesn't have braces round the dependent
statement, then the coding conventions you refer to should be violated,
because that's a perfectly fine thing to do.

If you mean because of the lack of spaces around the equals sign, then
fine.

>> System.out.println("sleeping for
>> :"+(timeinmillisecs/1000)+"secs");

>
> 'System.out.println()'? Really? Come on.
>
> If it's important enough to log, it's important enough to log correctly.
>
> If you really are such a schlemiel that you have to use a
> 'System.X.println()', use 'System.err' at least. But really, use a logger.


In a test case? No, absolutely not. There is nothing whatsoever wrong with
using the System streams in a test case. Using a logger is pointless
overkill.

However, in this case, i'd suggest not logging at all, by any means,
because the information being logged doesn't look very interesting.

>> public void testNormalOpButForgottToStop(){
>> watch.start();
>> sleepForSomeTime(3000);
>> //watch is not stopped;
>> assertEquals(3,watch.getTotalDurationSeconds());
>> }

>
> This is not a valid test. Since the watch is not stopped and sleep
> times are approximate, you have a small risk that this assertion will
> fail even with correct code.
>
> The granularity of one second will make this rare, but not theoretically
> impossible.


Absolutely right.

> You should comment your code that you depend on the granularity to avoid
> trouble.


I'm not sure that this will avoid trouble; it'll just make it clear what
the problem is when there is trouble.

I'd suggest a test more like:

public void testNormalOpButForgottToStop() {
long startTime = System.currentTimeMillis();
watch.start();
sleepForSomeTime(3000);
long endTime = System.currentTimeMillis();
long totalDuration = watch.getTotalDurationSeconds()
assertEquals(((endTime - startTime) / 1000), totalDuration);
}

tom

--
Ed editor textorum probatissimus est -- Cicero, De officiis IV.7
 
Reply With Quote
 
 
 
 
Stanimir Stamenkov
Guest
Posts: n/a
 
      10-30-2010
Sat, 30 Oct 2010 10:55:22 -0400, /Lew/:
> jimgardener wrote:
>
>> boolean isrunning=false;

>
> The variable name 'isrunning' violates the naming conventions.


Do you mean because 'running' is not capitalized as in 'isRunning'
or because it is using 'is' as in a boolean accessor method name?

--
Stanimir
 
Reply With Quote
 
Lew
Guest
Posts: n/a
 
      10-30-2010
jimgardener wrote:
>>> public void stop(){
>>> if (isrunning) stopTime=System.currentTimeMillis();


Lew wrote:
>> This 'if' block violates the coding conventions.


Tom Anderson wrote:
> Why? If you mean because it doesn't have braces round the dependent
> statement, then the coding conventions you refer to should be violated,
> because that's a perfectly fine thing to do.
>
> If you mean because of the lack of spaces around the equals sign, then
> fine.


I meant the lack of braces, something anyone familiar with the conventions
would have spotted, and whether "that's a perfectly fine thing to do" or not
is a separate question. That it violates the conventions is a fact.

<http://www.oracle.com/technetwork/java/codeconventions-142311.html#449>

--
Lew
 
Reply With Quote
 
Lew
Guest
Posts: n/a
 
      10-30-2010
jimgardener wrote:
>>> boolean isrunning=false;


Lew wrote:
>> The variable name 'isrunning' violates the naming conventions.


Stanimir Stamenkov wrote:
> Do you mean because 'running' is not capitalized as in 'isRunning' or
> because it is using 'is' as in a boolean accessor method name?


Familiarity with the conventions makes the answer obvious.

The former is distinctly against the conventions, as anyone familiar with the
conventions can see,
<http://www.oracle.com/technetwork/java/codeconventions-135099.html#367>

I am aware of nothing in that document that forbids 'isRunning' as a boolean
variable name, or any other type variable name for that matter.

--
Lew
 
Reply With Quote
 
Tom Anderson
Guest
Posts: n/a
 
      10-31-2010
On Sat, 30 Oct 2010, Lew wrote:

> jimgardener wrote:
>>>> public void stop(){
>>>> if (isrunning) stopTime=System.currentTimeMillis();

>
> Lew wrote:
>>> This 'if' block violates the coding conventions.

>
> Tom Anderson wrote:
>> Why? If you mean because it doesn't have braces round the dependent
>> statement, then the coding conventions you refer to should be violated,
>> because that's a perfectly fine thing to do.
>>
>> If you mean because of the lack of spaces around the equals sign, then
>> fine.

>
> I meant the lack of braces, something anyone familiar with the conventions
> would have spotted, and whether "that's a perfectly fine thing to do" or not
> is a separate question. That it violates the conventions is a fact.


True. I'm uncertain what the value of pointing the fact out is, though,
when it's a rule that isn't a very good one.

tom

--
The Impossible is True
 
Reply With Quote
 
Tom Anderson
Guest
Posts: n/a
 
      10-31-2010
On Sat, 30 Oct 2010, Lew wrote:

> markspace wrote:
>> I've used instance fields but initialized them each time before a test.
>> I don't know if this is good practice or not but there it is.
>>
>> Partial code:
>>
>> public class BasicIrcParserTest
>> {
>>
>> private Writer writer;
>> private MockCommandList commandList;
>> private ChatScreen screen;
>>
>> @Before
>> public void setUp()
>> {
>> writer = new StringWriter();
>> commandList = new MockCommandList();
>> screen = new ChatScreen()
>> ...
>> }
>>
>> ...

>
> I aver that that is good practice.


Agreed.

For some reason, it has become the normal style to do test setup in an
@Before method rather than a constructor or initialisers, so that's where
people reading tests will look for it. It's preferable to put it where
people will look for it.

tom

--
The Impossible is True
 
Reply With Quote
 
markspace
Guest
Posts: n/a
 
      10-31-2010
On 10/31/2010 12:16 PM, Tom Anderson wrote:

> For some reason, it has become the normal style to do test setup in an
> @Before method rather than a constructor or initialisers, so that's
> where people reading tests will look for it. It's preferable to put it
> where people will look for it.



@Before gets run before each individual test. In my test harness, that
@Before gets run about 8 times as each test executes.

That's the point: don't let state leak from one test to another. Set
the test harness up fresh before each test. @Before provides a
convenient way of doing this.

 
Reply With Quote
 
Jim Janney
Guest
Posts: n/a
 
      11-01-2010
markspace <(E-Mail Removed)> writes:

> On 10/30/2010 9:39 AM, Stanimir Stamenkov wrote:
>
>>> ... Relying on such default values, however, is generally
>>> considered bad programming style.

>>
>> I don't completely agree with the given statement, but then I can't come
>> up with arguments on any side. What do you think about it?

>
>
> Lew's comment that an explicit initialization requires a second,
> redundant and useless initialization is enough to counter Oracle's
> statement.


Premature and probably pointless micro-optimization. I'm having
trouble imagining a class where this could make a measurable
difference in execution speed or a meaningful difference in code size.

First make it readable. Then make it right. Then, as an optional
step if that turns out to be necessary, worry about making it fast.

> I'd say precisely the opposite. The default values are 100%
> guaranteed and deterministic, there's no reason not to rely on them,
> and it's certainly not bad style. I'm sorry for whoever wrote that
> line in the tutorial but I think they went off some place unknown to
> most professionals.


Professionals understand that readability normally trumps minor
efficiency gains. We can debate which style is clearer, but redundant
code shouldn't be a concern.

--
Jim Janney
 
Reply With Quote
 
Tom Anderson
Guest
Posts: n/a
 
      11-01-2010
On Sun, 31 Oct 2010, markspace wrote:

> On 10/31/2010 12:16 PM, Tom Anderson wrote:
>
>> For some reason, it has become the normal style to do test setup in an
>> @Before method rather than a constructor or initialisers, so that's
>> where people reading tests will look for it. It's preferable to put it
>> where people will look for it.

>
> @Before gets run before each individual test. In my test harness, that
> @Before gets run about 8 times as each test executes.


Right. But the constructor also gets run before each individual test,
because each test gets a fresh instance of the object. Hence, the
difference between @Before and the constructor is a matter of style, not
function.

An illustrative example:

import org.junit.*;
public class TestCount {
public TestCount() {System.err.println("constructor");}
@Before public void before() {System.err.println("before");}
@Test public void one() {System.err.println("one");}
@Test public void two() {System.err.println("two");}
}

I get:

JUnit version 4.6
.constructor
before
one
.constructor
before
two

Time: 0.032

OK (2 tests)

It occurs to me that there is no alternative to using @After, given that
finalizers and weak references and so on do not execute in bounded time.
Given that you have to use @After for teardown, symmetry favours using
@Before for setup.

tom

--
The literature is filled with bizarre occurrances for which we have
no explanation
 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
Differerence between Test::Unit::TestCase and RUNIT::TestCase Scott Ruby 1 08-20-2005 08:49 PM
unittest.TestCase, lambda and __getitem__ Steven Bethard Python 7 09-14-2004 02:43 PM
Create TestCase with WSAD on EJB Memi Lavi Java 0 08-05-2004 03:23 PM
Computing test methods in unittest.TestCase Jan Decaluwe Python 2 03-02-2004 06:33 PM
standard struts testcase failed ? Charlie Java 1 10-16-2003 04:58 AM



Advertisments