Timeout affects another test

[John Gorge]

Hi,

I have a problem with Junit parametric test.

If I run each test in the upTo5sec category separately, then tests will end without errors
If I run all the tests together then some give a timeout and some errors.

Example.
List.txt defines which tests will be executed. It can be generated by listgenerator.sh

If list.txt contains only upTo5sec/bacp/bacp-22 test will be passed.
If list.txt contains only upTo5sec/bacp/bacp-21 test will be passed
If list.txt contains another test bacp-21 will be timeout and at the same test upTo5sec/bacp/bacp-21 will be failed.

In some way timeout(bacp-22) affects another test(bacp-21).

https://bitbucket.org/maf2/jacop/get/36f60b9db11e.zip

[Stephan van Hulst]

That means you're tests don't work in isolation. Are you setting up and tearing down your test data correctly between each test? Show us the code of two tests that fail when they are run together, and your setup and teardown code.

[John Gorge]

" Are you setting up and tearing down your test data correctly between each test? " -  I think so.

Edit the list.txt file and copy it to list.txt(src/test/fz/upTo5sec/list.txt) and run src/test/java/org/jacop/MinizincBasedTestUpTo5Seconds.java:

assignment2/assignment2
atmost1_me/atmost1_me
bacp/bacp-22
bacp/bacp-21

Test failed.

Next copy to list txt:

atmost1_me/atmost1_me
assignment2/assignment2
bacp/bacp-22
bacp/bacp-21

Test is passed !!!

[Stephan van Hulst]

Copying files sounds like your problem. Unit tests should not perform file system operations. If you need to read data, read it from a resource embedded in your application. The resource must NOT be altered.

Show us the contents of the unit test please.

[John Gorge]

The test use two files. MinizincBasedTestUpTo5Seconds.java and MinizincBasedTestsHelper.java

MinizincBasedTestUpTo5Seconds.java

@RunWith(Parameterized.class) public class MinizincBasedTestUpTo5Seconds extends MinizincBasedTestsHelper {
    protected static final String timeCategory = "upTo5sec/";

    public MinizincBasedTestUpTo5Seconds(String testFilename) {


        this.testFilename = testFilename;

    }

    @Parameterized.Parameters public static Collection<String> parametricTest() throws IOException {

        return fileReader(timeCategory);
    }

    @Test(timeout = 15000) public void testMinizinc() throws IOException {

        testExecution(timeCategory);
    }
}

MinizincBasedTestsHelper.java

public class MinizincBasedTestsHelper {
    protected String testFilename;
    protected static Fz2jacop fz2jacop;
    protected static final String relativePath = "src/test/fz/";
    protected static String timeCategory;
    protected static final String listFileName = "list.txt";
    protected static final boolean printInfo = true;



    @BeforeClass public static void initialize() {
        fz2jacop = new Fz2jacop();
    }



    protected static List<String> result(String filename) {

        ByteArrayOutputStream baos = new ByteArrayOutputStream();
        PrintStream old = System.out;
        System.setOut(new PrintStream(baos));

        try {
            fz2jacop.main(new String[] {relativePath + filename});
        } finally {

            System.out.flush();
            System.setOut(old);

            String result = baos.toString();
            if (printInfo) {
                System.out.println(filename + "\n" + result);
            }

            return Arrays.asList(result.split("\n"));

        }

    }


    protected static List<String> expected(String filename) throws IOException {

        String filePath = new File(relativePath + filename).getAbsolutePath();
        return Files.readAllLines(Paths.get(filePath), StandardCharsets.UTF_8);
    }

    protected static Collection<String> fileReader(String timeCategory) throws IOException {
        System.out.println("timeCategory" + timeCategory);
        FileReader file = new FileReader(relativePath + timeCategory + listFileName);
        BufferedReader br = new BufferedReader(file);
        String line = "";
        List<String> list = new ArrayList<String>();
        int i = 0;
        while ((line = br.readLine()) != null) {
            list.add(i, line);
            i++;
        }
        return list;
    }

    protected void testExecution(String timeCategory) throws IOException {

        System.out.println("Test file: " + timeCategory + testFilename);

        System.out.println("Test filename " + testFilename);
//        testFilename = "";

        List<String> expectedResult = expected(timeCategory + testFilename + ".out");
        List<String> result = result(timeCategory + testFilename + ".fzn");
        System.out.println("Wartości " + expectedResult +" tuuuuuuuT " + result);


        if (result.size() == 0)
            fail("\n" + "File path: " + timeCategory + testFilename + ".fzn " + " gave no output to compare against.");

        for (int i = 0, j = 0; i < result.size() || j < expectedResult.size(); ) {
            if (i < result.size() && result.get(i).trim().isEmpty()) {
                i++;
                continue;
            }
            if (j < expectedResult.size() && expectedResult.get(j).trim().isEmpty()) {
                j++;
                continue;
            }
            if (result.size() == i)
                fail("\n" + "File path: " + timeCategory + testFilename + ".out "
                    + " gave as a result less textlines that was expected. Expected line " + (j + 1) + " not found.");
            if (expectedResult.size() == j)
                fail("\n" + "File path: " + timeCategory + testFilename + ".out "
                    + " gave as a result more textlines that was expected. Actual line " + (i + 1) + "not found in expected result");

            assertEquals(
                "\n" + "File path: " + timeCategory + testFilename + ".out " + "\nError line number (expected, actual): (" + (j + 1) + ","
                    + (i + 1) + ")\n", expectedResult.get(j).trim(), result.get(i).trim());
            i++;
            j++;

        }
        expectedResult = null;
        result = null;
        testFilename= null;
        System.out.println("Expected " + expectedResult + "result" + result + "testFilename" + testFilename);
    }
}

[Stephan van Hulst]

Yeah... I'm sorry to say this, but you need to go back to the drawing board and redesign your tests. There are many issues with your code:

It's complex. There's no point to unit testing if bugs can be introduced in the tests themselves.

It's not clear what your test cases actually test. Tests must be small, easy to read and they must test one thing really well.

It's performing file IO. If you have a whole lot of data that you don't want to hardcode, create reusable test fixtures that read data from application resources.

It uses inheritance for the wrong reasons. Is your MinizincBasedTestUpTo5Seconds really also a MinizincBasedTestHelper? Don't use inheritance just because you want to easily add behavior to your class.

You're not using the right tools for the job. All the error messages you're throwing around suggest to me you're not using the power of your testing framework.

For now, the best suggestions I have for you to clean this up is to write a couple of test fixtures that create strongly typed objects out of resources, and to use Hamcrest's matchers to perform your assertions. I'll write an example of how to do this shortly.

[Stephan van Hulst]

Okay, let's make a simple example where we mix additive colors together. Let's limit the set of colors to just the primary and the secondary colors:

enum Color {

  RED, GREEN, BLUE,
  YELLOW, MAGENTA, CYAN;

  /**
   * Returns whether this color is a primary color.
   *
   * @return true if this color is red, green or blue; false otherwise.
   */
  boolean isPrimary() {
    throw new UnsupportedOperationException("Not implemented yet.");
  }

  /**
   * Mix this color with another.
   *
   * All colors can be mixed with themselves to get the same color.
   * Primary colors can be mixed with other primary colors to form a secondary color.
   * Secondary colors can only be mixed with themselves, not with other primary or secondary colors.
   *
   * @return this if this is mixed with itself, yellow if red is mixed with green,
   *         magenta if red is mixed with blue, or cyan if green is mixed with blue.
   *
   * @throws IllegalArgumentException if other is null or a color that this can not be mixed with.
   */
  Color mix(Color other) {
    throw new UnsupportedOperationException("Not implemented yet.");
  }
}

Now, we want to write a test for the mix() method. We can do this really easily with JUnit's experimental theories API.

@RunWith(Theories.class)
public final class ColorTest {

  @DataPoints
  public Color[] colors = Color.values();

  @Rule
  public ExpectedException thrown = ExpectedException.none();

  @Theory
  mixColor_withItself_returnsItself(Color color) {
    Color mixed = color.mix(color);
    assertThat(mixed, is(color));
  }

  @Theory
  mixColor_withNull_throwsException(Color color) {
    thrown.expect(IllegalArgumentException.class);

    Color mixed = color.mix(null);
  }

  @Theory
  mixPrimary_withPrimary_returnsSecondary(Color firstColor, Color secondColor) {
    assumeThat(firstColor, is(not(secondColor)));
    assumeTrue(firstColor.isPrimary());
    assumeTrue(secondColor.isPrimary());

    Color mixed = firstColor.mix(secondColor);

    assertFalse(mixed.isPrimary());
  }

  @Theory
  mixPrimary_withSecondary_throwsException(Color firstColor, Color secondColor) {
    assumeTrue(firstColor.isPrimary());
    assumeFalse(secondColor.isPrimary());

    thrown.expect(IllegalArgumentException.class);

    Color mixed = firstColor.mix(secondColor);
  }

  @Theory
  mixSecondary_withPrimary_throwsException(Color firstColor, Color secondColor) {
    assumeFalse(firstColor.isPrimary());
    assumeTrue(secondColor.isPrimary());

    thrown.expect(IllegalArgumentException.class);

    Color mixed = firstColor.mix(secondColor);
  }

  @Theory
  mixSecondary_withSecondary_throwsException(Color firstColor, Color secondColor) {
    assumeThat(firstColor, is(not(secondColor)));
    assumeFalse(firstColor.isPrimary());
    assumeFalse(secondColor.isPrimary());

    thrown.expect(IllegalArgumentException.class);

    Color mixed = firstColor.mix(secondColor);
  }
}

For each theory, JUnit creates a Cartesian product of the data points it can assign to the theory's parameters. The assumptions we make at the start of the theory filter out combinations that are not valid to test that particular theory. Then we perform the method under test, and make assertions about the result.

Note that we already wrote 6 test methods to test invariants of one method under test. Each test method tests a very specific invariant, and it's clear from the code what is being tested in each method.

Now, in this case it's very easy to get the data points. But what if we have many data points that we need to generate from configuration data? You can then write a test fixture:

final class ColorFixture {

  private static final Collection<Color> PRIMARY_COLORS;
  private static final Collection<Color> SECONDARY_COLORS;

  static {
    PRIMARY_COLORS = Collections.unmodifiableCollection(loadColors("primary.txt"));
    SECONDARY_COLORS= Collections.unmodifiableCollection(loadColors("secondary.txt"));
  }

  private ColorFixture() {}

  static Collection<Color> getPrimaryColors() {
    return PRIMARY_COLORS;
  }

  static Collection<Color> getSecondaryColors() {
    return SECONDARY_COLORS;
  }

  private static Collection<Color> loadColors(String resourceName) {
    try (
      InputStream input = ColorFixture.class.getResourceAsStream(resourceName);
      BufferedReader reader = new BufferedReader(new InputStreamReader(input));
    ) {
      return reader.lines().map(ColorFixture::parseColor).collect(toList());
    }

    catch (IOException ex) {
      throw new AssertionError(ex);
    }
  }

  private static Color parseColor(String line) {
    //
  }
}

The nice thing about this is that once you have written your fixtures that contain types that are either mocked or loaded from a resource, you can easily fill your data points in many test classes.