Copilot commented on code in PR #7895:
URL: https://github.com/apache/hadoop/pull/7895#discussion_r2300174122
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotFileLength.java:
##########
@@ -26,15 +26,14 @@
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.hdfs.AppendTestUtil;
import org.apache.hadoop.hdfs.DFSConfigKeys;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
-import static org.hamcrest.CoreMatchers.is;
-import static org.hamcrest.CoreMatchers.not;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
+import static org.assertj.core.api.Assertions.assertThat;
Review Comment:
The import should use JUnit 5 assertions instead of AssertJ. Replace `import
static org.assertj.core.api.Assertions.assertThat;` with `import static
org.junit.jupiter.api.Assertions.assertThat;` to maintain consistency with the
rest of the migration.
```suggestion
import static org.junit.jupiter.api.Assertions.assertThat;
```
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestRenameWithOrderedSnapshotDeletion.java:
##########
@@ -101,10 +102,10 @@ public void testRename() throws Exception {
private void validateRename(Path src, Path dest) {
try {
hdfs.rename(src, dest);
- Assert.fail("Expected exception not thrown.");
+ Assertions.fail("Expected exception not thrown.");
} catch (IOException ioe) {
- Assert.assertTrue(ioe.getMessage().contains("are not under the" +
- " same snapshot root."));
+ Assertions
+ .assertTrue(ioe.getMessage().contains("are not under the" +
" same snapshot root."));
Review Comment:
[nitpick] The string concatenation is split awkwardly across the line break.
Consider keeping the complete string on one line or properly formatting the
concatenation for better readability.
```suggestion
Assertions.assertTrue(ioe.getMessage().contains("are not under the
same snapshot root."));
```
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshot.java:
##########
@@ -473,7 +472,8 @@ public void testSnapshotMtime() throws Exception {
newSnapshotStatus.getModificationTime());
}
- @Test(timeout = 60000)
+@Test
+@Timeout(value = 60)
Review Comment:
[nitpick] Inconsistent formatting for the @Test and @Timeout annotations.
Most other test methods in the migration have these on the same line or with
consistent spacing.
```suggestion
@Test @Timeout(value = 60)
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
