state of an object in setUp() of junit TestCase

[jimgardener]

If I forget to define a tearDown() method ,will an object initialized
in setUp() maintain its state in between two test methods?
suppose I am testing a Stopwatch class
<code>
public class Stopwatch {
public long startTime=0;
public long stopTime=0;
boolean isrunning=false;
public void start(){
startTime=System.currentTimeMillis();
isrunning=true;
}
public void stop(){
if (isrunning) stopTime=System.currentTimeMillis();
isrunning=false;
}

public long getTotalDurationMilliSeconds(){
long duration=0;
if( isrunning){
duration=(System.currentTimeMillis()-startTime);
}
else{
duration=stopTime-startTime;
}
return duration;
}
public void reset(){
startTime=0;
stopTime=0;
isrunning=false;
}

public long getTotalDurationSeconds(){
long duration=0;
if( isrunning){
duration=((System.currentTimeMillis()-startTime)/1000);
}
else{
duration=(stopTime-startTime)/1000;
}
return duration;
}
}

</code>

I have a testcase like below,in which I have a setUp() but no
tearDown()


<code>
class StopwatchTest extends TestCase{
private Stopwatch watch;
public StopwatchTest(String name){
super(name);
}
public void setUp(){
watch=new Stopwatch();
}
private void sleepForSomeTime(long timeinmillisecs){
try{
System.out.println("sleeping for :"+(timeinmillisecs/1000)+"secs");
Thread.sleep(timeinmillisecs);
}catch(InterruptedException e){

}
}
public void testNormalOpButForgottToStop(){
watch.start();
sleepForSomeTime(3000);
//watch is not stopped;
assertEquals(3,watch.getTotalDurationSeconds());
}
public void testStoppedBeforeStart(){
System.out.println("timeduration:"+watch.getTotalDurationSeconds());
sleepForSomeTime(2000);
watch.stop();
System.out.println("timeduration:"+watch.getTotalDurationSeconds());
assertEquals(0,watch.getTotalDurationSeconds());
}

}

</code>

It seems that the watch is initialized each time before running the
test methods.Is this the expected behaviour?


regards,
jim

 

[jimgardener]

It seems that the watch is initialized each time before running the
test methods.Is this the expected behaviour?

sorry..wrong wording..looks like the state of the object is reset each
time before the test method
jim

 

[Lew]

You answered your own question, so I will just address some side issues.

If I forget to define a tearDown() method ,will an object initialized
in setUp() maintain its state in between two test methods?
suppose I am testing a Stopwatch class
<code>
public class Stopwatch {
public long startTime=0;

These are the default values. Explicitly setting them in member
initialization causes them to be set twice. In the case of 'startTime' and
'isrunning' [sic] you don't even use the initial value, so why bother setting it?

public long stopTime=0;
boolean isrunning=false;

The variable name 'isrunning' violates the naming conventions.

public void start(){
startTime=System.currentTimeMillis();
isrunning=true;
}
public void stop(){
if (isrunning) stopTime=System.currentTimeMillis();

This 'if' block violates the coding conventions.

isrunning=false;
}

public long getTotalDurationMilliSeconds(){
long duration=0;

You never use the '0' value, so why do you set it? Just so you can discard it?

if( isrunning){
duration=(System.currentTimeMillis()-startTime);
}
else{
duration=stopTime-startTime;
}
return duration;
}
public void reset(){
startTime=0;
stopTime=0;
isrunning=false;
}

public long getTotalDurationSeconds(){
long duration=0;

Again, why did you initialize 'duration' to a value that is immediately discarded?

if( isrunning){
duration=((System.currentTimeMillis()-startTime)/1000);
}
else{
duration=(stopTime-startTime)/1000;
}
return duration;
}
}

</code>

I have a testcase like below,in which I have a setUp() but no
tearDown()


<code>
class StopwatchTest extends TestCase{

How come you aren't using '@Test'?

private Stopwatch watch;
public StopwatchTest(String name){
super(name);
}
public void setUp(){
watch=new Stopwatch();
}
private void sleepForSomeTime(long timeinmillisecs){

The variable name 'timeinmillisecs' egregiously violates the naming standards.

try{
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.

Thread.sleep(timeinmillisecs);
}catch(InterruptedException e){

}
}
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. You should comment your code that you depend on the granularity
to avoid trouble.

 

[jimgardener]

sorry..that was sloppy work on my side..Will be more careful..
thanks Lew for the suggestions.
regards
jim

 

[Tom Anderson]

If I forget to define a tearDown() method ,will an object initialized
in setUp() maintain its state in between two test methods?

No. JUnit creates a new instance of the test class for each test; if you
write your test classes as you do (which is the right way to do it), then
state won't leak from one test to the next, because there's a fresh set of
instance variables each time.

Note the if - if, on the other hand, you keep state in places which
persist between instances (static variables, objects outside the test
object), then it will leak from test to test. So don't do that.

tom

 

[Stanimir Stamenkov]

Sat, 30 Oct 2010 10:55:22 -0400, /Lew/:

These are the default values. Explicitly setting them in member
initialization causes them to be set twice.

I don't like specifying initializers using the default values for
the same reason, but I was once pointed to the Java Tutorial

... 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?

 

[markspace]

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.

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.

 

[markspace]

Note the if - if, on the other hand, you keep state in places which
persist between instances (static variables, objects outside the test
object), then it will leak from test to test. So don't do that.

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]

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

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.

This is a controversy. I sit somewhat in the middle - if the default initial
value carries algorithmic significance, e.g.,

private boolean initialized = false;

then the redundant initialization emphasizes that the algorithm depends on
that value, but if the usage pattern requires a call to some 'init()' or
'setup()' or 'prepare()' or such that sets the value before it's read, or if
otherwise there's nothing truly emphatic about the default value, then one
should follow markspace's advice.

private boolean initialized;

IME a need to emphasize initialization of member references to 'null' is rarer
than a need to emphasize initialization of primitives to their "zero-like" values.

Mind you, it is still completely unnecessary to initialize a member twice to
its default value just to make a point that a comment would make just as well,

private boolean initialized; // must start out 'false'

perhaps augmented with an invariant if you're anal retentive.

/** Constructor. */
public Foo()
{
// ... do all those lovely constructor things
assert ! initialized;
}

Now that makes the point and the JVM can help enforce it.

 

[Lew]

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. 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.

 

[Lew]

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.

 

[Tom Anderson]

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()'? 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.

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

 

[Stanimir Stamenkov]

Sat, 30 Oct 2010 10:55:22 -0400, /Lew/:

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?

 

[Lew]

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]

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.

 

[Tom Anderson]

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

 

[Tom Anderson]

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

 

[markspace]

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.

 

[Jim Janney]

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.

 

[Tom Anderson]

@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