3

I read somewhere that each test must test only one thing. But is grouping similar behavior allowed in the good practices handbook? I'm currently writing some tests (C# with NUnit) and bellow is an example of what I'm facing:

[TearDown]
public void Cleanup()
{
    Hotkeys.UnregisterAllLocals();
    Hotkeys.UnregisterAllGlobals();
}

[Test]
public void KeyOrderDoesNotMatter()
{
    Hotkeys.RegisterGlobal("Ctrl+Alt+P", delegate { });
    Assert.That(Hotkeys.IsRegisteredGlobal("Alt+P+Ctrl"), Is.True);
}

[Test]
public void KeyCaseDoesNotMatter()
{
    Hotkeys.RegisterGlobal("Ctrl+Alt+P", delegate { });
    Assert.That(Hotkeys.IsRegisteredGlobal("ctrl+alt+p"), Is.True);
}

[Test]
public void KeySpacesDoesNotMatter()
{
    Hotkeys.RegisterGlobal("Ctrl+Alt+P", delegate { });
    Assert.That(Hotkeys.IsRegisteredGlobal("Ctrl + Alt + P"), Is.True);
}

Grouped, they would become:

[TearDown]
public void Cleanup()
{
    Hotkeys.UnregisterAllLocals();
    Hotkeys.UnregisterAllGlobals();
}

[Test]
public void KeyIsNotStrict()
{
    // order
    Hotkeys.RegisterGlobal("Ctrl+Alt+A", delegate { });
    Assert.That(Hotkeys.IsRegisteredGlobal("Alt+A+Ctrl"), Is.True);

    // whitespace
    Hotkeys.RegisterGlobal("Ctrl+Alt+B", delegate { });
    Assert.That(Hotkeys.IsRegisteredGlobal("Ctrl + Alt + B"), Is.True);

    // case
    Hotkeys.RegisterGlobal("Ctrl+Alt+C", delegate { });
    Assert.That(Hotkeys.IsRegisteredGlobal("ctrl+alt+c"), Is.True);
}

So what is the best practice (if one exists) and why?

obs: i'm relatively new to unit testing...

2
  • You can use Assert.IsTrue Commented Jan 13, 2013 at 20:22
  • @lazyberezovsky I know, that's just the pattern I chose...because it gets more readable in more complex assertions.
    – Hugo Mota
    Commented Jan 13, 2013 at 20:24

2 Answers 2

6

The short answer is no. You should make you tests as simple as possible. each test should tests only one thing.

There is an Arrange-Act-Assert (AAA) pattern for unit-testing. Which states that you should do some preparation in the beginning of the test method (Arrange), then make action this test checks and make some assertions at the end of the method.

Also I may recommend you to read about FIRST pattern for unit-testing.

UPD:

When you make you tests complex - it's difficult to recognize what's wrong when test becomes "red", you know that some assert failed, but you have to read logs to understand which one. Moreover - if the first assert in your complex test fails you even don't know whether the rest asserts OK or not. It's also difficult to maintain big unit-tests, some work you done for the first assertion may have side-effects which affect next assertions.

But you should take into consideration that you tests with respect to GRASP also should be low-coupling / high-cohesion, so, as @Schwern mentioned in his answer, you should not write separated tests just to minimize assertions if that different tests will end up testing the same logical thing. It's always at the discretion of developer to make a decision which way is right in each particular case.

6
  • may i ask for the long one?
    – Hugo Mota
    Commented Jan 13, 2013 at 20:03
  • thank you! that pretty much clarified things for me. also, AAA is awesome (:
    – Hugo Mota
    Commented Jan 13, 2013 at 20:12
  • 1
    I'd never seen AAA nor FIRST before, good stuff. Could you link to GRASP please?
    – Schwern
    Commented Jan 13, 2013 at 20:25
  • @Schwern of course. Updated.
    – oxilumin
    Commented Jan 13, 2013 at 20:36
  • 1
    @hugo_leonardo You'd create a temp directory (so you know its empty), create the file and then check it exists. Better using temp directories and files anyway, no risk of name clashes if tests are run in parallel. But you can't always make things like AAA work, they are guidelines not rules.
    – Schwern
    Commented Jan 14, 2013 at 10:53
5

Note: I am not a C# programmer.

On the one hand, you've got "do only one thing in a test". On the other you have the DRY Principle. You're being asked which to violate. This depends on how badly you're violating each of them, how much benefit you get out of the violation, and why those rules exist in the first place.

Your grouped solution is not ideal, because it still repeats itself. If instead you did this...

[TearDown]
public void Cleanup()
{
    Hotkeys.UnregisterAllLocals();
    Hotkeys.UnregisterAllGlobals();
}

[Test]
public void IsRegisteredGlobal_InputNormalization()
{
    Hotkeys.RegisterGlobal("Ctrl+Alt+P", delegate { });

    Assert.IsTrue(Hotkeys.IsRegisteredGlobal("Alt+P+Ctrl"),     "order independent");
    Assert.IsTrue(Hotkeys.IsRegisteredGlobal("ctrl+alt+p"),     "case insensitive");
    Assert.IsTrue(Hotkeys.IsRegisteredGlobal("Ctrl + Alt + P"), "whitespace independent");
}

Then you're not violating DRY and you're hardly scuffing "do only one thing in a test". Its still doing one "thing" and that thing is normalizing the input of IsRegisteredGlobal.

You do just one thing per test in order to isolate them. This makes tests easier to isolate and debug. This doesn't mean one assert per test. The test above is still doing only one thing, but it is testing it in three very, very similar ways. This is ok. Previously your asserts were explained by the test name. Now they are explained by a message associated with each assert. The reason for failure remains obvious, the I in FIRST.

Furthermore, if you were to write all your tests with one assert per method and repeat the same code over and over unnecessarily, you not just violate DRY but the repeated code may start to slow things down violating the F in FIRST.

Is it possible that checking Hotkeys.IsRegisteredGlobal("Alt+P+Ctrl") might cause Hotkeys.IsRegisteredGlobal("ctrl+alt+p") to become true when if you reversed their order it wouldn't be? Yes. But there's always a trade off between isolation, speed and convenience. If you suspect one might interfere with another, you should isolate them. I'd say that if you wanted to check there's no coupling between then you should do that explicitly, in its own test, rather than saddling every test with that burden.

Yes, its fine to run code and do multiple asserts on it, but always remember it is a balancing act between good code and good tests. Usually the testing wins, but don't get silly about it.

I switched it to IsTrue rather than the generic That for compactness, clarity and potentially better failure diagnostics. Assert.That( thing, condition ) means you don't know what you're testing for until the end. You have to read the whole line, see what the condition is, and read the whole line again in light of what you're really testing for. Assert.IsTrue tells you up front. Assert.That may be more readable in the complex assertions, but its less readable in the simple ones. It's ok to use them as appropriate. (Note: Perl programmer)

It can also potentially produce better failure diagnostics because you're giving NUnit more information about your intent. It's not "this matches that" but "this is true" so it can produce a more exact and crafted assert. Though NUnit might also be smart enough to see that you're testing against Is.True and do that anyway. It doesn't hurt.

4
  • wow! I was about to accept the other answer but you have a pretty good point here sir!
    – Hugo Mota
    Commented Jan 13, 2013 at 20:17
  • Assert.IsTrue actually uses the constraint model (Is.True) underneath :p ~ I did a research at stack for this "versus-issue" weeks ago and the conclusion was that it doesn't quite matter ^^ (what matters is chosing one and sticking with it)
    – Hugo Mota
    Commented Jan 13, 2013 at 20:50
  • I don't agree that "choosing one and sticking with it" matters, but again, I'm a Perl programmer and consistency is a means, not an end. Consistency here loses you some clarity, which is an end. Furthermore, if Assert.IsTrue(foo) is just a wrapper around Assert.That(foo, IsTrue) it doesn't mean it can't do something more interesting later. You are violating encapsulation with that assumption. Finally, the goal of communicating your intent to the reader in a clear and simple manner remains. Of course, this is all diminishing returns.
    – Schwern
    Commented Jan 13, 2013 at 21:39
  • I think i gain clarity sticking with a pattern than using two of them (considering the two approaches acomplish the same). Probably just a matter of opinion...i don't think this discussion is stackoverlflow material :p
    – Hugo Mota
    Commented Jan 13, 2013 at 21:43

Not the answer you're looking for? Browse other questions tagged or ask your own question.