Random letter test

Question

My first test (randomUpperCaseLetter) checks if the returned random letter is an uppercase ASCII one. The second one (compareCharactersWithAsciiCodes) is just me trying to understand Java.

package com.company.letter;

import com.company.RandomLetter;

import org.junit.Test;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;

public class RandomLetterTest {
    @Test
    public void randomUpperCaseLetter() {
        char letter = new RandomLetter().generate();
        int upperCaseAAsciiCode = 65;
        int upperCaseZAsciiCode = 90;
        assertTrue(letter >= upperCaseAAsciiCode && letter <= upperCaseZAsciiCode);
    }

    @Test
    public void compareCharactersWithAsciiCodes() {
        assertEquals(65, 'A');
        assertEquals(90, 'Z');
        assertEquals(76, 'L');
        assertEquals(77, 'M');
        assertEquals(78, 'N');
    }
}

Is the first test valuable enough to keep it? What about the second one? Uncle Bob say something about tests to understand how the language works. I don't remember when/where.

Do you have suggestions to improve its readability?

Answer 1

Without having access to the class RandomLetter let's see what we can deduce from your tests.

@Test
public void randomUpperCaseLetter() {
    char letter = new RandomLetter().generate();
    int upperCaseAAsciiCode = 65;
    int upperCaseZAsciiCode = 90;
    assertTrue(letter >= upperCaseAAsciiCode && letter <= upperCaseZAsciiCode);
}

The name of the test looks good to me; we sure do test that an uppercase letter has been generated. The thing is we don't test if it's actually random. You're producing only one letter, checking if it's in the range of uppercase letters, and then be done with it. We've never actually checked that the letters that are produced are different/random every time. How can you be sure that it's random?

@Test
public void compareCharactersWithAsciiCodes() {
    assertEquals(65, 'A');
    assertEquals(90, 'Z');
    assertEquals(76, 'L');
    assertEquals(77, 'M');
    assertEquals(78, 'N');
}

The name is still good to me, we are indeed comparing char with ASCII codes. The thing is, it's not testing anything related to RandomLetter. It's just testing if in Java the char A is equal to 65; and unless the language changes, your test is not really useful to test the class.

So what can you do? First, I'm glad you have unit tests! This is one of the things I would like to see more often when someone is learning a language. Second, try to cover every bit of your code by asking (but not limited to):

• Did you test all the if else of your code?
• Did you try to test that generate() gives some different letters each time?
• Did you test all the public methods ? Did you test some corner/edge cases of your code?

There are tools to help you see what is the coverage of your tests, but you can begin with manually checking what your tests cover.

Answer 2

randomUpperCaseLetter()

I will assume that RandomLetter.generate() actually does return something randomly* generated. (Maybe it should be named generateUpperCase() instead?)

Unit tests should be deterministic. Without changing your code they should run the same every time. Let's say you run this test and it returns a valid 'A' and you think it works, well, what if it's possible it could also return 'a'? So you set up a loop to run it a few times, say 100.

@Test
public void randomUpperCaseLetter() {
    int upperCaseAAsciiCode = 65;
    int upperCaseZAsciiCode = 90;
    for (int i = 0; i < 100; i++) {
        char letter = RandomLetter.generate();
        assertTrue(letter >= upperCaseAAsciiCode && letter <= upperCaseZAsciiCode);
    }
}

If the code is actually random* then you can't be certain it is correct like this. Yes, you can be more certain like this, but it's still not really perfect. Even if you do it until it gives you every upper case letter, you can't do it until it doesn't give you every other character, that is a paradox.

Edit: Here is an answer on the programming stackexchange site with a lot of goo information on unit testing randomness.

All that said, in practice I think it's better than nothing to have something like this and have it run a high number of times (keeping it under a second or so execution time) to be more certain but it just doesn't feel right.

*: Pseudo-random, whatever, that's out of the scope of the question I think.

compareCharactersWithAsciiCodes()

There's nothing wrong with making snippets like this to learn something in the same way there's nothing wrong with making a main method in a new Example class to try something, but it's not the sort of thing I would check in to a repository.

This isn't checking your code for correct behavior, it's checking that the JVM's specification is correct, which isn't the point of unit testing. It's safe to assume (in a unit test) that the specification of the language works as intended.

Answer 3

The main thing I've noticed is the usage of int type to represent characters. Characters are stored in the memory as numbers but as for testing I would expect to see char in its human readable form. For example:

public void randomUpperCaseLetter() {
    char letter = new RandomLetter().generate();
    int upperCaseAAsciiCode = 65;
    int upperCaseZAsciiCode = 90;
    assertTrue(letter >= upperCaseAAsciiCode && letter <= upperCaseZAsciiCode);
}

should be written for clarity as

public void randomUpperCaseLetter() {
    char letter = new RandomLetter().generate();
    assertTrue(letter >= 'A' && letter <= 'Z' );
}

To better test the generating letter functionality, I would call generate method multiple times (possibly in a loop) to generate more letters.

For the second test method, it seems to be unnecessary. The letter 'A' will always have ASCII character code 65 and there is no need to test it.

public void compareCharactersWithAsciiCodes() {
    assertEquals(65, 'A');
    :
}