Skip to content

HADOOP-19425. [JDK17] Upgrade JUnit from 4 to 5 in hadoop-azure Part2. #7653

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
slfan1989 merged 6 commits into from
Jul 2, 2025
Merged

HADOOP-19425. [JDK17] Upgrade JUnit from 4 to 5 in hadoop-azure Part2. #7653

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7c44bf5
HADOOP-19425. [JDK17] Upgrade JUnit from 4 to 5 in hadoop-azure Part2.
slfan1989 May 5, 2025
2853939
Merge branch 'trunk' into HADOOP-19425-PART2
slfan1989 Jun 29, 2025
2da8148
HADOOP-19426. [JDK17] Upgrade JUnit from 4 to 5 in hadoop-azure-datal…
slfan1989 Jun 29, 2025
765f9d0
HADOOP-19425. Fix CheckStyle.
slfan1989 Jun 29, 2025
b0a5576
HADOOP-19425. Fix Error Junit Test.
slfan1989 Jul 1, 2025
c5127b1
HADOOP-19425. Improve Some Code.
slfan1989 Jul 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/integration/AzureTestConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ public interface AzureTestConstants {
*/
int SCALE_TEST_TIMEOUT_SECONDS = 30 * 60;

int SCALE_TEST_TIMEOUT_MILLIS = SCALE_TEST_TIMEOUT_SECONDS * 1000;
int SCALE_TEST_TIMEOUT_MILLIS = SCALE_TEST_TIMEOUT_SECONDS;
Copy link
Contributor

@anujmodi2021 anujmodi2021 May 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that Timeout in Junit5 is supposed to be defined in seconds, this is logically fine, but this does not sound right. May be we should get rid of the redundant variable here or rename it aptly to denote timeout in seconds only.

Copy link
Contributor Author

@slfan1989 slfan1989 Jun 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much for your suggestion! I have thought of a solution that has less impact on the code: when introducing Timeout, we can explicitly specify the time unit, so we won't need to modify the variable anymore.

Timeout(value = AzureTestConstants.AZURE_TEST_TIMEOUT, unit = TimeUnit.MILLISECONDS)




Expand Down
2 changes: 1 addition & 1 deletion ...op-azure/src/test/java/org/apache/hadoop/fs/azurebfs/constants/TestConfigurationKeys.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public final class TestConfigurationKeys {

public static final String TEST_CONFIGURATION_FILE_NAME = "azure-test.xml";
public static final String TEST_CONTAINER_PREFIX = "abfs-testcontainer-";
public static final int TEST_TIMEOUT = 15 * 60 * 1000;
public static final int TEST_TIMEOUT = 15 * 60;
Copy link
Contributor

@anujmodi2021 anujmodi2021 May 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we rename it as well to TEST_TIMEOUT_IN_SEC ?

Copy link
Contributor Author

@slfan1989 slfan1989 Jun 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned in the previous feedback, we can avoid modifying this variable.

public static final int ENCRYPTION_KEY_LEN = 32;

private TestConfigurationKeys() {}
Expand Down
16 changes: 8 additions & 8 deletions ...oop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/contract/TestBlobListXmlParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@
import javax.xml.parsers.SAXParserFactory;
import java.io.ByteArrayInputStream;
import java.io.InputStream;
import org.assertj.core.api.Assertions;
import java.util.List;

import org.junit.Test;
import org.junit.jupiter.api.Test;
import org.xml.sax.SAXException;

import org.apache.hadoop.fs.azurebfs.contracts.services.BlobListResultEntrySchema;
import org.apache.hadoop.fs.azurebfs.contracts.services.BlobListResultSchema;
import org.apache.hadoop.fs.azurebfs.contracts.services.BlobListXmlParser;
import static org.assertj.core.api.Assertions.assertThat;

public class TestBlobListXmlParser {
@Test
Expand Down Expand Up @@ -105,11 +105,11 @@ public void testXMLParser() throws Exception {
+ "</EnumerationResults>";
BlobListResultSchema listResultSchema = getResultSchema(xmlResponseWithDelimiter);
List<BlobListResultEntrySchema> paths = listResultSchema.paths();
Assertions.assertThat(paths.size()).isEqualTo(4);
Assertions.assertThat(paths.get(0).isDirectory()).isEqualTo(true);
Assertions.assertThat(paths.get(1).isDirectory()).isEqualTo(true);
Assertions.assertThat(paths.get(2).isDirectory()).isEqualTo(true);
Assertions.assertThat(paths.get(3).isDirectory()).isEqualTo(false);
assertThat(paths.size()).isEqualTo(4);
assertThat(paths.get(0).isDirectory()).isEqualTo(true);
assertThat(paths.get(1).isDirectory()).isEqualTo(true);
assertThat(paths.get(2).isDirectory()).isEqualTo(true);
assertThat(paths.get(3).isDirectory()).isEqualTo(false);
}

@Test
Expand All @@ -123,7 +123,7 @@ public void testEmptyBlobList() throws Exception {
+ "</EnumerationResults>";
BlobListResultSchema listResultSchema = getResultSchema(xmlResponse);
List<BlobListResultEntrySchema> paths = listResultSchema.paths();
Assertions.assertThat(paths.size()).isEqualTo(0);
assertThat(paths.size()).isEqualTo(0);
}

private static final ThreadLocal<SAXParser> SAX_PARSER_THREAD_LOCAL
Expand Down
2 changes: 1 addition & 1 deletion ...oop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/contract/TestDfsListJsonParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import java.io.IOException;

import com.fasterxml.jackson.databind.ObjectMapper;
import org.junit.Test;
import org.junit.jupiter.api.Test;

import org.apache.hadoop.fs.azurebfs.contracts.services.DfsListResultSchema;
import org.apache.hadoop.fs.azurebfs.contracts.services.ListResultEntrySchema;
Expand Down
67 changes: 42 additions & 25 deletions .../src/test/java/org/apache/hadoop/fs/azurebfs/diagnostics/TestConfigurationValidators.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@

package org.apache.hadoop.fs.azurebfs.diagnostics;

import org.junit.Assert;
import org.junit.Test;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

import org.apache.hadoop.fs.azurebfs.contracts.exceptions.InvalidConfigurationValueException;
import org.apache.hadoop.fs.azurebfs.utils.Base64;
Expand All @@ -36,7 +36,7 @@
/**
* Test configuration validators.
*/
public class TestConfigurationValidators extends Assert {
public class TestConfigurationValidators extends Assertions {

private static final String FAKE_KEY = "FakeKey";

Expand All @@ -54,11 +54,14 @@ public void testIntegerConfigValidator() throws Exception {
assertEquals(MAX_BUFFER_SIZE, (int) integerConfigurationValidator.validate("104857600"));
}

@Test(expected = InvalidConfigurationValueException.class)
@Test
public void testIntegerConfigValidatorThrowsIfMissingValidValue() throws Exception {
IntegerConfigurationBasicValidator integerConfigurationValidator = new IntegerConfigurationBasicValidator(
MIN_BUFFER_SIZE, MAX_BUFFER_SIZE, DEFAULT_READ_BUFFER_SIZE, FAKE_KEY, true);
integerConfigurationValidator.validate("3072");
assertThrows(InvalidConfigurationValueException.class, () -> {
IntegerConfigurationBasicValidator integerConfigurationValidator =
new IntegerConfigurationBasicValidator(
MIN_BUFFER_SIZE, MAX_BUFFER_SIZE, DEFAULT_READ_BUFFER_SIZE, FAKE_KEY, true);
integerConfigurationValidator.validate("3072");
});
}

@Test
Expand All @@ -73,12 +76,15 @@ public void testIntegerWithOutlierConfigValidator() throws Exception {
assertEquals(MAX_LEASE_DURATION, (int) integerConfigurationValidator.validate("60"));
}

@Test(expected = InvalidConfigurationValueException.class)
@Test
public void testIntegerWithOutlierConfigValidatorThrowsIfMissingValidValue() throws Exception {
IntegerConfigurationBasicValidator integerConfigurationValidator = new IntegerConfigurationBasicValidator(
INFINITE_LEASE_DURATION, MIN_LEASE_DURATION, MAX_LEASE_DURATION, DEFAULT_LEASE_DURATION, FAKE_KEY,
true);
integerConfigurationValidator.validate("14");
assertThrows(InvalidConfigurationValueException.class, () -> {
IntegerConfigurationBasicValidator integerConfigurationValidator =
new IntegerConfigurationBasicValidator(
INFINITE_LEASE_DURATION, MIN_LEASE_DURATION, MAX_LEASE_DURATION, DEFAULT_LEASE_DURATION, FAKE_KEY,
true);
integerConfigurationValidator.validate("14");
});
}

@Test
Expand All @@ -91,11 +97,13 @@ public void testLongConfigValidator() throws Exception {
assertEquals(MAX_BUFFER_SIZE, (long) longConfigurationValidator.validate("104857600"));
}

@Test(expected = InvalidConfigurationValueException.class)
@Test
public void testLongConfigValidatorThrowsIfMissingValidValue() throws Exception {
LongConfigurationBasicValidator longConfigurationValidator = new LongConfigurationBasicValidator(
MIN_BUFFER_SIZE, MAX_BUFFER_SIZE, DEFAULT_READ_BUFFER_SIZE, FAKE_KEY, true);
longConfigurationValidator.validate(null);
assertThrows(InvalidConfigurationValueException.class, () -> {
LongConfigurationBasicValidator longConfigurationValidator = new LongConfigurationBasicValidator(
MIN_BUFFER_SIZE, MAX_BUFFER_SIZE, DEFAULT_READ_BUFFER_SIZE, FAKE_KEY, true);
longConfigurationValidator.validate(null);
});
}

@Test
Expand All @@ -107,10 +115,13 @@ public void testBooleanConfigValidator() throws Exception {
assertEquals(false, booleanConfigurationValidator.validate(null));
}

@Test(expected = InvalidConfigurationValueException.class)
@Test
public void testBooleanConfigValidatorThrowsIfMissingValidValue() throws Exception {
BooleanConfigurationBasicValidator booleanConfigurationValidator = new BooleanConfigurationBasicValidator(FAKE_KEY, false, true);
booleanConfigurationValidator.validate("almostTrue");
assertThrows(InvalidConfigurationValueException.class, () -> {
BooleanConfigurationBasicValidator booleanConfigurationValidator =
new BooleanConfigurationBasicValidator(FAKE_KEY, false, true);
booleanConfigurationValidator.validate("almostTrue");
});
}

@Test
Expand All @@ -121,10 +132,13 @@ public void testStringConfigValidator() throws Exception {
assertEquals("someValue", stringConfigurationValidator.validate("someValue"));
}

@Test(expected = InvalidConfigurationValueException.class)
@Test
public void testStringConfigValidatorThrowsIfMissingValidValue() throws Exception {
StringConfigurationBasicValidator stringConfigurationValidator = new StringConfigurationBasicValidator(FAKE_KEY, "value", true);
stringConfigurationValidator.validate(null);
assertThrows(InvalidConfigurationValueException.class, () -> {
StringConfigurationBasicValidator stringConfigurationValidator =
new StringConfigurationBasicValidator(FAKE_KEY, "value", true);
stringConfigurationValidator.validate(null);
});
}

@Test
Expand All @@ -136,9 +150,12 @@ public void testBase64StringConfigValidator() throws Exception {
assertEquals(encodedVal, base64StringConfigurationValidator.validate(encodedVal));
}

@Test(expected = InvalidConfigurationValueException.class)
@Test
public void testBase64StringConfigValidatorThrowsIfMissingValidValue() throws Exception {
Base64StringConfigurationBasicValidator base64StringConfigurationValidator = new Base64StringConfigurationBasicValidator(FAKE_KEY, "value", true);
base64StringConfigurationValidator.validate("some&%Value");
assertThrows(InvalidConfigurationValueException.class, () -> {
Base64StringConfigurationBasicValidator base64StringConfigurationValidator =
new Base64StringConfigurationBasicValidator(FAKE_KEY, "value", true);
base64StringConfigurationValidator.validate("some&%Value");
});
}
}
6 changes: 3 additions & 3 deletions ...p-azure/src/test/java/org/apache/hadoop/fs/azurebfs/extensions/KerberizedAbfsCluster.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@

import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_SECURITY_AUTHENTICATION;
import static org.apache.hadoop.security.UserGroupInformation.loginUserFromKeytabAndReturnUGI;
import static org.junit.Assert.assertTrue;
import static org.junit.jupiter.api.Assertions.assertTrue;

/**
* composite service for adding kerberos login for ABFS
Expand Down Expand Up @@ -256,8 +256,8 @@ public void loginPrincipal() throws IOException {
* General assertion that security is turred on for a cluster.
*/
public static void assertSecurityEnabled() {
assertTrue("Security is needed for this test",
UserGroupInformation.isSecurityEnabled());
assertTrue(UserGroupInformation.isSecurityEnabled(),
"Security is needed for this test");
}


Expand Down
2 changes: 1 addition & 1 deletion .../test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsInputStreamSmallFileReads.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import java.io.IOException;
import java.util.Map;

import org.junit.Test;
import org.junit.jupiter.api.Test;

import org.apache.hadoop.fs.azurebfs.AbfsConfiguration;
import org.apache.hadoop.fs.FileSystem;
Expand Down
32 changes: 16 additions & 16 deletions ...op-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestAbfsBackoffMetrics.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,8 @@

package org.apache.hadoop.fs.azurebfs.services;

import org.assertj.core.api.Assertions;
import org.junit.Before;
import org.junit.Test;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import static org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.NUMBER_OF_REQUESTS_SUCCEEDED;
import static org.apache.hadoop.fs.azurebfs.enums.AbfsBackoffMetricsEnum.TOTAL_NUMBER_OF_REQUESTS;
Expand All @@ -29,6 +28,7 @@
import static org.apache.hadoop.fs.azurebfs.enums.RetryValue.TWO;
import static org.apache.hadoop.fs.azurebfs.enums.StatisticTypeEnum.TYPE_COUNTER;
import static org.apache.hadoop.fs.azurebfs.enums.StatisticTypeEnum.TYPE_GAUGE;
import static org.assertj.core.api.Assertions.assertThat;

public class TestAbfsBackoffMetrics {
private AbfsBackoffMetrics metrics;
Expand All @@ -38,7 +38,7 @@ public class TestAbfsBackoffMetrics {
/**
* Sets up the test environment by initializing the AbfsBackoffMetrics instance.
*/
@Before
@BeforeEach
public void setUp() {
metrics = new AbfsBackoffMetrics();
}
Expand All @@ -48,12 +48,12 @@ public void setUp() {
*/
@Test
public void retrievesMetricNamesBasedOnStatisticType() {
String[] counterMetrics = metrics.getMetricNamesByType(TYPE_COUNTER);
String[] gaugeMetrics = metrics.getMetricNamesByType(TYPE_GAUGE);
Assertions.assertThat(counterMetrics.length)
String[] counterMetrics = metrics.getMetricNamesByType(TYPE_COUNTER);
String[] gaugeMetrics = metrics.getMetricNamesByType(TYPE_GAUGE);
assertThat(counterMetrics.length)
.describedAs("Counter metrics should have 22 elements")
.isEqualTo(TOTAL_COUNTERS);
Assertions.assertThat(gaugeMetrics.length)
assertThat(gaugeMetrics.length)
.describedAs("Gauge metrics should have 21 elements")
.isEqualTo(TOTAL_GAUGES);
}
Expand All @@ -64,10 +64,10 @@ public void retrievesMetricNamesBasedOnStatisticType() {
@Test
public void retrievesValueOfSpecificMetric() {
metrics.setMetricValue(NUMBER_OF_REQUESTS_SUCCEEDED, 5, ONE);
Assertions.assertThat(metrics.getMetricValue(NUMBER_OF_REQUESTS_SUCCEEDED, ONE))
assertThat(metrics.getMetricValue(NUMBER_OF_REQUESTS_SUCCEEDED, ONE))
.describedAs("Number of request succeeded for retry 1 should be 5")
.isEqualTo(5);
Assertions.assertThat(metrics.getMetricValue(NUMBER_OF_REQUESTS_SUCCEEDED, TWO))
assertThat(metrics.getMetricValue(NUMBER_OF_REQUESTS_SUCCEEDED, TWO))
.describedAs("Number of request succeeded for other retries except 1 should be 0")
.isEqualTo(0);
}
Expand All @@ -78,10 +78,10 @@ public void retrievesValueOfSpecificMetric() {
@Test
public void incrementsValueOfSpecificMetric() {
metrics.incrementMetricValue(NUMBER_OF_REQUESTS_SUCCEEDED, ONE);
Assertions.assertThat(metrics.getMetricValue(NUMBER_OF_REQUESTS_SUCCEEDED, ONE))
assertThat(metrics.getMetricValue(NUMBER_OF_REQUESTS_SUCCEEDED, ONE))
.describedAs("Number of request succeeded for retry 1 should be 1")
.isEqualTo(1);
Assertions.assertThat(metrics.getMetricValue(NUMBER_OF_REQUESTS_SUCCEEDED, THREE))
assertThat(metrics.getMetricValue(NUMBER_OF_REQUESTS_SUCCEEDED, THREE))
.describedAs("Number of request succeeded for other retries except 1 should be 0")
.isEqualTo(0);
}
Expand All @@ -91,10 +91,10 @@ public void incrementsValueOfSpecificMetric() {
*/
@Test
public void returnsStringRepresentationOfEmptyBackoffMetrics() {
Assertions.assertThat(metrics.getMetricValue(TOTAL_NUMBER_OF_REQUESTS))
assertThat(metrics.getMetricValue(TOTAL_NUMBER_OF_REQUESTS))
.describedAs("String representation of backoff metrics should be empty")
.isEqualTo(0);
Assertions.assertThat(metrics.toString())
assertThat(metrics.toString())
.describedAs("String representation of backoff metrics should be empty")
.isEmpty();
}
Expand All @@ -105,10 +105,10 @@ public void returnsStringRepresentationOfEmptyBackoffMetrics() {
@Test
public void returnsStringRepresentationOfBackoffMetrics() {
metrics.incrementMetricValue(TOTAL_NUMBER_OF_REQUESTS);
Assertions.assertThat(metrics.getMetricValue(TOTAL_NUMBER_OF_REQUESTS))
assertThat(metrics.getMetricValue(TOTAL_NUMBER_OF_REQUESTS))
.describedAs("String representation of backoff metrics should not be empty")
.isEqualTo(1);
Assertions.assertThat(metrics.toString())
assertThat(metrics.toString())
.describedAs("String representation of backoff metrics should not be empty")
.contains("$TR=1");
}
Expand Down
Loading