[ 
https://issues.apache.org/jira/browse/HADOOP-19415?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17950443#comment-17950443
 ] 

ASF GitHub Bot commented on HADOOP-19415:
-----------------------------------------

anujmodi2021 commented on code in PR #7419:
URL: https://github.com/apache/hadoop/pull/7419#discussion_r2080945394


##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractAppendTest.java:
##########
@@ -41,6 +42,7 @@ public abstract class AbstractContractAppendTest extends 
AbstractFSContractTestB
   private Path testPath;
   private Path target;
 
+  @BeforeEach

Review Comment:
   Earlier there was no @Before tag here.
   How was setup done? Why we need it now?



##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/extend/TestName.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * 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.hadoop.fs.extend;

Review Comment:
   Seems like there is a similar class present in `org.apache.hadoop.test` 
package. Are we planning to replace that?



##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/rawlocal/TestRawlocalContractPathHandle.java:
##########
@@ -18,17 +18,13 @@
 package org.apache.hadoop.fs.contract.rawlocal;
 
 import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.fs.Options;
 import org.apache.hadoop.fs.contract.AbstractContractPathHandleTest;
 import org.apache.hadoop.fs.contract.AbstractFSContract;
-import org.apache.hadoop.fs.contract.rawlocal.RawlocalFSContract;
 
 public class TestRawlocalContractPathHandle
     extends AbstractContractPathHandleTest {
 
-  public TestRawlocalContractPathHandle(String testname,
-      Options.HandleOpt[] opts, boolean serialized) {
-    super(testname, opts, serialized);
+  public TestRawlocalContractPathHandle() {

Review Comment:
   Is implementatios removed because there were no usage?



##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/localfs/TestLocalFSContractLoaded.java:
##########
@@ -38,16 +38,16 @@ protected AbstractFSContract createContract(Configuration 
conf) {
   @Test
   public void testContractWorks() throws Throwable {
     String key = getContract().getConfKey(SUPPORTS_ATOMIC_RENAME);
-    assertNotNull("not set: " + key, getContract().getConf().get(key));
-    assertTrue("not true: " + key,
-               getContract().isSupported(SUPPORTS_ATOMIC_RENAME, false));
+    assertNotNull(getContract().getConf().get(key), "not set: " + key);
+    assertTrue(getContract().isSupported(SUPPORTS_ATOMIC_RENAME, false),
+        "not true: " + key);
   }
 
   @Test
   public void testContractResourceOnClasspath() throws Throwable {
     URL url = this.getClass()
                        .getClassLoader()
                        .getResource(LocalFSContract.CONTRACT_XML);
-    assertNotNull("could not find contract resource", url);
+    assertNotNull(url, "could not find contract resource");
   }
 }

Review Comment:
   EOF



##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/auth/delegation/ITestSessionDelegationInFilesystem.java:
##########
@@ -306,21 +307,22 @@ public void testCanRetrieveTokenFromCurrentUserCreds() 
throws Throwable {
     LOG.info("Token = " + token0);
     Token<?> token1 = requireNonNull(
         ugi.getCredentials().getToken(service), "Token from " + service);
-    assertEquals("retrieved token", token0, token1);
-    assertNotNull("token identifier of "  + token1,
-        token1.getIdentifier());
+    assertEquals(token0, token1, "retrieved token");
+    assertNotNull(token1.getIdentifier(),
+        "token identifier of "  + token1);
   }
 
   @Test
   public void testDTCredentialProviderFromCurrentUserCreds() throws Throwable {
     describe("Add credentials to the current user, "
         + "then verify that they can be found when S3ADelegationTokens binds");
     Credentials cred = createDelegationTokens();
-    assertThat("Token size", cred.getAllTokens(), hasSize(1));
+    assertThat(cred.getAllTokens()).hasSize(1).

Review Comment:
   This assertThat method is from `org.assertj.core.api` and other assert 
related methods like assertTrue are from `org.junit.jupiter.api`
   What is the general guideline here to follow for deciding which one to go 
with?
   Or is the goal here to just get rid of all `org.junit.Assert` methods and 
other two can be used as per developers preference?





> Upgrade JUnit from 4 to 5 in hadoop-common.
> -------------------------------------------
>
>                 Key: HADOOP-19415
>                 URL: https://issues.apache.org/jira/browse/HADOOP-19415
>             Project: Hadoop Common
>          Issue Type: Sub-task
>            Reporter: Shilun Fan
>            Assignee: Shilun Fan
>            Priority: Major
>              Labels: pull-request-available
>




--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

Reply via email to