JUnit testing for Calculator in Java

I made a simple calculator.

I would also like to write some simple unit tests for my CalculatorEngine class. I had to make some of my private methods as package-private over this. How is it usually better to do it - to enhance access modifier or to make inner test class?

And what another more complicated cases it's better to test?

My CalculatorEngineTest class is:

```
import org.junit.Before;
import org.junit.Test;

import static junit.framework.Assert.assertTrue;
import static junit.framework.TestCase.assertEquals;

public class CalculatorEngineTest {

CalculatorEngine calcEngine = new CalculatorEngine();

@Before
public void runBeforeEveryTest() {
calcEngine.currentTotal = 0;
}

@Test
public void testGetTotalStringInt() throws Exception {
calcEngine.currentTotal = 50;
assertTrue(calcEngine.currentTotal % 1.0 == 0);
}

@Test
public void testGetTotalStringDouble() throws Exception {
calcEngine.equal("50.5");
assertTrue(calcEngine.currentTotal % 1.0 != 0);
}

@Test
public void testEqual() throws Exception {
calcEngine.equal("20");
assertEquals(calcEngine.currentTotal, 20.0);
}

@Test
public void testAdd() throws Exception {
calcEngine.add(20);
assertEquals(calcEngine.currentTotal, 20.0);

}

@Test
public void testSubtract() throws Exception {
calcEngine.subtract(20);
assertEquals(calcEngine.currentTotal, -20.0);
}

@Test
public void testMultiplyByZero() throws Exception {
calcEngine.multiply(10);
assertEquals(calcEngine.currentTotal, 0.0);
}

@Test
public void testDivideByZero() throws Exception {
calcEngine.divide(10);
assertEquals(calcEngine.currentTotal, 0.0);
}

@Test
public void testMultiply() throws Exception {
calcEngine.add(10);
calcEngine.multiply(10);
assertEquals(calcEngine.currentTotal, 100.0);

Do you mean the default access modifier (i.e. none specified) by package-private? If you can offer your CalculatorEngine's code as another review then I'm sure some of us can give you good-quality advice there. As pointed out by OP, it's actually in this question: Simple calculator in Java using Swing and AWT. :)

Running down your list of tests...

First, why do all your tests throws Exception? Are they expected to? If they are, shouldn't you want JUnit to catch it and assert the right Exception is thrown? Your tests should be written in the minimal way possible, as you can also think of it as a form of documentation describing what your methods can or cannot do. If another developer (e.g. "You" six months down the road) sees the throws Exception, they may be mislead into thinking "an Exception may be thrown here... somehow.".

@Before
public void runBeforeEveryTest() {
    calcEngine.currentTotal = 0;
}

Your CalculatorEngine should probable have a reset() method for this. Manipulating a field directly in this way is prone to errors.

@Test
public void testGetTotalStringDouble() throws Exception {
    calcEngine.equal("50.5");
    assertTrue(calcEngine.currentTotal % 1.0 != 0);
}

So what does this do? Presumably, equal("50.5") sets currentTotal as 50.5, and we're testing that it's not an integer...? I wouldn't be worried about this. If the intention is to test whether equal() accepts a decimal-number-as-a-String, test that directly.

@Test
public void testMultiplyByZero() throws Exception {
    calcEngine.multiply(10);
    assertEquals(calcEngine.currentTotal, 0.0);
}

@Test
public void testDivideByZero() throws Exception {
    calcEngine.divide(10);
    assertEquals(calcEngine.currentTotal, 0.0);
}

The method names are misleading here, because the argument you use isn't zero, and your other method names do not read as e.g. addToZero(). Originally, I thought the division test was a division by zero test, and wanted to point out that doesn't give 0.0... Testing should involve edge cases too.

@Test
public void testMultiply() throws Exception {
    calcEngine.add(10);
    calcEngine.multiply(10);
    assertEquals(calcEngine.currentTotal, 100.0);
}

@Test
public void testDivide() throws Exception {
    calcEngine.add(10);
    calcEngine.divide(10);
    assertEquals(calcEngine.currentTotal, 1.0);
}

Ok, so now these are the 'normal' multiplication and division tests... It's still slightly odd that you have to do an 'add' first, which seems to be a merely convenient method (pun intended) of 'initializing' currentTotal as 10. You may want to look into this particular API design too.

StackExchange Code Review Q#93156, answer score: 3