We have junit tests
-
A guy on our team just
finishedcoded-and-left-for-an-extended-vacation a major chunk of code. He proudly notified the entire team that he build more than 1,000 JUnit tests for his module. Today, we merged in his stuff, and immediately noticed that test # 1 of his module was failing in the hourly build. Since he's gone, we looked into it, and found a single 10K+ line test method:public class MyClassTest extends TestCase {
public void setUp() { ... }
public void testMethod() {
// Test 1: ...
// code of test 1
assertFalse(someVariable);
// Test 2: ...
// code of test 2
assertTrue(someVariable);
...
// Test 1320: ...
// code of test 1320
assertNotNull(somevariable);
}
}
-
@snoofle said:
A guy on our team just
Âfinishedcoded-and-left-for-an-extended-vacation a major chunk of code.Just how extended was this vacation intended to be, and how extended will it become after he returns from said vacation?
And are those comments verbatim? If so, that's epic.
-
To be fair, the // code for test n comments are me saving lots of typing.
-
No, you have a JUnit test. When your coworker returns from his vacation, I suggest you explain to him the concept of unit tests.
-
@snoofle said:
A guy on our team just
 Is there always exactly one assertXXX statement at the end of each block of testcase code? In that case, how about this:finishedcoded-and-left-for-an-extended-vacation a major chunk of code. He proudly notified the entire team that he build more than 1,000 JUnit tests for his module. Today, we merged in his stuff, and immediately noticed that test # 1 of his module was failing in the hourly build. Since he's gone, we looked into it, and found a single 10K+ line test method:public class MyClassTest extends TestCase {
public void setUp() { ... }
public void testMethod() {
// Test 1: ...
// code of test 1
assertFalse(someVariable);
// Test 2: ...
// code of test 2
assertTrue(someVariable);
...
// Test 1320: ...
// code of test 1320
assertNotNull(somevariable);
}
}$ sed < tests.java -e '/assert/=;/^assert/=;s/\(.*\)/@\1/' | \ sed -e 's/^\(.*\)assert\(.*\)$/\1assert\2\n@ }\n@\n@ public void testMethodXXX() {\n@/' | \ sed -e 's/^@//; t DONE ; h ; d ; : DONE ; /XXX/{g;s/\(.*\)/ public void testMethod \1() {/}'
public class MyClassTest extends TestCase { public void setUp() { ... }public void testMethod() {
// Test 1: ...
// code of test 1
assertFalse(someVariable);
}public void testMethod7() {
// Test 2: ... // code of test 2 assertTrue(someVariable);}
public void testMethod11() {
... // Test 1320: ... // code of test 1320 assertNotNull(somevariable);}
public void testMethod17() {
}
}admin@ubik /tmp/tdwtf
$
I used line numbers to uniquify the names instead of a sequentially incrementing counter because of the hideousness of doing maths in SED, so that's left as an exercise for the reader...
-
-
@blakeyrat said:
@DaveK said:
No, it isn't, for at least two reasons.(snip)
This is how you spend your holiday weekend Sunday mornings?
Â
-
@DaveK said:
[ . . . ] how about this:
Oops, there shouldn't be a space between "testMethod" and "\1()" in the third line there, that crept in somehow when I rewrapped the lines.$ sed < tests.java -e '/assert/=;/^assert/=;s/\(.*\)/@\1/' | \
sed -e 's/^\(.*\)assert\(.*\)$/\1assert\2\n@ }\n@\n@ public void testMethodXXX() {\n@/' | \
sed -e 's/^@//; t DONE ; h ; d ; : DONE ; /XXX/{g;s/\(.*\)/ public void testMethod \1() {/}'
-
So yeah, that's definitely a WTF. But, only because of the implementation. It's not as bad as it could be; at least this guy made an effort to write tests, even if it was wrong through and through. I'd suggest sitting him down when he gets back and giving him a code review on his test: outline the things that should be in tests, including how to write proper tests (multiple methods, classes, fixtures, etc), and then commend him on the things that he did correct: such as the various ways that he tests possible values for his variable(s).
