Uploaded image for project: 'Hadoop YARN'
  1. Hadoop YARN
  2. YARN-6450

TestContainerManagerWithLCE requires override for each new test added to ContainerManagerTest

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.9.0, 3.0.0-alpha4, 2.8.2
    • Component/s: test
    • Labels:
      None

      Description

      Every test in TestContainerManagerWithLCE looks like this:

        @Override
        public void testSomething() throws Exception {
          // Don't run the test if the binary is not available.
          if (!shouldRunTest()) {
            LOG.info("LCE binary path is not passed. Not running the test");
            return;
          }
          LOG.info("Running something");
          super.testSomething();
        }
      

      If a new test is added to ContainerManagerTest then by default ContainerManagerTestWithLCE will fail when the LCE has not been configured. This is an unnecessary maintenance burden.

        Activity

        Hide
        jlowe Jason Lowe added a comment -

        Using Assume.assumeTrue(shouldRunTest()) in the existing setup function will automatically skip the tests if the LCE hasn't been configured. Then we don't need to revisit this code every time a new unit test is added to ContainerManagerTest.

        Attaching a patch that implements this approach.

        Show
        jlowe Jason Lowe added a comment - Using Assume.assumeTrue(shouldRunTest()) in the existing setup function will automatically skip the tests if the LCE hasn't been configured. Then we don't need to revisit this code every time a new unit test is added to ContainerManagerTest. Attaching a patch that implements this approach.
        Hide
        eepayne Eric Payne added a comment -

        Thanks Jason Lowe. The patch LGTM.
        +1. Will commit shortly.

        Show
        eepayne Eric Payne added a comment - Thanks Jason Lowe . The patch LGTM. +1. Will commit shortly.
        Hide
        eepayne Eric Payne added a comment -

        Thanks Jason Lowe. I committed the patch to trunk and branch-2.

        Show
        eepayne Eric Payne added a comment - Thanks Jason Lowe . I committed the patch to trunk and branch-2.
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11576 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11576/)
        YARN-6450. TestContainerManagerWithLCE requires override for each new (epayne: rev a16ab2be91e29f0aa4ad3488d35977cb5d1e1f5c)

        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestContainerManagerWithLCE.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11576 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11576/ ) YARN-6450 . TestContainerManagerWithLCE requires override for each new (epayne: rev a16ab2be91e29f0aa4ad3488d35977cb5d1e1f5c) (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestContainerManagerWithLCE.java
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11591 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11591/)
        YARN-6450. TestContainerManagerWithLCE requires override for each new (epayne: rev a16ab2be91e29f0aa4ad3488d35977cb5d1e1f5c)

        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestContainerManagerWithLCE.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11591 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11591/ ) YARN-6450 . TestContainerManagerWithLCE requires override for each new (epayne: rev a16ab2be91e29f0aa4ad3488d35977cb5d1e1f5c) (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestContainerManagerWithLCE.java
        Hide
        jlowe Jason Lowe added a comment -

        I committed this to branch-2.8 and branch-2.8.2 as well.

        Show
        jlowe Jason Lowe added a comment - I committed this to branch-2.8 and branch-2.8.2 as well.

          People

          • Assignee:
            jlowe Jason Lowe
            Reporter:
            jlowe Jason Lowe
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development