How to use correctly assertThrows on set of values?

341 Views Asked by At

I've been learning Java just for a bit, so please advise how exception throwing test should look like in this case? I have following Gambling Machine Class. And then 2 tests for it. I do not really know what should follow the "Integer" in second method (shouldThrowWhenNumbersOutOfRange). Could you please advise as to the exact syntax?


public class GamblingMachine {

    public int howManyWins(Set<Integer> userNumbers) throws InvalidNumbersException {
        validateNumbers(userNumbers);
        Set<Integer> computerNumbers = generateComputerNumbers();
        int count = 0;
        for (Integer number : userNumbers) {
            if (computerNumbers.contains(number)) {
                count++;
            }
        }
        return count;
    }

    private void validateNumbers(Set<Integer> numbers) throws InvalidNumbersException {
        if (numbers.size() != 6) {
            throw new InvalidNumbersException();
        }

        if (numbers.stream().anyMatch(number -> number < 1 || number > 49)) {   //anyMatch-function to check whether any element in list satisfy given condition
            throw new InvalidNumbersException();
        }
    }

    private Set<Integer> generateComputerNumbers() {
        Set<Integer> numbers = new HashSet<>();
        Random generator = new Random();
        while(numbers.size() < 6) {
            numbers.add(generator.nextInt(49) + 1);
        }
        return numbers;
    }
}

 private GamblingMachine machine = new GamblingMachine();

    @ParameterizedTest
    @NullAndEmptySource
    public void shouldThrowWhenNumbersEmpty(Set<Integer> numbers) throws InvalidNumbersException {
        Assertions.assertThrows(NumberFormatException.class, () -> {
        Integer.parseInt(" ");
        });
    }

    @ParameterizedTest
    @CsvFileSource(resources ="/numbersOutOfRange.cvs", numLinesToSkip = 1)
    public void shouldThrowWhenNumbersOutOfRange(Set<Integer> numbers) throws InvalidNumbersException {
      Assertions.assertThrows(NumberFormatException.class, () -> {
            Integer.   //how code should look like here?
        });

    }
1

There are 1 best solutions below

0
On

The point of a test is to, you know, test something. Your shouldThrowWhenNumbersEmpty test doesn't do that (well, it tests that Integer.parseInt(" ") throws something. It does, of course. You... don't have to test the core libraries).

In other words, your gambling machine tests need to be calling some stuff from your GamblingMachine class. The idea is to test GamblingMachine. Not to test Integer.parseInt.

It's also a bizarre test: Why in the blazes is shouldThrowWhenNumbersEmpty parameterized? I assume the point of that test is: "Ensure that the gambling machine works as designed when passing an empty set of numbers in, specifically, the part of the design that states that an InvalidNumbersException is thrown if you do that".

Which is done with something like:

@Test
public void shouldThrowWhenNumbersEmpty() {
    Assertions.assertThrows(InvalidNumbersException.class, () -> {
        Set<Integer> empty = Set.of();
        machine.howManyWins(empty);
    });
}

Parameterized tests are a fairly exotic concept. Your test setup appears to be falling into a trap: It appears to be set up that you repeat all the logic that is already in your gamblingmachine class, to then apply this logic to the incoming (parameterized) data, figure out what your gambling machine ought to be doing, and then double check its work.

That's not how you should write tests. Tests focus on a specific result. Parameterized tests can make sense, but only if the stuff you have to do for any given input is roughly the same. For example:

Good use of parameterized testing

You have a csv file containing a bunch of lines, each of which has 6 rolls + the correct answer. Your parameterized test treats each line the same: Call howManyWins using the 6 rolls as input, then check that howManyWins returns the expected value.

Bad use of parameterized testing

You have a csv file containing a bunch of lines, each of which has 6 rolls. Your parameterized test will calculate the right result for the rolls, then invoke gambling machine, and check that the gambling machine gives the same answer as what you calculated.

This is bad: You're just repeating the code. It also means your test code is itself doing more than the very basics (it's doing a bunch of business logic), thus raising the question: Who tests your test, then?

Both of your test methods seem like they should NOT be parameterized, unless that csv also contains results.