Uploaded image for project: 'Bigtop'
  1. Bigtop
  2. BIGTOP-1225

TestHDFSQuota can fail if tests are run out of order

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.7.0
    • Fix Version/s: 0.8.0
    • Component/s: None
    • Labels:
      None

      Description

      When running TestHDFSQuota some tests can fail when it is run out of order.

      1. BIGTOP-1225.patch
        2 kB
        Virginia Wang
      2. BIGTOP-1225.patch
        6 kB
        Virginia Wang
      3. BIGTOP-1225.patch
        7 kB
        Virginia Wang

        Issue Links

          Activity

          Hide
          Virginia Wang Virginia Wang added a comment -

          Need be implemented by the fix from BIGTOP-1224.

          Show
          Virginia Wang Virginia Wang added a comment - Need be implemented by the fix from BIGTOP-1224 .
          Hide
          cos Konstantin Boudnik added a comment -

          Virginia, could you please provide more details on which tests are order dependent?

          Show
          cos Konstantin Boudnik added a comment - Virginia, could you please provide more details on which tests are order dependent?
          Hide
          Virginia Wang Virginia Wang added a comment -

          In my run the two tests that failed were testNewlyCreatedDir and testQuotas.

          testNewlyCreatedDir failed because another test had run before and changed the quota of that file throwing the error: Newly created directory had a set name quota

          testQuotas failed because it has to appear AFTER testQuotasPostViolation throwing the error: mkdir should not have worked . testQuotasPostViolation sets a quota to 2 making the mkdir fail. When out of order the mkdir becomes possible.

          What I would do is set the all the test in order from 0-8 as I think that was intended.

          Show
          Virginia Wang Virginia Wang added a comment - In my run the two tests that failed were testNewlyCreatedDir and testQuotas. testNewlyCreatedDir failed because another test had run before and changed the quota of that file throwing the error: Newly created directory had a set name quota testQuotas failed because it has to appear AFTER testQuotasPostViolation throwing the error: mkdir should not have worked . testQuotasPostViolation sets a quota to 2 making the mkdir fail. When out of order the mkdir becomes possible. What I would do is set the all the test in order from 0-8 as I think that was intended.
          Hide
          cos Konstantin Boudnik added a comment -

          The reason I am bringing this up is that while setting up an order of execution would likely to solve the problem - as you have discovered in the experiments - fixing the tests to be independent might be a better idea going forward. In this particular case, the offending tests should cleanup after themselves or there should be an @After method that restores the state of the filesystem. Another way to fix it is to make sure that tests aren't using conflicting resources, e.g. two tests shouldn't reuse /directory1: instead they should create what they need and remove it once they are done.

          Do you think it would be possible to fix the tests without relying on the order? Thanks!

          Show
          cos Konstantin Boudnik added a comment - The reason I am bringing this up is that while setting up an order of execution would likely to solve the problem - as you have discovered in the experiments - fixing the tests to be independent might be a better idea going forward. In this particular case, the offending tests should cleanup after themselves or there should be an @After method that restores the state of the filesystem. Another way to fix it is to make sure that tests aren't using conflicting resources, e.g. two tests shouldn't reuse /directory1 : instead they should create what they need and remove it once they are done. Do you think it would be possible to fix the tests without relying on the order? Thanks!
          Hide
          Virginia Wang Virginia Wang added a comment -

          I have considered this case as well. Will work on it. Thanks

          Show
          Virginia Wang Virginia Wang added a comment - I have considered this case as well. Will work on it. Thanks
          Hide
          Virginia Wang Virginia Wang added a comment -

          I removed the @BeforeClass and @AfterClass so the setUp() and tearDown() can now be called before and after the tests. This way it does not matter what order the tests get run, it will perform a fresh set of directories and a clean after.

          I've merged testQuotas into testQuotasPostViolation() since it was dependent on it, might as well have it continue as part of the test.

          Show
          Virginia Wang Virginia Wang added a comment - I removed the @BeforeClass and @AfterClass so the setUp() and tearDown() can now be called before and after the tests. This way it does not matter what order the tests get run, it will perform a fresh set of directories and a clean after. I've merged testQuotas into testQuotasPostViolation() since it was dependent on it, might as well have it continue as part of the test.
          Hide
          cos Konstantin Boudnik added a comment - - edited

          You are know calling the setup and teardown explicitly in each test case. You can do this by using @Before and @After annotations on the setup and teardown methods.
          Also, there's a white space change at the top

          Show
          cos Konstantin Boudnik added a comment - - edited You are know calling the setup and teardown explicitly in each test case. You can do this by using @Before and @After annotations on the setup and teardown methods. Also, there's a white space change at the top
          Hide
          Virginia Wang Virginia Wang added a comment -

          Ah right, some reason I thought those annotations did something else. Fixed and patched added.

          Show
          Virginia Wang Virginia Wang added a comment - Ah right, some reason I thought those annotations did something else. Fixed and patched added.
          Hide
          cos Konstantin Boudnik added a comment -
          • there are unused imports: they were unused before, but I don't see any point of keeping them around
             import static org.junit.Assert.assertNotNull;
             import static org.junit.Assert.assertEquals;
            
          • there are a few whitespace changes again. Run git diff command that will show exactly what's in the patch so you can remove unused pieces.
          Show
          cos Konstantin Boudnik added a comment - there are unused imports: they were unused before, but I don't see any point of keeping them around import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertEquals; there are a few whitespace changes again. Run git diff command that will show exactly what's in the patch so you can remove unused pieces.
          Hide
          Virginia Wang Virginia Wang added a comment -

          Removed unused imports and white spaces.

          Show
          Virginia Wang Virginia Wang added a comment - Removed unused imports and white spaces.
          Hide
          cos Konstantin Boudnik added a comment -

          +1 looks good now.

          Show
          cos Konstantin Boudnik added a comment - +1 looks good now.
          Hide
          cos Konstantin Boudnik added a comment -

          Committed to the master. Thanks Virginia

          Show
          cos Konstantin Boudnik added a comment - Committed to the master. Thanks Virginia

            People

            • Assignee:
              Virginia Wang Virginia Wang
              Reporter:
              Virginia Wang Virginia Wang
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development