You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2022/03/03 19:10:35 UTC

[GitHub] [accumulo] DomGarguilo opened a new pull request #2545: WIP - Migrate test and hadoop-mapreduce modules to JUnit5

DomGarguilo opened a new pull request #2545:
URL: https://github.com/apache/accumulo/pull/2545


   This PR coverts the accumulo-test and accumulo-hadoop-mapreduce modules from JUnit4 to JUnit5.
   
   These modules had to be bundled together due to some dependency overlap.
   
   All ITs currently pass ([Full ITs](https://github.com/DomGarguilo/accumulo/runs/5411749737?check_suite_focus=true)) but there is still some work to be done before this is ready to merge.
   
   TODO:
   - Scalable timeouts need to be sorted out ([here is a description of the problem](https://github.com/apache/accumulo/issues/2441#issuecomment-1050166774))


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [accumulo] DomGarguilo commented on pull request #2545: Migrate test and hadoop-mapreduce modules to JUnit5

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on pull request #2545:
URL: https://github.com/apache/accumulo/pull/2545#issuecomment-1069591446


   > I think this is all good now, but I want to run my own full IT build before it is merged, just to be sure.
   
   Sounds good. Thanks for your review and feel free to merge this PR after your testing, if you would like to.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [accumulo] ctubbsii commented on a change in pull request #2545: Migrate test and hadoop-mapreduce modules to JUnit5

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #2545:
URL: https://github.com/apache/accumulo/pull/2545#discussion_r828330105



##########
File path: server/monitor/src/test/java/org/apache/accumulo/monitor/TagNameConstants.java
##########
@@ -0,0 +1,23 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.monitor;

Review comment:
       Similar comment as above, regarding the uniqueness of the test package name if the class is used for integration tests, due to jar sealing.

##########
File path: test/src/main/java/org/apache/accumulo/harness/AccumuloClusterHarness.java
##########
@@ -60,7 +59,7 @@
  * advanced ITs that do crazy things. For more typical, expected behavior of a cluster see
  * {@link SharedMiniClusterBase}. This instance can be MAC or a standalone instance.
  */
-@Category(StandaloneCapableClusterTests.class)
+@Tag("StandaloneCapableCluster")

Review comment:
       Works for me.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [accumulo] ctubbsii merged pull request #2545: Migrate test and hadoop-mapreduce modules to JUnit5

Posted by GitBox <gi...@apache.org>.
ctubbsii merged pull request #2545:
URL: https://github.com/apache/accumulo/pull/2545


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [accumulo] DomGarguilo commented on a change in pull request #2545: Migrate test and hadoop-mapreduce modules to JUnit5

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on a change in pull request #2545:
URL: https://github.com/apache/accumulo/pull/2545#discussion_r824753909



##########
File path: TESTING.md
##########
@@ -98,7 +98,7 @@ the tests.
 These tests can be run by providing a system property.  Specific ITs can be run using "-Dit.test" or run all tests using:
 
 ```bash
-mvn clean verify -Dtest=foo -Daccumulo.it.properties=/home/user/my_cluster.properties -Dfailsafe.groups=org.apache.accumulo.test.categories.StandaloneCapableClusterTests -Dspotbugs.skip
+mvn clean verify -Dtest=foo -Daccumulo.it.properties=/home/user/my_cluster.properties -Dfailsafe.groups=StandaloneCapableClusterTests -Dspotbugs.skip

Review comment:
       Addressed in [d1fa1f3](https://github.com/apache/accumulo/pull/2545/commits/d1fa1f31dbbf5e8728ceaa9359f2e92b67cd4d75)




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [accumulo] DomGarguilo commented on a change in pull request #2545: Migrate test and hadoop-mapreduce modules to JUnit5

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on a change in pull request #2545:
URL: https://github.com/apache/accumulo/pull/2545#discussion_r828269838



##########
File path: hadoop-mapreduce/src/test/java/org/apache/accumulo/WithTestNames.java
##########
@@ -16,10 +16,27 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.accumulo.test.categories;
+package org.apache.accumulo;

Review comment:
       Sounds good. I'll move it into the package you mentioned. Do you think this is something that will also need to happen for [`WithTestNames` in the test module](https://github.com/DomGarguilo/accumulo/blob/testAndHadoopMRmodulesMigrateJUnit/test/src/main/java/org/apache/accumulo/WithTestNames.java)?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [accumulo] ctubbsii commented on a change in pull request #2545: Migrate test and hadoop-mapreduce modules to JUnit5

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #2545:
URL: https://github.com/apache/accumulo/pull/2545#discussion_r824167367



##########
File path: TESTING.md
##########
@@ -98,7 +98,7 @@ the tests.
 These tests can be run by providing a system property.  Specific ITs can be run using "-Dit.test" or run all tests using:
 
 ```bash
-mvn clean verify -Dtest=foo -Daccumulo.it.properties=/home/user/my_cluster.properties -Dfailsafe.groups=org.apache.accumulo.test.categories.StandaloneCapableClusterTests -Dspotbugs.skip
+mvn clean verify -Dtest=foo -Daccumulo.it.properties=/home/user/my_cluster.properties -Dfailsafe.groups=StandaloneCapableClusterTests -Dspotbugs.skip

Review comment:
       These tag names can be simplified further. The original names like `SunnyDayTests` was chosen because we had to create a class name that made sense on its own. Now that's not necessary, since the class is deleted, and the fact that we're in a "Test" context is already obvious when using the Tag annotation. So, something like `@Tag("SunnyDay")` should suffice instead of `@Tag("SunnyDayTests")`.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [accumulo] ctubbsii commented on a change in pull request #2545: Migrate test and hadoop-mapreduce modules to JUnit5

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #2545:
URL: https://github.com/apache/accumulo/pull/2545#discussion_r824195351



##########
File path: TESTING.md
##########
@@ -98,7 +98,7 @@ the tests.
 These tests can be run by providing a system property.  Specific ITs can be run using "-Dit.test" or run all tests using:
 
 ```bash
-mvn clean verify -Dtest=foo -Daccumulo.it.properties=/home/user/my_cluster.properties -Dfailsafe.groups=org.apache.accumulo.test.categories.StandaloneCapableClusterTests -Dspotbugs.skip
+mvn clean verify -Dtest=foo -Daccumulo.it.properties=/home/user/my_cluster.properties -Dfailsafe.groups=StandaloneCapableClusterTests -Dspotbugs.skip

Review comment:
       I agree it is superfluous, but it's historical, so it's probably worth keeping. When we initially added these category annotations, I resisted creating a bunch of new profiles with different combinations of activating them, because you can just use the `-Dfailsafe.groups=` property. I still think it's unnecessary to create new profiles, but keeping this one around for historical reasons is probably desirable. If you wanted to simplify it further, you could call it `@Tag("Sunny")` in order to get even closer to the profile name.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [accumulo] DomGarguilo commented on pull request #2545: WIP - Migrate test and hadoop-mapreduce modules to JUnit5

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on pull request #2545:
URL: https://github.com/apache/accumulo/pull/2545#issuecomment-1064472281


   I think this is ready for review. As far as I can tell, everything works as before with all tests passing. I know this is a huge PR to review so this might help when looking through:
   
   > Here are some quick notes that might help to reference for those who work on the migration or for reviewers.
   > 
   > | JUnit4  | JUnit5  |
   > | ----------- | ----------- |
   > | `@Test` | `@Test` |
   > | `@Before` | `@BeforeEach` |
   > | `@After` | `@AfterEach` |
   > | `@BeforeClass` | `@BeforeAll` |
   > | `@AfterClass` | `AfterAll` |
   > | `@Ignore` | `@Disable` |
   > | `@Category` | `@Tag` |
   > 
   > | | JUnit4  | JUnit5  |
   > | ----------- | ----------- | ----------- |
   > | Testing class package | `org.junit` | `org.junit.jupiter.api` |
   > | Assertions class | `Assert` | `Assertions` |
   > 
   > `TemporaryFolder` was removed and instead a `File` or `Path` is used in JUnit5 with the `@TempDir` annotation.
   > 
   > Additionally, the order of parameters in assert statements has changed. The optional message is now the last parameter rather than the first parameter. e.g. `assertTrue("should be equal", 1==3)` becomes `assertTrue(1==3, "should be equal")`
   
   _Originally posted by @DomGarguilo in https://github.com/apache/accumulo/issues/2441#issuecomment-1041908425_
   
   In addition to the things mentioned above, `@Category` has been replaced with `@Tag` which no longer requires an interface so those were deleted as well. This slightly changes how groups of tests are run. for example to run all tests annotated with the `ZooKeeperTestingServerTests` tag, you would run `mvn clean verify -Dtest=foo -Dfailsafe.groups=ZooKeeperTestingServerTests -Dspotbugs.skip` where as previously you would have to specify the category package i.e., `mvn clean verify -Dtest=foo -Dfailsafe.groups=org.apache.accumulo.test.categories.ZooKeeperTestingServerTests -Dspotbugs.skip`


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [accumulo] DomGarguilo commented on pull request #2545: Migrate test and hadoop-mapreduce modules to JUnit5

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on pull request #2545:
URL: https://github.com/apache/accumulo/pull/2545#issuecomment-1069408071


   @ctubbsii I think I addressed all the comments you had in [c150728](https://github.com/apache/accumulo/pull/2545/commits/c1507285b4f52785d4c3e4b9fc6ab3ad17b32185)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [accumulo] DomGarguilo commented on a change in pull request #2545: Migrate test and hadoop-mapreduce modules to JUnit5

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on a change in pull request #2545:
URL: https://github.com/apache/accumulo/pull/2545#discussion_r824189143



##########
File path: TESTING.md
##########
@@ -98,7 +98,7 @@ the tests.
 These tests can be run by providing a system property.  Specific ITs can be run using "-Dit.test" or run all tests using:
 
 ```bash
-mvn clean verify -Dtest=foo -Daccumulo.it.properties=/home/user/my_cluster.properties -Dfailsafe.groups=org.apache.accumulo.test.categories.StandaloneCapableClusterTests -Dspotbugs.skip
+mvn clean verify -Dtest=foo -Daccumulo.it.properties=/home/user/my_cluster.properties -Dfailsafe.groups=StandaloneCapableClusterTests -Dspotbugs.skip

Review comment:
       Good idea. I'm wondering if in that case we could remove the [`-Psunny` profile](https://github.com/apache/accumulo/blob/97af14509d2755de29e2c0196201018f576cab68/pom.xml#L1548-L1555) since to run the same tests now, it would just be `-Dfailsafe.groups=SunnyDay`. The prior is still shorter but just thought I would point it out and see what you/others thought. I assume the profile was created since this group of tests is run often, and before, one would have to type out the full class name (org.apache.accumulo.test.categories.SunnyDayTests).




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [accumulo] ctubbsii commented on a change in pull request #2545: Migrate test and hadoop-mapreduce modules to JUnit5

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #2545:
URL: https://github.com/apache/accumulo/pull/2545#discussion_r828329545



##########
File path: hadoop-mapreduce/src/test/java/org/apache/accumulo/hadoop/WithTestNames.java
##########
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.accumulo;
+package org.apache.accumulo.hadoop;

Review comment:
       This isn't unique enough to ensure that this package will only appear in the test classes/test jar. This package name is still one that could conceivable collide with a package in the main jar for this module if it is ever referenced by an integration test.
   
   If it's only referenced by unit tests, that's fine, because jar sealing won't come into play... because the maven-surefire-plugin runs before the jars are built. However, integration tests run after the package phase when the jars are built, so the classes used for integration tests must not be in packages that could exist in the main jar.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [accumulo] DomGarguilo commented on a change in pull request #2545: Migrate test and hadoop-mapreduce modules to JUnit5

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on a change in pull request #2545:
URL: https://github.com/apache/accumulo/pull/2545#discussion_r828393927



##########
File path: hadoop-mapreduce/src/test/java/org/apache/accumulo/hadoop/WithTestNames.java
##########
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.accumulo;
+package org.apache.accumulo.hadoop;

Review comment:
       I'm not sure I fully understand but I think I moved them into a package that is only used for tests. The new package for this class reads `org.apache.accumulo.hadoop.its`




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [accumulo] ctubbsii commented on a change in pull request #2545: Migrate test and hadoop-mapreduce modules to JUnit5

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #2545:
URL: https://github.com/apache/accumulo/pull/2545#discussion_r826311785



##########
File path: hadoop-mapreduce/src/test/java/org/apache/accumulo/WithTestNames.java
##########
@@ -16,10 +16,27 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.accumulo.test.categories;
+package org.apache.accumulo;

Review comment:
       It looks like this is being used for integration tests. If that's the case, then this needs to be in a unique test directory for this specific test module so no other package collides in any jar, so that it doesn't cause problems with jar sealing.
   
   This could be in the the `org.apache.accumulo.hadoop.testing` package for example, so it could not possibly collide with anything in any other jar.

##########
File path: hadoop-mapreduce/src/test/java/org/apache/accumulo/hadoop/its/mapred/AccumuloOutputFormatIT.java
##########
@@ -73,7 +73,7 @@ protected void configure(MiniAccumuloConfigImpl cfg, Configuration hadoopCoreSit
   public void testMapred() throws Exception {
     try (AccumuloClient client = Accumulo.newClient().from(getClientProperties()).build()) {
       // create a table and put some data in it
-      client.tableOperations().create(testName.getMethodName());
+      client.tableOperations().create(testName());

Review comment:
       This table name should really be put in its own variable, so `testName()` is only called once, and it's obvious what it's being used for later.

##########
File path: test/src/main/java/org/apache/accumulo/WithTestNames.java
##########
@@ -16,10 +16,27 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.accumulo.test.categories;
+package org.apache.accumulo;

Review comment:
       Same comment as before, about this being in a unique package for this module.

##########
File path: test/src/main/java/org/apache/accumulo/harness/TestingKdc.java
##########
@@ -62,8 +62,8 @@ public static File computeKdcDir() {
     File targetDir = new File(System.getProperty("user.dir"), "target");
     if (!targetDir.exists())
       assertTrue(targetDir.mkdirs());
-    assertTrue("Could not find Maven target directory: " + targetDir,
-        targetDir.exists() && targetDir.isDirectory());
+    assertTrue(targetDir.exists() && targetDir.isDirectory(),

Review comment:
       Exists is redundant here. If it's a directory, it exists. I know it's not your change, but could clean it up while in here.

##########
File path: test/src/main/java/org/apache/accumulo/harness/AccumuloClusterHarness.java
##########
@@ -60,7 +59,7 @@
  * advanced ITs that do crazy things. For more typical, expected behavior of a cluster see
  * {@link SharedMiniClusterBase}. This instance can be MAC or a standalone instance.
  */
-@Category(StandaloneCapableClusterTests.class)
+@Tag("StandaloneCapableCluster")

Review comment:
       It'd be very easy to have typos in these strings. These string literals could be constants, as in:
   
   ```suggestion
   @Tag(ConstantsForTesting.STANDALONE_CAPABLE_CLUSTER)
   ```
   
   That way, there's no chance of mistyping the category, and it not running when you select that category.

##########
File path: test/src/main/java/org/apache/accumulo/test/fate/zookeeper/ServiceLockIT.java
##########
@@ -405,7 +409,8 @@ public void testUnexpectedEvent() throws Exception {
 
   }
 
-  @Test(timeout = 60000)
+  @Test
+  @Timeout(value = 1, unit = MINUTES)

Review comment:
       These should probably just show 60. For timeouts less than or equal to a minute, it's probably simpler to just show seconds:
   
   ```suggestion
     @Timeout(60)
   ```

##########
File path: test/src/main/java/org/apache/accumulo/test/functional/KerberosRenewalIT.java
##########
@@ -137,15 +138,16 @@ public void configureMiniCluster(MiniAccumuloConfigImpl cfg, Configuration coreS
     UserGroupInformation.setConfiguration(conf);
   }
 
-  @After
+  @AfterEach
   public void stopMac() throws Exception {
     if (mac != null) {
       mac.stop();
     }
   }
 
   // Intentionally setting the Test annotation timeout. We do not want to scale the timeout.
-  @Test(timeout = TEST_DURATION)
+  @Test
+  @Timeout(value = TEST_DURATION_MINUTES, unit = MINUTES)

Review comment:
       Does the registered extension still scale these when the timeout is explicitly set this way? If it does, we probably need to modify the extension to skip these that have their timeout explicitly set (if possible).




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [accumulo] DomGarguilo commented on a change in pull request #2545: Migrate test and hadoop-mapreduce modules to JUnit5

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on a change in pull request #2545:
URL: https://github.com/apache/accumulo/pull/2545#discussion_r828312930



##########
File path: test/src/main/java/org/apache/accumulo/harness/AccumuloClusterHarness.java
##########
@@ -60,7 +59,7 @@
  * advanced ITs that do crazy things. For more typical, expected behavior of a cluster see
  * {@link SharedMiniClusterBase}. This instance can be MAC or a standalone instance.
  */
-@Category(StandaloneCapableClusterTests.class)
+@Tag("StandaloneCapableCluster")

Review comment:
       I added constants, where possible, to AccumuloITBase.java but for the single `@Tag` that is used outside of accumulo-test module ([here](https://github.com/apache/accumulo/blob/c1507285b4f52785d4c3e4b9fc6ab3ad17b32185/server/monitor/src/test/java/org/apache/accumulo/monitor/it/WebViewsIT.java#L63)), I had to create [a class to store the constant](https://github.com/apache/accumulo/blob/c1507285b4f52785d4c3e4b9fc6ab3ad17b32185/server/monitor/src/test/java/org/apache/accumulo/monitor/TagNameConstants.java). Please let me know what you think of this approach and if you have any alternatives in mind.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [accumulo] DomGarguilo commented on a change in pull request #2545: Migrate test and hadoop-mapreduce modules to JUnit5

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on a change in pull request #2545:
URL: https://github.com/apache/accumulo/pull/2545#discussion_r828245722



##########
File path: test/src/main/java/org/apache/accumulo/test/functional/KerberosRenewalIT.java
##########
@@ -137,15 +138,16 @@ public void configureMiniCluster(MiniAccumuloConfigImpl cfg, Configuration coreS
     UserGroupInformation.setConfiguration(conf);
   }
 
-  @After
+  @AfterEach
   public void stopMac() throws Exception {
     if (mac != null) {
       mac.stop();
     }
   }
 
   // Intentionally setting the Test annotation timeout. We do not want to scale the timeout.
-  @Test(timeout = TEST_DURATION)
+  @Test
+  @Timeout(value = TEST_DURATION_MINUTES, unit = MINUTES)

Review comment:
       No, these explicitly set timeouts are not scaled and take precedence over the registered extension.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org