Two different ways to create bad logic in unit tests

My blog has moved. You can view this post at the following address: http://www.osherove.com/blog/2010/8/14/two-different-ways-to-create-bad-logic-in-unit-tests.html
Published Saturday, August 14, 2010 7:41 AM by RoyOsherove
Filed under:

Comments

Saturday, August 14, 2010 8:00 AM by Ben Hall

# re: Two different ways to create bad logic in unit tests

Interesting, so how would you recommend fixing this test?

Ben

@Ben_Hall

Saturday, August 14, 2010 11:22 AM by Emilio

# re: Two different ways to create bad logic in unit tests

When u talk about create manual mocks an stubs, do you mean create an entire class that "emulate" the behavior of a given class because this is a third party sealed class for example? Or do u mean to set the behavior of a given class with some mock framewrk like typemock, (in typemock's case you set the outcome with a method named "will return" that missused could lead to potential test issues)

Saturday, August 14, 2010 12:07 PM by dennisgorelik

# re: Two different ways to create bad logic in unit tests

"for example"?

It looks you forgot to cut&paste the example.

Saturday, August 14, 2010 12:11 PM by Mark

# re: Two different ways to create bad logic in unit tests

Totally agree.  This is something I always point out in code reviews because it's very, very bad!

It also makes the test code less declarative -- instead of just saying what the result is meant to be, you are also saying _how_ to get the result.

Sunday, August 15, 2010 7:28 AM by Mark Seemann

# re: Two different ways to create bad logic in unit tests

I can't really agree with this rule. While I do understand the reasoning, I don't agree with the conclusion. Shying away from specifying domain logic in the test leads to magic numbers or strings that may seem unrelated to each other.

That leads to Obscure Tests.

I'm not saying that a test should reproduce a complete algorithm because that could obviously lead to bugs in the tests, but the key is to chop up complex business rules into constituent parts and only test one part per test.

I wrote about this overall principle a while back: blog.ploeh.dk/.../DerivedValuesEnsureExecutableSpecification.aspx

As a rule of thumb, I find Derived Values much more communicative than magic values. Not always, but most of the time.

Sunday, August 15, 2010 7:37 AM by RoyOsherove

# re: Two different ways to create bad logic in unit tests

Mark:

that's where the name of the test, as well as other factors (how simple is the test, how simple is the input\expected output) come in to play.

for example, you could name the expected 42 value in your post to a variable name that explains what the result means.

Sunday, August 15, 2010 9:04 AM by smnbss

# re: Two different ways to create bad logic in unit tests

@Emilio

remove line 8 and chane line 9 to

Assert.AreEqual("hello user",result);

in this way you remove the logic bit from the test

Sunday, August 15, 2010 3:42 PM by Mark Seemann

# re: Two different ways to create bad logic in unit tests

Well, I never liked those rigid test name rules. Things change and stuff gets refactored and suddently, the class or method under test gets renamed, yet the test name remains the same.

For that reason I consider the test code the primary specification, and the name as secondary. I still prefer helpful test names, but I don't rely on them.

Monday, August 16, 2010 3:38 AM by Paul

# re: Two different ways to create bad logic in unit tests

@smnbss:  In your example, you would need two or more tests just to make sure the code didn't always return "Hello User" rather than the paramter.  Would the following be better

var stringPrefix = "Hello ";

Assert.IsTrue(result.StartsWith(stringPrefix, result);

Assert.IsTrue(result.EndsWith(string.Format(" {0}", user), result);

Assert.AreEqual(stringPrefix.Length + userName.Length, result.Length);

Wednesday, August 18, 2010 11:19 AM by Robi.Y

# re: Two different ways to create bad logic in unit tests

Paul: Or just add another test that calls:

> cut.SayHello("bill");

and asserts again without logic