The Daily WTF: Curious Perversions in Information Technology

2014-02-13 Reply Admin

Mortgage Driven Development!

2014-02-13 Reply Admin

Don't know what to write. //Fix this comment later

2014-02-13 Reply Admin

I have seen similar naming conventions - but if I apply it to this it means "TestCase 119a" - so you are supposed to look into the document describing testcases. Still bad code, but refering to a TestCase is not too bad in my book... it forces ppl to read them.

TGV · 2014-02-13 Reply Admin

The number is cumbersome, but the test case should be copied in the comment.

Anyway, there's not always a need for an assert in a test case. If Hold or Retrieve throw an exception, the test fails.

2014-02-13 Reply Admin

Then at the very minimum document/comment that in the test. All it takes is:

// Wot? No assertions? Test is considered a pass if no exceptions raised.

Just those few words save the future developer the headache of trying to figure out the original developers' intentions.

2014-02-13 Reply Admin

thrixth!

Remy Porter · 2014-02-13 Reply Admin

Personally, I always include assertions, even if all I'm testing is that no exceptions were raised (or that the correct exception was raised). Assert.IsTrue(true) seems like a silly statement to write, but it's a nice indicator that if I hit this line, the test is currently passing. I can put an Assert.IsTrue(false) in the catch block for exceptions.

TGV · 2014-02-13 Reply Admin

Remy Porter:
Personally, I *always* include assertions, even if all I'm testing is that no exceptions were raised (or that the *correct* exception was raised). Assert.IsTrue(true) seems like a silly statement to write, but it's a nice indicator that *if I hit this line, the test is currently passing*. I can put an Assert.IsTrue(false) in the catch block for exceptions.

I don't like it. Matter of taste, but the semantics of Test are quite clear: raise an exception and it fails. Assert is merely a way of conditionally raising one.

2014-02-13 Reply Admin

semubiz:
Don't know what to write. //Fix this comment later

assert(hadBeenLoggedInWhenWritingComment('semubiz')); => failed. // TODO: build time machine

2014-02-13 Reply Admin

TGV:
Remy Porter:
Personally, I *always* include assertions, even if all I'm testing is that no exceptions were raised (or that the *correct* exception was raised). Assert.IsTrue(true) seems like a silly statement to write, but it's a nice indicator that *if I hit this line, the test is currently passing*. I can put an Assert.IsTrue(false) in the catch block for exceptions.
I don't like it. Matter of taste, but the semantics of Test are quite clear: raise an exception and it fails. Assert is merely a way of conditionally raising one.

I agree. I personally would write something like:

noExceptionsThrown = true;
try {
  method_that_should_not_throw();
}
catch (ex) {
  noExceptionThrown = false;
}
assert(noExceptionsThrown);

2014-02-13 Reply Admin

Fly:
I have seen similar naming conventions - but if I apply it to this it means "TestCase 119a" - so you are supposed to look into the document describing testcases. Still bad code, but refering to a TestCase is not too bad in my book... it forces ppl to read them.

I doubt that when someone puts a comment like "// Somehow detect that hold fails. How? Fix this later." into a test case, that someone will have taken the time to create a document that describes what the test case tests...

ratchet freak · 2014-02-13 Reply Admin

some frameworks let you specify which exceptions are expected, why not use that then...

2014-02-13 Reply Admin

4chan:
Fly:
I have seen similar naming conventions - but if I apply it to this it means "TestCase 119a" - so you are supposed to look into the document describing testcases. Still bad code, but refering to a TestCase is not too bad in my book... it forces ppl to read them.
I doubt that when someone puts a comment like "// Somehow detect that hold fails. How? Fix this later." into a test case, that someone will have taken the time to create a document that describes what the test case tests...

Not necessarily. I've been on projects which are documented to an insane level of detail, such that the actual paragraph numbers in that specification are expected to be recorded (either by comment or, more usually and insanely, by method naming convention) in the code itself.

What you often find, in such environment, is that the senior developers are the ones writing that specification, while the juniors and monkeys are set with the exercise of converting those specs to working code. And of course the unit tests are subject to the same level of management, but for some reason the actual coders can't find the appropriate level of enthusiasm for that task ...

2014-02-13 Reply Admin

Fly:
I have seen similar naming conventions - but if I apply it to this it means "TestCase 119a" - so you are supposed to look into the document describing testcases. Still bad code, but refering to a TestCase is not too bad in my book... it forces ppl to read them.

No, that is the biggest WTF today. Terrible terrible terrible.

Slipping into girlfriend speak now: "If you don't know why then there is no point in me explaining it to you"

2014-02-13 Reply Admin

TC14 - Assert.AreEqual(TC14,C3PO)

2014-02-13 Reply Admin

Remy Porter:
Assert.IsTrue(true) seems like a silly statement to write, but it's a nice indicator that *if I hit this line, the test is currently passing*. I can put an Assert.IsTrue(false) in the catch block for exceptions.

...WTF? Other than completely trashing the original exception, what do you accomplish by catching it and then throwing a completely different one?

2014-02-13 Reply Admin

anonymous:
Remy Porter:
Assert.IsTrue(true) seems like a silly statement to write, but it's a nice indicator that *if I hit this line, the test is currently passing*. I can put an Assert.IsTrue(false) in the catch block for exceptions.
...WTF? Other than completely trashing the original exception, what do you accomplish by catching it and then throwing a completely different one?

That's a bugbear of mine - unit tests that swallow a perfectly usable exception and replace it with a completely generic "oh, something broke". The original exception should at a minimum be logged somewhere first. Unit tests shouldn't be written to purposefully frustrate the person who has to diagnose the failures...

2014-02-13 Reply Admin

Mike:
No, that is the biggest WTF today. Terrible terrible terrible.
Slipping into girlfriend speak now: "If you don't know why then there is no point in me explaining it to you"

You know, I completely misinterpreted where you were going with that sentence when I read the highlighted words...

2014-02-13 Reply Admin

Since the test is not asserting anything, and no one knows what it's testing, and it only fails on exception,

then the obvious solution is to use the try, catch, discard pattern of exception handling. No exceptions reported = no worries.

try {
 //test code...
 //test code...
 //test code...
} catch (Throwable t) {
  //TODO: someone should figure out how to handle this case.
}

2014-02-13 Reply Admin

4chan:
TGV:
Remy Porter:
Personally, I *always* include assertions, even if all I'm testing is that no exceptions were raised (or that the *correct* exception was raised). Assert.IsTrue(true) seems like a silly statement to write, but it's a nice indicator that *if I hit this line, the test is currently passing*. I can put an Assert.IsTrue(false) in the catch block for exceptions.
I don't like it. Matter of taste, but the semantics of Test are quite clear: raise an exception and it fails. Assert is merely a way of conditionally raising one.
I agree. I personally would write something like:
noExceptionsThrown = true;
try {
  method_that_should_not_throw();
}
catch (ex) {
  noExceptionThrown = false;
}
assert(noExceptionsThrown);

So you prefer to write 8 lines when 1 line will do because the framework takes care of everything else already? (And does so better, cf. other comments WRT trashing exceptions.) You're paid by LOC, right?

(Not talking about your other antipatterns, such as obscuring the code flow with unnecessary booleans, or using booleans with inverted semantics ...)

2014-02-13 Reply Admin

anonymous:
Remy Porter:
Assert.IsTrue(true) seems like a silly statement to write, but it's a nice indicator that *if I hit this line, the test is currently passing*. I can put an Assert.IsTrue(false) in the catch block for exceptions.
...WTF? Other than completely trashing the original exception, what do you accomplish by catching it and then throwing a completely different one?

It depends on how the framework was constructed. Perhaps there was a generic framework and all tester would do is place a breakpoint in the assert function. That way it would always be hit if the test failed, and you could step back to the catch() block and see what the actual failure was.

Is it the most elegant way? Probably not, but maybe the function being checked can't be tested in an elegant fashion.

RichP · 2014-02-13 Reply Admin

TC117a() // // Tests: "are you at your post?" // // Expected behavior: True // // Possible failure cases: // // Bad communicator // Too short to be a Stormtrooper. //

ObiWayneKenobi · 2014-02-13 Reply Admin

Correct me if I'm wrong but... a test case should assert that something occurred. If you just let it run and the pass condition is "No errors" then you aren't really testing anything that you can verify, because you aren't checking that Foo was changed to Bar or whatever you expect to happen.

2014-02-13 Reply Admin

This is what happens when you rely on auto-generated reports from "management" tools that just report pass/fail of unit tests and code coverage percentages, rather than actually doing statistical sampling reviews of developers' work.

A little pair-programming and mentoring go a long way, especially if code reviews are randomly done. ("No one expects the Spanish Code Review" - contemporized Monty Python)

2014-02-13 Reply Admin

ObiWayneKenobi:
Correct me if I'm wrong but... a test case should assert that something occurred. If you just let it run and the pass condition is "No errors" then you aren't really testing anything that you can verify, because you aren't checking that Foo was changed to Bar or whatever you expect to happen.

OK, you're wrong.

"How did this happen? We're smarter than this!"

Well, the purpose of a testing harness is to verify that the code you've written does not break any of the tests you've written. The skill comes in figuring out what needs to be tested against.

Consider: You have a class method that turns Foo to Bar. You build a test that passes Foo to your method, and an Assert that the output is Bar. If your code works, your test passes (Green light).

Now, let's say six months from now, you add another method to the same class, or you change the functionality of your method. You write a test for this new functionality, and it passes (Green light). HOWEVER, this change altered the previous functionality, and now Foo returns DoubleFoo. This causes your first test to fail (Red light).

For me, the main advantage is not that my most recent changes get green lights, but that all PREVIOUS tests get green lights as well. I tend to break things I wrote six months ago, which is why testing is so useful. I might not notice the change to the earlier functionality otherwise.

too_many_usernames · 2014-02-13 Reply Admin

CigarDoug:
I might not notice the change to the earlier functionality otherwise.

Change impact analysis is your friend.

Auction_God · 2014-02-13 Reply Admin

TGV:
The number is cumbersome, but the test case should be copied in the comment.
Anyway, there's not always a need for an assert in a test case. If Hold or Retrieve throw an exception, the test fails.

I disagree with your first statement. That's akin to copying code - A reference to the test case is best (and ideally a version of the document). Otherwise you run the risk of the test requirement changing and the comment not changing. I hate copying anything!

2014-02-13 Reply Admin

Smug Unix User:
TC14 - Assert.AreEqual(TC14,C3PO)

+1

chubertdev · 2014-02-13 Reply Admin

I always use assert for results:

(pseudocode, SFDC-style)

AssertEquals(ConvertFtoC(212), 100);
AssertEquals(ConvertCtoF(-40), -40);

I'd only use try/catch when you EXPECT an exception. Otherwise, let it cause your test to fail on its own.

try
{
   ConvertCtoK(-275);
   //fail here
}
catch (Exception ex)
{
   //probably test for specific error, but at least ensure that it throws the proper exception
}

2014-02-13 Reply Admin

so he went to look and see what he needed to change to update the tests.

Excuse me? Your code changes break the unit tests and your first response is to start making changes to the tests? You need to fix your code, NOT change the unit tests.

Matt, the door is that way, and don't let it hit you on the way out.

chubertdev · 2014-02-13 Reply Admin

Bananas:
so he went to look and see what he needed to change to update the tests.
Excuse me? Your code changes break the unit tests and your first response is to start making changes to the tests? You need to fix your code, NOT change the unit tests.
Matt, the door is that way, and don't let it hit you on the way out.

Wooooooooooow. Completely barking up the wrong tree.

Addendum (2014-02-13 13:49): Think about it: you've inherited an app at a small company.

You update the ConvertFtoC() and ConvertCtoF() methods to raise an exception if values below absolute zero are passed to them. You run the tests. You get this:

Test failed. Value passed to ConvertFtoC(): -40. Expected result: 57. Actual result: -40.

You wouldn't look into the unit test?

2014-02-13 Reply Admin

Conversely, the test could describe what it is testing and you could refer to that directly instead............and you'd already be reading it.

chubertdev · 2014-02-13 Reply Admin

html6dev:
Conversely, the test could describe what it is testing and you could refer to that directly instead............and you'd already be reading it.

Yeah, if you have the test named "Test42" just have a comment saying "check Feature Request 2348 for what this means", you open up the possibility of losing that documentation since it's stored separately. The unit test should have a meaningful name and some comments to at least give you an idea of what it should do if it's not obvious, although it doesn't have to hvae all of the documentation.

2014-02-13 Reply Admin

Bananas:
so he went to look and see what he needed to change to update the tests.
Excuse me? Your code changes break the unit tests and your first response is to start making changes to the tests? You need to fix your code, NOT change the unit tests.
Matt, the door is that way, and don't let it hit you on the way out.

To be fair, one look at the unit test naming scheme is enough to throw suspicion onto their correctness.

2014-02-13 Reply Admin

Bananas:
so he went to look and see what he needed to change to update the tests.
Excuse me? Your code changes break the unit tests and your first response is to start making changes to the tests? You need to fix your code, NOT change the unit tests.
Matt, the door is that way, and don't let it hit you on the way out.

To be fair, the unit tests provided in the article are just useless.

2014-02-13 Reply Admin

Who tests the test code?

2014-02-13 Reply Admin

Watchman:
Who tests the test code?

It's testers all the way down.

TGV · 2014-02-13 Reply Admin

anonymous:
Watchman:
Who tests the test code?
It's testers all the way down.

No, at the end there are four elephants on the back of a turtle. Everyone knows that. Here, take a blue pill.

chubertdev · 2014-02-13 Reply Admin

Are we going to start the comments on unit tests again?

2014-02-13 Reply Admin

TGV:
anonymous:
Watchman:
Who tests the test code?
It's testers all the way down.
No, at the end there are four elephants on the back of a turtle. Everyone knows that. Here, take a blue pill.

What is the turtle standing on?

2014-02-13 Reply Admin

ObiWayneKenobi:
Correct me if I'm wrong but... a test case should assert that something occurred. If you just let it run and the pass condition is "No errors" then you aren't really testing anything that you can verify, because you aren't checking that Foo was changed to Bar or whatever you expect to happen.

Yes, I agree, this. Test cases are generally looking for two things: 1) Something happened 2) Nothing broke

Nothing broke is easy - that just means we didn't throw a whole bunch of exception. Something happened can be trickier, but each method/function/subroutine has a purpose, and we need to make sure that it is fulfilling that purpose.

The assertion measures that the result of the functionality was what you expected, and an exception means that there's a problem that doesn't even let the functionality execute - irrespective of whether it theoretically works (oops, we ran out of memory).

2014-02-13 Reply Admin

CigarDoug:
ObiWayneKenobi:
Correct me if I'm wrong but... a test case should assert that something occurred. If you just let it run and the pass condition is "No errors" then you aren't really testing anything that you can verify, because you aren't checking that Foo was changed to Bar or whatever you expect to happen.
OK, you're wrong.
"How did this happen? We're smarter than this!"
Well, the purpose of a testing harness is to verify that the code you've written does not break any of the tests you've written. The skill comes in figuring out what needs to be tested against.
Consider: You have a class method that turns Foo to Bar. You build a test that passes Foo to your method, and an Assert that the output is Bar. If your code works, your test passes (Green light).

Now, let's say six months from now, you add another method to the same class, or you change the functionality of your method. You write a test for this new functionality, and it passes (Green light). HOWEVER, this change altered the previous functionality, and now Foo returns DoubleFoo. This causes your first test to fail (Red light).

For me, the main advantage is not that my most recent changes get green lights, but that all PREVIOUS tests get green lights as well. I tend to break things I wrote six months ago, which is why testing is so useful. I might not notice the change to the earlier functionality otherwise.

yes, that's what regression tests are - that our new shit works, and our old shit is still ok too....

I have worked somewhere where failing test cases are either ignored, justified or modified so that the don';t fail (that's right, we change the test case, not what it's testing)....Then again, they also used to wax on about Six Sigma - except they'd talk about defects vs potential defects - and I figured this would always give a false sense of security, because by identifying a potential defect you would normally protect against it....the real skilkl would be finding potential defects that you didn't think of....if you know what I mean

chubertdev · 2014-02-13 Reply Admin

a; er:
....the real skill would be finding potential defects that you didn't think of....if you know what I mean

as we know, there are known knowns; there are things we know that we know. there are known unknowns; that is to say, there are things that we now know we don't know. but there are also unknown unknowns – there are things we do not know we don't know.

D-Coder · 2014-02-13 Reply Admin

chubertdev:
a; er:
....the real skill would be finding potential defects that you didn't think of....if you know what I mean

as we know, there are known knowns; there are things we know that we know. there are known unknowns; that is to say, there are things that we now know we don't know. but there are also unknown unknowns – there are things we do not know we don't know.

Aren't there also unknown knowns? Things that we don't realize that we know, but we do?

Hmmm...

chubertdev · 2014-02-13 Reply Admin

D-Coder:
chubertdev:
a; er:
....the real skill would be finding potential defects that you didn't think of....if you know what I mean

as we know, there are known knowns; there are things we know that we know. there are known unknowns; that is to say, there are things that we now know we don't know. but there are also unknown unknowns – there are things we do not know we don't know.
Aren't there also unknown knowns? Things that we don't realize that we know, but we do?

Hmmm...

Yes, but I wasn't aware that I knew that.

2014-02-13 Reply Admin

chubertdev:
D-Coder:
chubertdev:
a; er:
....the real skill would be finding potential defects that you didn't think of....if you know what I mean

as we know, there are known knowns; there are things we know that we know. there are known unknowns; that is to say, there are things that we now know we don't know. but there are also unknown unknowns – there are things we do not know we don't know.
Aren't there also unknown knowns? Things that we don't realize that we know, but we do?

Hmmm...

Yes, but I wasn't aware that I knew that.

Ladies and Gentlemen....we have a winner!

2014-02-13 Reply Admin

Watchman:
Who tests the test code?

The tester's tests test the testee's code, while the testee's code tests the tester's tests.

Of course continual testing can make testees testy. Peer programming can make this effect worse, as you can end up with testy testees clashing against each other like a Newton's cradle with 2 balls.

2014-02-13 Reply Admin

Fly:
I have seen similar naming conventions - but if I apply it to this it means "TestCase 119a" - so you are supposed to look into the document describing testcases. Still bad code, but refering to a TestCase is not too bad in my book... it forces ppl to read them.

hahahahahahah

aaahhhahahahahahahahaha

2014-02-13 Reply Admin

Jimmy:
ObiWayneKenobi:
Correct me if I'm wrong but... a test case should assert that something occurred. If you just let it run and the pass condition is "No errors" then you aren't really testing anything that you can verify, because you aren't checking that Foo was changed to Bar or whatever you expect to happen.
Yes, I agree, this. Test cases are generally looking for two things: 1) Something happened 2) Nothing broke
Nothing broke is easy - that just means we didn't throw a whole bunch of exception.

No exceptions were thrown when Windows 95 pretended to write my backups, but everything broke.

Jimmy:
Something happened can be trickier, but each method/function/subroutine has a purpose, and we need to make sure that it is fulfilling that purpose.

Each time Windows 95 wrote a file, the latest file that it wrote was written, usually. When the file was written in the wrong place, stuff that had previously been written that belonged in that place was no longer there. So the "Something happened" would appear to pass (until a later file obliterates this one). "Nothing broke" is a lot harder. "Nothing broke" needs a lot more testing.

2014-02-14 Reply Admin

anonymous:
TGV:
anonymous:
Watchman:
Who tests the test code?
It's testers all the way down.
No, at the end there are four elephants on the back of a turtle. Everyone knows that. Here, take a blue pill.
What is the turtle standing on?

Oh you fool -- it's not standing on anything! It's swimming!

TC119a()

Leave a comment on “TC119a()”