minizibi
minizibi

Reputation: 671

Unit Tests name convention for grouping tests

I read some articles about tests naming conventions and decided to use one with "should". It works pretty nice in most cases like:

But I encountered problems while testing DecimalRepresentation class which displays numbers in diffrent numeral systems, just look at code:

public class DecimalRepresentationTest {

    private DecimalRepresentation decimal;

    @BeforeEach
    void setup() {
        decimal = new DecimalRepresentation();
    }

    @Test
    void shouldReturnZeroIfNumberNotSpecified() {
        assertEquals("0", decimal.toBinary());
    }

    @Test
    void shouldReturn10IfNumber2() {
        decimal.setNumber(2);
        assertEquals("10", decimal.toBinary());
    }

    @Test
    void shouldReturn1111IfNumber15() {
        decimal.setNumber(15);
        assertEquals("1111", decimal.toBinary());
    }
}

Now it's not bad, but in case I'm testing negative inputs it looks terrible:

    @Test
    void shouldReturn11111111111111111111111110001000IfNumberNegative120() {
        decimal.setNumber(-120);
        assertEquals("11111111111111111111111110001000", decimal.toBinary());
    }

    @Test
    void shouldReturn11111111111111111111111111111111IfNumberNegative1() {
        decimal.setNumber(-1);
        assertEquals("11111111111111111111111111111111", decimal.toBinary());
    }

In examples above I'm testing twice for positive and negative input to be sure there is no hardcoded result and algorithm works fine so i decided to group tests in nested classes for keeping convention:

@Nested
@DisplayName("Tests for positive numbers")
class PositiveConverter {
    @Test
    void shouldReturn10IfNumber2() {
        decimal.setNumber(2);
        assertEquals("10", decimal.toBinary());
    }

    @Test
    void shouldReturn1111IfNumber15() {
        decimal.setNumber(15);
        assertEquals("1111", decimal.toBinary());

    }
}

@Nested
@DisplayName("Tests for negative numbers")
class NegativeConverter {
    @Test
    void shouldReturn11111111111111111111111110001000IfNumberNegative120() {
        decimal.setNumber(-120);
        assertEquals("11111111111111111111111110001000", decimal.toBinary());
    }

    @Test
    void shouldReturn11111111111111111111111111111111IfNumberNegative1() {
        decimal.setNumber(-1);
        assertEquals("11111111111111111111111111111111", decimal.toBinary());
    }
}

I realize it's overcomplicated because of convention. In case I make lapse it could look much better:

@Test
void testPositiveConversions() {
    assertAll(
            () -> {decimal.setNumber(2); assertEquals("10", decimal.toBinary());},
            () -> {decimal.setNumber(15); assertEquals("1111", decimal.toBinary());}
    );
}

@Test
void testNegativeConversions() {
    assertAll(
            () -> {decimal.setNumber(-120); assertEquals("11111111111111111111111110001000", decimal.toBinary());},
            () -> {decimal.setNumber(-1); assertEquals("11111111111111111111111111111111", decimal.toBinary());}
    );
} 

Should i break convention to keep it simple? The same naming problem i have with tests they get Lists with input and outputs or dynamic tests:

@TestFactory
Stream<DynamicTest> shouldReturnGoodResultsForPositiveNumbers(){ // look at method name lol
    List<Integer> inputs = new ArrayList<>(Arrays.asList(2, 15));
    List<String> outputs = new ArrayList<>(Arrays.asList("10", "1111"));

    return inputs.stream().map(number -> DynamicTest.dynamicTest("Test positive " + number, () -> {
        int idx = inputs.indexOf(number);
        decimal.setNumber(inputs.get(idx));
        assertEquals(outputs.get(idx), decimal.toBinary());
    }));
}

Upvotes: 2

Views: 7958

Answers (2)

Ryhan
Ryhan

Reputation: 1885

i've modeled mine after Roy Osherove's method, here's the regex

^(setup|teardown|([A-Z]{1}[0-9a-z]+)+_([A-Z0-9]+[0-9a-z]+)+_([A-Z0-9]+[0-9a-z]+)+)$

Upvotes: 0

GhostCat
GhostCat

Reputation: 140631

Names are supposed to be helpful. Sometimes rules help finding good names, sometimes, they do not. And then the answer is to drop the rule, and maybe go for something completely different, like:

@Test
void testResultForNegativeInput() {
  decimal.setNumber(-120);
  assertEquals("11111111111111111111111110001000", decimal.toBinary());
 }

And if you have several of these methods, maybe adding "ForMinus120" or so would be acceptable.

But instead of spending energy naming here: the real issue is that you are using the wrong kind of testing: you have a whole bunch of input data, that simply result in different output values to check. All your tests are about: one special input value should lead to a special output value.

You don't do that with many almost similar test methods - instead you turn to parameterized tests! Meaning: use a table to drive your test. For JUnit5 and parameterized tests turn here (thanks to user Sam Brannen).

It is great that you spend time and energy to make your tests easy to read. But in this case, that leads to a lot of code duplication. Instead, put down the input/output values into a table, and have one test to check all entries in that table.

Upvotes: 5

Related Questions