Skip to content
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.

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

MAPREDUCE-7418. [JDK17] Upgrade Junit 4 to 5 in hadoop-mapreduce-client-app. #7350

Merged
merged 6 commits into from
Feb 6, 2025
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,10 @@
<artifactId>junit-vintage-engine</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-junit-jupiter</artifactId>
</dependency>
</dependencies>

<build>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,17 @@
import org.apache.hadoop.yarn.api.records.NodeId;
import org.apache.hadoop.yarn.event.Event;
import org.apache.hadoop.yarn.event.EventHandler;
import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Timeout;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import static org.junit.jupiter.api.Assertions.assertNotEquals;

public class TestLocalContainerLauncher {
private static final Logger LOG =
LoggerFactory.getLogger(TestLocalContainerLauncher.class);
Expand All @@ -75,7 +77,7 @@ private static void delete(File dir) throws IOException {
fs.delete(p, true);
}

@BeforeClass
@BeforeAll
public static void setupTestDirs() throws IOException {
testWorkDir = new File("target",
TestLocalContainerLauncher.class.getCanonicalName());
Expand All @@ -89,15 +91,16 @@ public static void setupTestDirs() throws IOException {
}
}

@AfterClass
@AfterAll
public static void cleanupTestDirs() throws IOException {
if (testWorkDir != null) {
delete(testWorkDir);
}
}

@SuppressWarnings("rawtypes")
@Test(timeout=10000)
@Test
@Timeout(value = 10)
public void testKillJob() throws Exception {
JobConf conf = new JobConf();
AppContext context = mock(AppContext.class);
Expand Down Expand Up @@ -198,8 +201,8 @@ public void testRenameMapOutputForReduce() throws Exception {
final Path mapOut = mrOutputFiles.getOutputFileForWrite(1);
conf.set(MRConfig.LOCAL_DIR, localDirs[1].toString());
final Path mapOutIdx = mrOutputFiles.getOutputIndexFileForWrite(1);
Assert.assertNotEquals("Paths must be different!",
mapOut.getParent(), mapOutIdx.getParent());
assertNotEquals(mapOut.getParent(), mapOutIdx.getParent(),
"Paths must be different!");

// make both dirs part of LOCAL_DIR
conf.setStrings(MRConfig.LOCAL_DIR, localDirs);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@
import org.apache.hadoop.yarn.event.EventHandler;
import org.apache.hadoop.yarn.util.SystemClock;

import org.junit.Test;
import static org.junit.Assert.assertTrue;
import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

Expand Down Expand Up @@ -87,7 +87,7 @@ public void testFinshingAttemptTimeout()
}
taskAttemptFinishingMonitor.stop();

assertTrue("Finishing attempt didn't time out.", eventHandler.timedOut);
assertTrue(eventHandler.timedOut, "Finishing attempt didn't time out.");

}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,19 @@

import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Supplier;

import org.junit.After;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Timeout;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.ArgumentCaptor;
import org.mockito.Captor;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnitRunner;
import org.mockito.junit.jupiter.MockitoExtension;

import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.Path;
Expand Down Expand Up @@ -69,12 +69,12 @@
import org.apache.hadoop.yarn.util.SystemClock;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.eq;
Expand All @@ -87,7 +87,7 @@
/**
* Tests the behavior of TaskAttemptListenerImpl.
*/
@RunWith(MockitoJUnitRunner.class)
@ExtendWith(MockitoExtension.class)
public class TestTaskAttemptListenerImpl {
private static final String ATTEMPT1_ID =
"attempt_123456789012_0001_m_000001_0";
Expand Down Expand Up @@ -172,15 +172,16 @@ protected void stopRpcServer() {
}
}

@After
@AfterEach
public void after() throws IOException {
if (listener != null) {
listener.close();
listener = null;
}
}

@Test (timeout=5000)
@Test
@Timeout(value = 5)
public void testGetTask() throws IOException {
configureMocks();
startListener(false);
Expand Down Expand Up @@ -238,7 +239,8 @@ public void testGetTask() throws IOException {

}

@Test (timeout=5000)
@Test
@Timeout(value = 5)
public void testJVMId() {

JVMId jvmid = new JVMId("test", 1, true, 2);
Expand All @@ -247,7 +249,8 @@ public void testJVMId() {
assertEquals(0, jvmid.compareTo(jvmid1));
}

@Test (timeout=10000)
@Test
@Timeout(value = 10)
public void testGetMapCompletionEvents() throws IOException {
TaskAttemptCompletionEvent[] empty = {};
TaskAttemptCompletionEvent[] taskEvents = {
Expand All @@ -257,12 +260,6 @@ public void testGetMapCompletionEvents() throws IOException {
createTce(3, false, TaskAttemptCompletionEventStatus.FAILED) };
TaskAttemptCompletionEvent[] mapEvents = { taskEvents[0], taskEvents[2] };
Job mockJob = mock(Job.class);
when(mockJob.getTaskAttemptCompletionEvents(0, 100))
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this code removed intentionally?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your comment!

We removed this part of the code based on the suggestion from Mockito, which indicated that this code was not called in the test and recommended its removal.

The error messages are as follows:

org.mockito.exceptions.misusing.UnnecessaryStubbingException: 
Unnecessary stubbings detected.
Clean & maintainable test code requires zero unnecessary code.
Following stubbings are unnecessary (click to navigate to relevant line of code):
  1. -> at org.apache.hadoop.mapred.TestTaskAttemptListenerImpl.testGetMapCompletionEvents(TestTaskAttemptListenerImpl.java:264)
  2. -> at org.apache.hadoop.mapred.TestTaskAttemptListenerImpl.testGetMapCompletionEvents(TestTaskAttemptListenerImpl.java:266)
  3. -> at org.apache.hadoop.mapred.TestTaskAttemptListenerImpl.testGetMapCompletionEvents(TestTaskAttemptListenerImpl.java:268)
Please remove unnecessary stubbings or use 'lenient' strictness. More info: javadoc for UnnecessaryStubbingException class.

.thenReturn(taskEvents);
when(mockJob.getTaskAttemptCompletionEvents(0, 2))
.thenReturn(Arrays.copyOfRange(taskEvents, 0, 2));
when(mockJob.getTaskAttemptCompletionEvents(2, 100))
.thenReturn(Arrays.copyOfRange(taskEvents, 2, 4));
when(mockJob.getMapAttemptCompletionEvents(0, 100)).thenReturn(
TypeConverter.fromYarn(mapEvents));
when(mockJob.getMapAttemptCompletionEvents(0, 2)).thenReturn(
Expand Down Expand Up @@ -312,7 +309,8 @@ private static TaskAttemptCompletionEvent createTce(int eventId,
return tce;
}

@Test (timeout=10000)
@Test
@Timeout(value = 10)
public void testCommitWindow() throws IOException {
SystemClock clock = SystemClock.getInstance();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,15 @@

import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.ClusterStorageCapacityExceededException;
import org.junit.Before;
import org.junit.Test;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import static org.mockito.Mockito.*;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.anyString;
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

/**
* Tests the behavior of YarnChild.
Expand All @@ -36,7 +41,7 @@ public class TestYarnChild {
final static private String KILL_LIMIT_EXCEED_CONF_NAME =
"mapreduce.job.dfs.storage.capacity.kill-limit-exceed";

@Before
@BeforeEach
public void setUp() throws Exception {
task = mock(Task.class);
umbilical = mock(TaskUmbilicalProtocol.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
package org.apache.hadoop.mapreduce.jobhistory;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
Expand All @@ -40,7 +40,8 @@
import org.apache.hadoop.mapreduce.v2.app.job.impl.JobImpl;
import org.apache.hadoop.yarn.api.records.timelineservice.TimelineEvent;
import org.apache.hadoop.yarn.api.records.timelineservice.TimelineMetric;
import org.junit.Test;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Timeout;

public class TestEvents {

Expand All @@ -50,7 +51,8 @@ public class TestEvents {
*
* @throws Exception
*/
@Test(timeout = 10000)
@Test
@Timeout(value = 10)
public void testTaskAttemptFinishedEvent() throws Exception {

JobID jid = new JobID("001", 1);
Expand Down Expand Up @@ -79,7 +81,8 @@ public void testTaskAttemptFinishedEvent() throws Exception {
* @throws Exception
*/

@Test(timeout = 10000)
@Test
@Timeout(value = 10)
public void testJobPriorityChange() throws Exception {
org.apache.hadoop.mapreduce.JobID jid = new JobID("001", 1);
JobPriorityChangeEvent test = new JobPriorityChangeEvent(jid,
Expand All @@ -89,7 +92,8 @@ public void testJobPriorityChange() throws Exception {

}

@Test(timeout = 10000)
@Test
@Timeout(value = 10)
public void testJobQueueChange() throws Exception {
org.apache.hadoop.mapreduce.JobID jid = new JobID("001", 1);
JobQueueChangeEvent test = new JobQueueChangeEvent(jid,
Expand All @@ -103,7 +107,8 @@ public void testJobQueueChange() throws Exception {
*
* @throws Exception
*/
@Test(timeout = 10000)
@Test
@Timeout(value = 10)
public void testTaskUpdated() throws Exception {
JobID jid = new JobID("001", 1);
TaskID tid = new TaskID(jid, TaskType.REDUCE, 2);
Expand All @@ -118,7 +123,8 @@ public void testTaskUpdated() throws Exception {
* instance of HistoryEvent Different HistoryEvent should have a different
* datum.
*/
@Test(timeout = 10000)
@Test
@Timeout(value = 10)
public void testEvents() throws Exception {

EventReader reader = new EventReader(new DataInputStream(
Expand Down
Loading