HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Understanding my permalink service tests

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
understandingservicetestspermalink

Problem

I started writing unit tests cases recently. For now, I created only "perfect case" where there is no error. However, the test seems to me difficult to maintain and difficult to understand.

How can I make it better for other people to understand?

All permalink attributes are set within the service itself I'm testing (PermalinkService.java).

My test

```
@RunWith(SpringJUnit4ClassRunner.class)
@ContextConfiguration(classes = Configuration.class)
@FixMethodOrder(MethodSorters.NAME_ASCENDING)

public class PermalinkServiceTest {

@Autowired
PermalinkService permalinkService;

private static Permalink PERMALINK;
private static Validacao VALIDATION;
private static DBObject PECA;
private static BasicDBObject COMMENTS;

@BeforeClass
public static void init(){
PERMALINK = new Permalink();
VALIDATION = new Validacao().setId("55534e63ccf2f879efcbd2a3");
}

@Test
public void testPermalinkServiceA_createPermalinkWithoutValidation_permalinkWithAllFilesAndWithoutValidation() throws Exception {
PERMALINK = permalinkService.createPermalink(VALIDATION.getId(), "Permalink sem validação");
Assert.notNull(PERMALINK);
}

@Test
public void testPermalinkServiceB_createPermalinkWithValidation_permalinkWithAllFilesAndValidation() throws Exception {
PERMALINK = permalinkService.createPermalink(VALIDATION.getId(), "Permalink com validação");
PECA = (DBObject) JSON.parse(PERMALINK.getPecas().get(0).toString());
Assert.notNull(PERMALINK);
}

@Test
public void testPermalinkServiceB_getAllPermalinks_getAllPermalinks() throws Exception {
Assert.notNull(permalinkService.getPermalinks(VALIDATION.getId()));
}

@Test
public void testPermalinkServiceC_getPermalink_getPermalink() throws Exception {
Assert.notNull(permalinkService.getPermalinks(VALIDATION.getId(), PERMALINK.getId()));
}

@Test
public void testPermalinkServiceD_setStatus_set

Solution

Naming

Good names are very important for readability. For example, this:

testPermalinkServiceA_createPermalinkWithoutValidation_permalinkWithAllFilesAndWithoutValidation


is just way too long a name.

You can reduce it's size by removing all references to permalinks (it's in the PermalinkServiceTest class, so it's assumed that the tests test permalinks). It's also not very clear what the A stands for, but it seems to be only necessary because the tests need to be executed in a specific order, which doesn't seem like a good idea

The name also contains more duplication, as it contains WithoutValidation twice.

A shorter name might be testCreateLinkAllFilesNoValidation, or even just allFilesNoValidation (if that accurately described what is tested, which I'm not sure it does; It's hard to say without the actual code, but I'm eg unsure what allFiles are, or how they are used in the test).

Tests building on each other

I don't really like how the tests seem to build on each other; I would prefer if each test is self-contained.

Self-contained tests seem a lot more stable and maintainable, and would also result in easier to understand code. If you are worried about duplication, just extract that code to a method.

This would also get rid of many of the fields, which I think also results in nicer code.

Misc

Many of your tests don't actually contain any tests, why is that?

Code Snippets

testPermalinkServiceA_createPermalinkWithoutValidation_permalinkWithAllFilesAndWithoutValidation

Context

StackExchange Code Review Q#90739, answer score: 4

Revisions (0)

No revisions yet.