2
\$\begingroup\$

I'm trying to get familiar with unit testing. I never implement TDD previously, but I believe it's the best way to write software.

So, here I'm trying to test my isLastTwoDates(position) method of my model class:

package se.abccompany.models;

import org.joda.time.DateTimeZone;
import org.joda.time.tz.UTCProvider;
import org.junit.Before;
import org.junit.Test;

import models.Traktamente;

import static com.google.common.truth.Truth.assertThat;

public class TraktamenteTest {

    private static final String ORIGIN = "Stockholm";
    private static final String DESTINATION = "Goteborg";
    private static final String COMMENTS = "Lorem ipsum dolor sit amet.";
    private final int CASE_1_REPORT_COUNT = 6;
    private final int CASE_2_REPORT_COUNT = 7;
    private final int CASE_3_REPORT_COUNT = 14;
    private final int CASE_4_REPORT_COUNT = 12;
    private final int CASE_5_REPORT_COUNT = 12;
    private final int CASE_6_REPORT_COUNT = 16;

    private Traktamente case1;
    private Traktamente case2;
    private Traktamente case3;
    private Traktamente case4;
    private Traktamente case5;
    private Traktamente case6;

    @Before
    public void setupOnce() {

        case1 = new Traktamente();
        case1.startTime = "13:00";
        case1.startDate = "2019-01-07"; // Måndag
        case1.endTime = "21:00";
        case1.endDate = "2019-01-10"; // Torsdag
        case1.origin = ORIGIN;
        case1.destination = DESTINATION;
        case1.switchEgetBoendeState = true;
        case1.switchNinetyDaysState = true;
        case1.comments = COMMENTS;

        case2 = new Traktamente();
        case2.startTime = "20:00";
        case2.startDate = "2019-01-06"; // Söndag
        case2.endTime = "21:00";
        case2.endDate = "2019-01-10"; // Torsdag
        case2.origin = ORIGIN;
        case2.destination = DESTINATION;
        case2.switchEgetBoendeState = true;
        case2.switchNinetyDaysState = true;
        case2.comments = COMMENTS;

        case3 = new Traktamente();
        case3.startTime = "08:00";
        case3.startDate = "2019-01-07"; // Måndag
        case3.endTime = "17:00";
        case3.endDate = "2019-01-18"; // Fredag
        case3.origin = ORIGIN;
        case3.destination = DESTINATION;
        case3.switchEgetBoendeState = true;
        case3.switchNinetyDaysState = true;
        case3.comments = COMMENTS;

        case4 = new Traktamente();
        case4.startTime = "08:00";
        case4.startDate = "2019-01-07"; // Måndag
        case4.endTime = "17:00";
        case4.endDate = "2019-01-16"; // Onsdag
        case4.origin = ORIGIN;
        case4.destination = DESTINATION;
        case4.switchEgetBoendeState = true;
        case4.switchNinetyDaysState = true;
        case4.comments = COMMENTS;

        case5 = new Traktamente();
        case5.startTime = "08:00";
        case5.startDate = "2019-01-07"; // Måndag
        case5.endTime = "17:00";
        case5.endDate = "2019-01-16"; // Onsdag
        case5.origin = ORIGIN;
        case5.destination = DESTINATION;
        case5.switchEgetBoendeState = true;
        case5.switchNinetyDaysState = true;
        case5.comments = COMMENTS;

        case6 = new Traktamente();
        case6.startTime = "12:00";
        case6.startDate = "2019-01-04"; // Fredag
        case6.endTime = "17:00";
        case6.endDate = "2019-01-17"; // Torsdag
        case6.origin = ORIGIN;
        case6.destination = DESTINATION;
        case6.switchEgetBoendeState = true;
        case6.switchNinetyDaysState = true;
        case6.comments = COMMENTS;

        DateTimeZone.setProvider(new UTCProvider());

    }

    @Test
    public void generateReport_shouldAddTwoDatesAfterTheEndDate() {
        case1.generateReport();
        case2.generateReport();
        case3.generateReport();
        case4.generateReport();
        case5.generateReport();
        case6.generateReport();

        assertThat(case1.getDateArray().size()).isEqualTo(CASE_1_REPORT_COUNT);
        assertThat(case2.getDateArray().size()).isEqualTo(CASE_2_REPORT_COUNT);
        assertThat(case3.getDateArray().size()).isEqualTo(CASE_3_REPORT_COUNT);
        assertThat(case4.getDateArray().size()).isEqualTo(CASE_4_REPORT_COUNT);
        assertThat(case5.getDateArray().size()).isEqualTo(CASE_5_REPORT_COUNT);
        assertThat(case6.getDateArray().size()).isEqualTo(CASE_6_REPORT_COUNT);

        assertThat(case1.startDate.equals(case1.getDateArray().get(0).toString())).isTrue();
        assertThat(case2.startDate.equals(case2.getDateArray().get(0).toString())).isTrue();
        assertThat(case3.startDate.equals(case3.getDateArray().get(0).toString())).isTrue();
        assertThat(case4.startDate.equals(case4.getDateArray().get(0).toString())).isTrue();
        assertThat(case5.startDate.equals(case5.getDateArray().get(0).toString())).isTrue();
        assertThat(case6.startDate.equals(case6.getDateArray().get(0).toString())).isTrue();

        assertThat(case1.endDate.equals(case1.getDateArray().get(CASE_1_REPORT_COUNT - 3).toString())).isTrue();
        assertThat(case2.endDate.equals(case2.getDateArray().get(CASE_2_REPORT_COUNT - 3).toString())).isTrue();
        assertThat(case3.endDate.equals(case3.getDateArray().get(CASE_3_REPORT_COUNT - 3).toString())).isTrue();
        assertThat(case4.endDate.equals(case4.getDateArray().get(CASE_4_REPORT_COUNT - 3).toString())).isTrue();
        assertThat(case5.endDate.equals(case5.getDateArray().get(CASE_5_REPORT_COUNT - 3).toString())).isTrue();
        assertThat(case6.endDate.equals(case6.getDateArray().get(CASE_6_REPORT_COUNT - 3).toString())).isTrue();
    }

    @Test
    public void isEndDateOfTheTrip_shouldReturnsTrue() {
        case1.generateReport();
        assertThat(case1.isEndDateOfTheTrip(CASE_1_REPORT_COUNT-3)).isTrue();
        case2.generateReport();
        assertThat(case2.isEndDateOfTheTrip(CASE_2_REPORT_COUNT-3)).isTrue();
        case3.generateReport();
        assertThat(case3.isEndDateOfTheTrip(CASE_3_REPORT_COUNT-3)).isTrue();
        case4.generateReport();
        assertThat(case4.isEndDateOfTheTrip(CASE_4_REPORT_COUNT-3)).isTrue();
        case5.generateReport();
        assertThat(case5.isEndDateOfTheTrip(CASE_5_REPORT_COUNT-3)).isTrue();
        case6.generateReport();
        assertThat(case6.isEndDateOfTheTrip(CASE_6_REPORT_COUNT-3)).isTrue();
    }

    @Test
    public void isLastTwoDates_shouldReturnsCorrectValue() {
        case1.generateReport();
        assertThat(case1.getDateArray()).isNotNull();
        // Test array lower bound
        for (int position = 0; position < CASE_1_REPORT_COUNT - 2; position++) {
            assertThat(case1.isLastTwoDates(position)).isFalse();
        }
        for (int position = CASE_1_REPORT_COUNT - 2; position < CASE_1_REPORT_COUNT; position++) {
            assertThat(case1.isLastTwoDates(position)).isTrue();
        }
        // Test array upper bound
        assertThat(case1.isLastTwoDates(CASE_1_REPORT_COUNT)).isFalse();

        case3.generateReport();
        assertThat(case3.getDateArray()).isNotNull();
        for (int position = 0; position < CASE_3_REPORT_COUNT - 2; position++) {
            assertThat(case3.isLastTwoDates(position)).isFalse();
        }
        for (int position = CASE_3_REPORT_COUNT - 2; position < CASE_3_REPORT_COUNT; position++) {
            assertThat(case3.isLastTwoDates(position)).isTrue();
        }
        assertThat(case3.isLastTwoDates(CASE_3_REPORT_COUNT)).isFalse();

        case5.generateReport();
        assertThat(case5.getDateArray()).isNotNull();
        for (int position = 0; position < CASE_5_REPORT_COUNT - 2; position++) {
            assertThat(case5.isLastTwoDates(position)).isFalse();
        }
        for (int position = CASE_5_REPORT_COUNT - 2; position < CASE_5_REPORT_COUNT; position++) {
            assertThat(case5.isLastTwoDates(position)).isTrue();
        }
        assertThat(case5.isLastTwoDates(CASE_5_REPORT_COUNT)).isFalse();

    }

    @Test
    public void isLastThreeDates_shouldReturnsCorrectValue() {
        case2.generateReport();
        assertThat(case2.getDateArray()).isNotNull();
        for (int position = 0; position < CASE_2_REPORT_COUNT - 3; position++) {
            assertThat(case2.isLastThreeDates(position)).isFalse();
        }
        for (int position = CASE_2_REPORT_COUNT - 3; position < CASE_2_REPORT_COUNT; position++) {
            assertThat(case2.isLastThreeDates(position)).isTrue();
        }
        assertThat(case2.isLastThreeDates(CASE_2_REPORT_COUNT)).isFalse();
    }
}

The class I'm trying to test is a simple data model I get from Firebase. I store a range of report dates in an array and want to verify that the given position is the last (and only the last) two dates of the report dates:

public class Traktamente {


      // More fields

      public String startDate;
      public String startTime;
      public String endDate;
      public String endTime;
      public String origin;
      public String destination;


       // More fields

       @Exclude
        private List<LocalDate> dateArray;

        @Exclude
        public void generateReport() {
            LocalDate startDate = LocalDate.parse(this.startDate);
            LocalDate endDate = LocalDate.parse(this.endDate).plusDays(2);

            int daysNumber = Days.daysBetween(startDate, endDate).getDays() + 1; // endDate inclusive
            dateArray = new ArrayList<>(daysNumber);

            for (int i = 0; i < daysNumber; i++) {
                LocalDate date = startDate.withFieldAdded(DurationFieldType.days(), i);
                dateArray.add(date);
            }
        }


        @Exclude
        private boolean isBefore12Pm(String date) {
            String format = "yyyy-MM-dd HH:mm";
            Date date1 = Utils.getDateFromString(format, date + " 12:00");
            Date date2 = Utils.getDateFromString(format, date + " " + startTime);
            assert date2 != null;
            return date2.compareTo(date1) < 0;
        }

        @Exclude
        private boolean isAfter17Pm(String date) {
            String format = "yyyy-MM-dd HH:mm";
            Date date1 = Utils.getDateFromString(format, date + " 17:00");
            Date date2 = Utils.getDateFromString(format, date + " " + endTime);
            assert date2 != null;
            return date2.compareTo(date1) > 0;
        }

        @Exclude
        private boolean isFirstDateOfTheTrip(int position) {
            return position == 0;
        }

        @Exclude
        public boolean isEndDateOfTheTrip(int position) {
            return position == dateArray.size() - 3;
        }

        @Exclude
        public boolean isLastThreeDates(int position) {
            return position >= dateArray.size() - 3 && position < dateArray.size();
        }

        @Exclude
        public boolean isLastTwoDates(int position) {
            return position >= dateArray.size() - 2 && position < dateArray.size();
        }
}

I also have isLastThreeDates(position) with identical purpose.

Am I on the right track to test my code? Or is it something I can improve? Or should I even need to write the test for this code?

\$\endgroup\$
2
  • \$\begingroup\$ is it possible to add the hole unit-test class and data model? \$\endgroup\$
    – Roman
    Commented Mar 11, 2019 at 15:32
  • \$\begingroup\$ @Roman I added the test code, \$\endgroup\$
    – Seto
    Commented Mar 11, 2019 at 16:27

1 Answer 1

3
\$\begingroup\$

Welcome to CodeReview @Seto! :]


In a summery for tests of the book Clean Code by Robert C. Martin the following points are important

  • One assert per test.
  • Readable.
  • Fast.
  • Independent.
  • Repeatable.

At first glance, we can see that the tests break the rule of one assert per test and are not as readable as they could be.


The Builder Pattern for Tests

@Before
public void setupOnce() {

   case1 = new Traktamente();
   case1.startTime = "13:00";
   case1.startDate = "2019-01-07";
   // ...

   case2 = new Traktamente();
   case2.startTime = "20:00";
   case2.startDate = "2019-01-06";
   // ..

   case2 = new Traktamente();
   // ..
}

Many fields share the same values. With the builder pattern we can add common values to clean up the setupOnce.

public class TraktamenteBuilder {

    public String startDate = "2019-01-07";
    public String startTime = "13:00";
    public String endDate = // add your default here;
    public String endTime = // add your default here;
    public String origin = // add your default here;
    public String destination = // add your default here;
    public boolean switchEgetBoendeState = true;
    public boolean switchNinetyDaysState = true;
    public String comments = "Lorem ipsum dolor sit amet.";

    public TraktamenteBuilder withStartDate(String startDate) {
        this.startDate = startDate;
        return this;
    }

    public TraktamenteBuilder withStartTime(String tartTime) {
        this.startTime = startTime;
        return this;
    }

    public TraktamenteBuilder withEgetBoendeState() {
        this.switchEgetBoendeState = true;
        return this;
    }

    // make them expressive - much better then setEgetBoendeState(boolean)
    public TraktamenteBuilder withoutEgetBoendeState() {
        this.switchEgetBoendeState = false;
        return this;
    }

    // add more..


    public Traktamente build() {
        Traktamente traktamente = new Traktamente();
        traktamente.startTime = startTime;
        traktamente.startDate = startDate; 
        traktamente.endTime = endTime;
        traktamente.endDate = endDate;
        traktamente.origin = origin;
        traktamente.destination = destination;
        traktamente.switchEgetBoendeState = switchEgetBoendeState;
        traktamente.switchNinetyDaysState = switchNinetyDaysState;
        traktamente.comments = comments;
        return traktamente;
    }

}
public class TraktamenteTest {

    private final int CASE_1_REPORT_COUNT = 6;
    private final int CASE_2_REPORT_COUNT = 7;
    private final int CASE_3_REPORT_COUNT = 14;
    private final int CASE_4_REPORT_COUNT = 12;
    private final int CASE_5_REPORT_COUNT = 12;
    private final int CASE_6_REPORT_COUNT = 16;

    private final Traktamente case1 = new TraktamenteBuilder().withStartDate("2019-01-07")
                                                              .withStartTime("13:00")
                                                              .withEndDate("2019-01-10")
                                                              .withEndTime("21:00")
                                                              .build();

    private final Traktamente case2 = new TraktamenteBuilder().withStartDate("2019-01-06")
                                                              .withStartTime("20:00")
                                                              .withEndDate("2019-01-10")
                                                              .withEndTime("21:00")
                                                              .build();


    // more cases

    @Before
    public void setupOnce() {
        DateTimeZone.setProvider(new UTCProvider());
    }

    // test cases ...
}

Parameterized Tests

You have 4 test and 3 of them contains repetitive code, because you want to check the logic for different cases. The disadvantage is that your test cases are big, contains duplicate code and you need to change at so many places if the API of your application will change at one day.

Actually you can think of your different cases as parameters of one test. For jUnit4 you can take a look at a github guide for parameterized tests or if you want to take a look at jUnit5 you can take a look into the user guide for parameterized tests. There also exists a tool with the name JUnitParams


Focus on one Assertion

@Test
public void generateReport_shouldAddTwoDatesAfterTheEndDate() {
   case1.generateReport();
   // ..

   assertThat(case1.getDateArray().size()).isEqualTo(CASE_1_REPORT_COUNT);
   // ..

   assertThat(case1.startDate.equals(case1.getDateArray().get(0).toString())).isTrue();
   // ..

   assertThat(case1.endDate.equals(case1.getDateArray().get(CASE_1_REPORT_COUNT - 3).toString())).isTrue();
   // ..
}

The test generateReport_shouldAddTwoDatesAfterTheEndDate tests more than just adding to dates to the end. It also check the array size and the first startDate. This tests should be in their own tests.


Arrange Act Assert

Each method should group these functional sections, separated by blank lines:

Arrange all necessary preconditions and inputs.
Act on the object or method under test.
Assert that the expected results have occurred.

The AAA-Pattern makes it easier to understand and maintain the code - not only for your self but for other programmers that need to read or change your code too.

@Test
public void isEndDateOfTheTrip_shouldReturnsTrue() {
    // arrange
    case1.generateReport();

    // act
    boolean isEndDateOfTheTrip = case1.isEndDateOfTheTrip(CASE_1_REPORT_COUNT-3);

    // assert
    assertThat(isEndDateOfTheTrip).isTrue();
}
\$\endgroup\$
4
  • \$\begingroup\$ A really comprehensive answer. Accepted. I'll read it line by line. \$\endgroup\$
    – Seto
    Commented Mar 13, 2019 at 10:38
  • \$\begingroup\$ "The test generateReport_shouldAddTwoDatesAfterTheEndDate tests more than just adding to dates to the end. It also check the array size and the first startDate. This tests should be in their own tests." A question, I also need to verify that it's adding the correct date array, right? \$\endgroup\$
    – Seto
    Commented Mar 13, 2019 at 12:10
  • 1
    \$\begingroup\$ Yes, this is desirable and I thing this would be the most important test to check the correctness :] \$\endgroup\$
    – Roman
    Commented Mar 13, 2019 at 12:13
  • \$\begingroup\$ Ah, so I need to make another test only for this, right? \$\endgroup\$
    – Seto
    Commented Mar 13, 2019 at 12:15

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.