Oozie
  1. Oozie
  2. OOZIE-1597

Cleanup database before every test

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: trunk
    • Fix Version/s: trunk
    • Component/s: tests
    • Labels:
      None

      Description

      While investigating a flakey test (org.apache.oozie.sla.TestSLAJobEventListener.testOnJobEvent) I realized that some of the flakey SLA tests that I've seen lately are the same issue: The database has some leftover stuff from a previous test that its not expecting.

      Normally this is easy to fix because we can simply call cleanUpDBTables(). However, cleanUpDBTables requires some of the Services to be running, so you have to call it after starting Services; but, some of the failures were occurring during Services initialization (specifically when SLAService initializes the SLACalculatorMemory, which tries to load some data from the database, which may be incomplete (e.g. SLA registration for a job that doesn't exist)). So, in this case, we can't call cleanUpDBTables() before or after starting Services.

      This brings the larger issue that we should be cleaning up the database before every test anyway to make sure that the tests are truly independent and to prevent harmful leaking (just like we did a while back with the Services). I think we should have XTestCase.setup() call cleanUpDBTables() so that every test automatically it (and handle the Services dependency appropriately).

      1. OOZIE-1597.patch
        82 kB
        Robert Kanter

        Activity

        Hide
        Robert Kanter added a comment -

        TestSLAEventGeneration.testWorkflowJobSLANew is another example where we can't simply call cleanUpDBTables() for the same reason.

        Show
        Robert Kanter added a comment - TestSLAEventGeneration.testWorkflowJobSLANew is another example where we can't simply call cleanUpDBTables() for the same reason.
        Hide
        Robert Kanter added a comment -

        The patch makes XTestCase.setUp() call cleanUpDBTables() so that all tests clean up the database. It also removes the direct call to cleanUpDBTables() from any of the tests that were calling it in their setUp() method. cleanUpDBTables() will now also start and end the Services with the minimal set of required services needed for cleaning the database, unless the Services is already started. There were a few tests, notably, the ones under the tools dir, where I had to make a way to not clean the database as it was interfering with those and causing surefire to get stuck.

        The overhead of running cleanUpDBTables for every test shouldn't be too expensive. It only takes about a second to run and we were already running it for about half of the test classes already anyway.

        Show
        Robert Kanter added a comment - The patch makes XTestCase.setUp() call cleanUpDBTables() so that all tests clean up the database. It also removes the direct call to cleanUpDBTables() from any of the tests that were calling it in their setUp() method. cleanUpDBTables() will now also start and end the Services with the minimal set of required services needed for cleaning the database, unless the Services is already started. There were a few tests, notably, the ones under the tools dir, where I had to make a way to not clean the database as it was interfering with those and causing surefire to get stuck. The overhead of running cleanUpDBTables for every test shouldn't be too expensive. It only takes about a second to run and we were already running it for about half of the test classes already anyway.
        Hide
        Robert Kanter added a comment -

        (Most of the changes in the patch are removing the calls to cleanUpDBTables(); if you want to see the "real" change, look at XTestCase in the patch)

        Show
        Robert Kanter added a comment - (Most of the changes in the patch are removing the calls to cleanUpDBTables() ; if you want to see the "real" change, look at XTestCase in the patch)
        Hide
        Hadoop QA added a comment -

        Testing JIRA OOZIE-1597

        Cleaning local svn workspace

        ----------------------------

        +1 PATCH_APPLIES
        +1 CLEAN
        +1 RAW_PATCH_ANALYSIS
        . +1 the patch does not introduce any @author tags
        . +1 the patch does not introduce any tabs
        . +1 the patch does not introduce any trailing spaces
        . +1 the patch does not introduce any line longer than 132
        . +1 the patch does adds/modifies 124 testcase(s)
        +1 RAT
        . +1 the patch does not seem to introduce new RAT warnings
        +1 JAVADOC
        . +1 the patch does not seem to introduce new Javadoc warnings
        +1 COMPILE
        . +1 HEAD compiles
        . +1 patch compiles
        . +1 the patch does not seem to introduce new javac warnings
        +1 BACKWARDS_COMPATIBILITY
        . +1 the patch does not change any JPA Entity/Colum/Basic/Lob/Transient annotations
        . +1 the patch does not modify JPA files
        +1 TESTS
        . Tests run: 1347
        +1 DISTRO
        . +1 distro tarball builds with the patch

        ----------------------------
        +1 Overall result, good!, no -1s

        The full output of the test-patch run is available at

        . https://builds.apache.org/job/oozie-trunk-precommit-build/860/

        Show
        Hadoop QA added a comment - Testing JIRA OOZIE-1597 Cleaning local svn workspace ---------------------------- +1 PATCH_APPLIES +1 CLEAN +1 RAW_PATCH_ANALYSIS . +1 the patch does not introduce any @author tags . +1 the patch does not introduce any tabs . +1 the patch does not introduce any trailing spaces . +1 the patch does not introduce any line longer than 132 . +1 the patch does adds/modifies 124 testcase(s) +1 RAT . +1 the patch does not seem to introduce new RAT warnings +1 JAVADOC . +1 the patch does not seem to introduce new Javadoc warnings +1 COMPILE . +1 HEAD compiles . +1 patch compiles . +1 the patch does not seem to introduce new javac warnings +1 BACKWARDS_COMPATIBILITY . +1 the patch does not change any JPA Entity/Colum/Basic/Lob/Transient annotations . +1 the patch does not modify JPA files +1 TESTS . Tests run: 1347 +1 DISTRO . +1 distro tarball builds with the patch ---------------------------- +1 Overall result, good!, no -1s The full output of the test-patch run is available at . https://builds.apache.org/job/oozie-trunk-precommit-build/860/
        Hide
        Alejandro Abdelnur added a comment -

        +1 LGTM

        Show
        Alejandro Abdelnur added a comment - +1 LGTM
        Hide
        Robert Kanter added a comment -

        Committed to trunk!

        Show
        Robert Kanter added a comment - Committed to trunk!

          People

          • Assignee:
            Robert Kanter
            Reporter:
            Robert Kanter
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development