Skip to content

HADOOP-19431. [JDK17] Upgrade JUnit from 4 to 5 in hadoop-distcp. #7368

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
merged 6 commits into from
Feb 10, 2025
Merged

HADOOP-19431. [JDK17] Upgrade JUnit from 4 to 5 in hadoop-distcp. #7368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
889e430
HADOOP-19431. Upgrade JUnit from 4 to 5 in hadoop-distcp.
Feb 8, 2025
febabee
HADOOP-19431. Fix Unit test.
Feb 8, 2025
023b1c9
HADOOP-19431. Fix Unit test & CheckStyle.
Feb 8, 2025
53f18d6
HADOOP-19431. Fix CheckStyle.
Feb 8, 2025
55a05f7
HADOOP-19431. Fix CheckStyle.
Feb 8, 2025
7d8d0c2
HADOOP-19431. Add VisibleForTesting On SimpleCopyListing#initSimpleCo…
Feb 9, 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
15 changes: 13 additions & 2 deletions hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/SimpleCopyListing.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ public class SimpleCopyListing extends CopyListing {
private long totalDirs = 0;
private long totalBytesToCopy = 0;
private int numListstatusThreads = 1;
private final int fileStatusLimit;
private final boolean randomizeFileListing;
private int fileStatusLimit;
private boolean randomizeFileListing;
private final int maxRetries = 3;
private CopyFilter copyFilter;
private DistCpSync distCpSync;
Expand Down Expand Up @@ -119,6 +119,17 @@ protected SimpleCopyListing(Configuration configuration,
this.randomizeFileListing = randomizeFileListing;
}

@VisibleForTesting
protected void initSimpleCopyListing(Configuration pConfiguration,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please mark this as VisibleForTesting?

Maybe TestCopyListing shouldn't be extending SimpleCopyListing, but no need to take on further refactoring right now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you please mark this as VisibleForTesting?

Thank you for your explanation! I have added the VisibleForTesting annotation above the initSimpleCopyListing function.

Maybe TestCopyListing shouldn't be extending SimpleCopyListing, but no need to take on further refactoring right now.

This is a great suggestion! I will record it as a to-do. Once we complete the upgrade from JUnit4 to JUnit5, we will implement this optimization.

Credentials pCredentials, int pNumListstatusThreads, int pFileStatusLimit,
boolean pRandomizeFileListing) {
setConf(pConfiguration);
setCredentials(pCredentials);
this.numListstatusThreads = pNumListstatusThreads;
this.fileStatusLimit = Math.max(1, pFileStatusLimit);
this.randomizeFileListing = pRandomizeFileListing;
}

protected SimpleCopyListing(Configuration configuration,
Credentials credentials,
DistCpSync distCpSync) {
Expand Down
16 changes: 8 additions & 8 deletions hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestCopyFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@

package org.apache.hadoop.tools;

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

import org.apache.hadoop.conf.Configuration;

import static org.apache.hadoop.test.LambdaTestUtils.intercept;
import static org.junit.Assert.assertTrue;
import static org.junit.jupiter.api.Assertions.assertTrue;

/**
* Test {@link CopyFilter}.
Expand All @@ -34,17 +34,17 @@ public class TestCopyFilter {
public void testGetCopyFilterTrueCopyFilter() {
Configuration configuration = new Configuration(false);
CopyFilter copyFilter = CopyFilter.getCopyFilter(configuration);
assertTrue("copyFilter should be instance of TrueCopyFilter",
copyFilter instanceof TrueCopyFilter);
assertTrue(copyFilter instanceof TrueCopyFilter,
"copyFilter should be instance of TrueCopyFilter");
}

@Test
public void testGetCopyFilterRegexCopyFilter() {
Configuration configuration = new Configuration(false);
configuration.set(DistCpConstants.CONF_LABEL_FILTERS_FILE, "random");
CopyFilter copyFilter = CopyFilter.getCopyFilter(configuration);
assertTrue("copyFilter should be instance of RegexCopyFilter",
copyFilter instanceof RegexCopyFilter);
assertTrue(copyFilter instanceof RegexCopyFilter,
"copyFilter should be instance of RegexCopyFilter");
}

@Test
Expand All @@ -54,8 +54,8 @@ public void testGetCopyFilterRegexpInConfigurationFilter() {
Configuration configuration = new Configuration(false);
configuration.set(DistCpConstants.CONF_LABEL_FILTERS_CLASS, filterName);
CopyFilter copyFilter = CopyFilter.getCopyFilter(configuration);
assertTrue("copyFilter should be instance of RegexpInConfigurationFilter",
copyFilter instanceof RegexpInConfigurationFilter);
assertTrue(copyFilter instanceof RegexpInConfigurationFilter,
"copyFilter should be instance of RegexpInConfigurationFilter");
}

@Test
Expand Down
Loading