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? -- 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: common-issues-unsubscr...@hadoop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
