[ 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