2
\$\begingroup\$

I have tests for simple POJO class ProgressableItem (i.e. the one which has attributes total work and work done).

I use JUnit 3 (it's from Android application).

public class ProgressableItemTest extends AndroidTestCase {
    private ProgressableItem item;

    @Override
    protected void setUp() throws Exception {
        super.setUp();
        item = new ProgressableItem("item", "test item", 0, 100);
    }


    public void testNotEquals() {
        ProgressableItem itemWithDifferentName = copyOf(item);
        itemWithDifferentName.setName("Not the same " + item.getName());
        assertThat(itemWithDifferentName, not(item));
        ProgressableItem itemWithDifferentDescription = copyOf(item);
        itemWithDifferentName.setDescription("Not the same " + item.getDescription());
        assertThat(itemWithDifferentDescription, not(item));
        ProgressableItem itemWithDifferentProgress = copyOf(item);
        itemWithDifferentName.setProgress(item.getProgress() + 1);
        assertThat(itemWithDifferentProgress, not(item));
        ProgressableItem itemWithDifferentMax = copyOf(item);
        itemWithDifferentName.setMax(item.getMax() + 1);
        assertThat(itemWithDifferentMax, not(item));
    }

    private static ProgressableItem copyOf(ProgressableItem item) {
        return new ProgressableItem(item.getName(), item.getDescription(), item.getProgress(), item.getMax());
    }

}


public interface Handler<T> {
    void handle(T obj);
}

Then I modified it into:

public void testNotEquals() {
    for (Handler<ProgressableItem> progressableItemModifier : PROGRESSABLE_ITEM_MODIFIERS) {
        ProgressableItem differentItem = copyOf(item);
        progressableItemModifier.handle(differentItem);
        assertThat(differentItem, not(item));
    }
}

private static final Collection<? extends Handler<ProgressableItem>>
        PROGRESSABLE_ITEM_MODIFIERS = Arrays.asList(
                new Handler<ProgressableItem>() {
                    @Override
                    public void handle(ProgressableItem obj) {
                        obj.setName("Not the same " + obj.getName());
                    }
                },
                new Handler<ProgressableItem>() {
                    @Override
                    public void handle(ProgressableItem obj) {
                        obj.setDescription("Not the same " + obj.getDescription());
                    }
                },
                new Handler<ProgressableItem>() {
                    @Override
                    public void handle(ProgressableItem obj) {
                        obj.setProgress(obj.getProgress() + 1);
                    }
                },
                new Handler<ProgressableItem>() {
                    @Override
                    public void handle(ProgressableItem obj) {
                        obj.setMax(obj.getMax() + 1);
                    }
                }
);

But now I have many more lines of code.

Which of the two options is better? Should I separate the test method into 4 small methods? Is there no need to check each case?

\$\endgroup\$
1
  • 3
    \$\begingroup\$ Loops inside unit tests are not done. You want one test for each scenario, otherwise when one test fails, you're left to guess which one it was. \$\endgroup\$ Dec 11, 2014 at 21:48

1 Answer 1

2
\$\begingroup\$

A couple of tips for unit testing:

  • One test case per subject: so you should definitely split the cases
  • Ideally no logic at all: avoid loops
  • Duplication is acceptable: clarity is most important

I would write this way:

public void testNotEquals_IfNameChanged() {
    ProgressableItem itemWithDifferentName = copyOf(item);
    itemWithDifferentName.setName("Not the same " + item.getName());
    assertThat(itemWithDifferentName, not(item));
}

public void testNotEquals_IfDescriptionChanged() {
    ProgressableItem itemWithDifferentDescription = copyOf(item);
    itemWithDifferentName.setDescription("Not the same " + item.getDescription());
    assertThat(itemWithDifferentDescription, not(item));
}

public void testNotEquals_IfProgressChanged() {
    ProgressableItem itemWithDifferentProgress = copyOf(item);
    itemWithDifferentName.setProgress(item.getProgress() + 1);
    assertThat(itemWithDifferentProgress, not(item));
}

public void testNotEquals_IfMaxChanged() {
    ProgressableItem itemWithDifferentMax = copyOf(item);
    itemWithDifferentName.setMax(item.getMax() + 1);
    assertThat(itemWithDifferentMax, not(item));
}

Since you never change item, you don't really need the setUp method:

private final ProgressableItem item = new ProgressableItem("item", "test item", 0, 100);

Of course just because it's final nothing prevents you from mutating it.

To be extra safe and stay efficient, you could create an unmodifiable ProgressableItem, for example:

private final ProgressableItem item = new ProgressableItem("item", "test item", 0, 100) {
    @Override
    public void setName(String name) {
        throw new UnsupportedOperationException();
    }
    // ... + the other setters too
};
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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