Re: [PR] HBASE-29968 Upgrade hbase-mapreduce to use junit5 [hbase]
liuxiaocs7 merged PR #8022: URL: https://github.com/apache/hbase/pull/8022 -- 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]
Re: [PR] HBASE-29968 Upgrade hbase-mapreduce to use junit5 [hbase]
liuxiaocs7 merged PR #8021: URL: https://github.com/apache/hbase/pull/8021 -- 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]
Re: [PR] HBASE-29968 Upgrade hbase-mapreduce to use junit5 [hbase]
liuxiaocs7 merged PR #7996: URL: https://github.com/apache/hbase/pull/7996 -- 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]
Re: [PR] HBASE-29968 Upgrade hbase-mapreduce to use junit5 [hbase]
Copilot commented on code in PR #8022:
URL: https://github.com/apache/hbase/pull/8022#discussion_r3031828690
##
hbase-mapreduce/pom.xml:
##
@@ -215,11 +215,6 @@
junit-jupiter-params
test
-
- org.junit.vintage
- junit-vintage-engine
- test
-
Review Comment:
With the removal of the vintage engine, this module still pulls in
`hbase-server` test-jar classes (e.g., `TestReplicationBase`) that reference
JUnit4 types (`org.junit.*` annotations/asserts). There is no explicit JUnit4
API dependency here, so tests can fail at runtime with missing `org.junit.*`
classes. Add an explicit `junit:junit` (JUnit4 API) test-scope dependency
(without vintage) or remove JUnit4 references from the shared test base classes
used by this module.
```suggestion
junit
junit
test
```
##
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestWALInputFormat.java:
##
@@ -17,27 +17,31 @@
*/
package org.apache.hadoop.hbase.mapreduce;
-import static org.junit.Assert.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
import java.util.ArrayList;
import java.util.List;
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.LocatedFileStatus;
import org.apache.hadoop.fs.Path;
-import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HBaseTestingUtility;
import org.apache.hadoop.hbase.testclassification.MapReduceTests;
-import org.apache.hadoop.hbase.testclassification.SmallTests;
import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
-import org.junit.ClassRule;
-import org.junit.Test;
-import org.junit.experimental.categories.Category;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
-@Category({ MapReduceTests.class, SmallTests.class })
+@Tag(MapReduceTests.TAG)
+@Tag(MediumTests.TAG)
public class TestWALInputFormat {
- @ClassRule
- public static final HBaseClassTestRule CLASS_RULE =
-HBaseClassTestRule.forClass(TestWALInputFormat.class);
+ private static final HBaseTestingUtility TEST_UTIL = new
HBaseTestingUtility();
+
+ @BeforeAll
+ public static void setupClass() throws Exception {
+TEST_UTIL.startMiniCluster();
+TEST_UTIL.createWALRootDir();
+ }
Review Comment:
`@Tag(MediumTests.TAG)` is used but `MediumTests` is neither imported nor in
this class’s previous classification (it imported `SmallTests`). This will not
compile. Either change the tag to `SmallTests.TAG` or add the correct
`MediumTests` import and update the intent consistently.
--
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]
[PR] HBASE-29968 Upgrade hbase-mapreduce to use junit5 [hbase]
liuxiaocs7 opened a new pull request, #8022: URL: https://github.com/apache/hbase/pull/8022 (cherry picked from commit 0dc9652b554a8f3542480225d7ac2162bd4c4a0a) - for branch-2.5 - see: HBASE-29968 -- 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]
Re: [PR] HBASE-29968 Upgrade hbase-mapreduce to use junit5 [hbase]
Copilot commented on code in PR #8021: URL: https://github.com/apache/hbase/pull/8021#discussion_r3031808828 ## hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestTableInputFormatBase.java: ## @@ -17,9 +17,9 @@ */ package org.apache.hadoop.hbase.mapreduce; -import static org.junit.Assert.*; -import static org.mockito.Mockito.any; -import static org.mockito.Mockito.anyBoolean; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; Review Comment: `assertEquals` is now statically imported from JUnit Jupiter, but the test still contains at least one JUnit4-style 3-arg invocation (`assertEquals(message, expected, actual)`). JUnit Jupiter expects `assertEquals(expected, actual, message)`, so this will not compile. Please update the argument order (e.g., the call in `testTableInputFormatBaseReverseDNSForIPv6`). -- 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]
[PR] HBASE-29968 Upgrade hbase-mapreduce to use junit5 [hbase]
liuxiaocs7 opened a new pull request, #8021: URL: https://github.com/apache/hbase/pull/8021 (cherry picked from commit dfc8f30d9ea33615cc84c835ee1343e2e47e8c6e) - for branch-2.6 - see: HBASE-29968 -- 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]
Re: [PR] HBASE-29968 Upgrade hbase-mapreduce to use junit5 [hbase]
liuxiaocs7 commented on PR #7996: URL: https://github.com/apache/hbase/pull/7996#issuecomment-4182280744 After verification, the unit tests before and after the modification are completely consistent, so let me create the backport for other branches. -- 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]
Re: [PR] HBASE-29968 Upgrade hbase-mapreduce to use junit5 [hbase]
Copilot commented on code in PR #7996:
URL: https://github.com/apache/hbase/pull/7996#discussion_r3026416009
##
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestImportTSVWithVisibilityLabels.java:
##
@@ -390,22 +389,22 @@ private static void validateHFiles(FileSystem fs, String
outputPath, String fami
String[] elements = cfStatus.getPath().toString().split(Path.SEPARATOR);
String cf = elements[elements.length - 1];
foundFamilies.add(cf);
- assertTrue(String.format(
-"HFile ouput contains a column family (%s) not present in input
families (%s)", cf,
-configFamilies), configFamilies.contains(cf));
+ assertTrue(configFamilies.contains(cf),
+String.format(
+ "HFile ouput contains a column family (%s) not present in input
families (%s)", cf,
+ configFamilies));
Review Comment:
Typo in the assertion message: "HFile ouput contains..." should be "HFile
output contains...".
##
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestImportTsv.java:
##
@@ -536,25 +522,25 @@ private static void validateHFiles(FileSystem fs, String
outputPath, String fami
String[] elements = cfStatus.getPath().toString().split(Path.SEPARATOR);
String cf = elements[elements.length - 1];
foundFamilies.add(cf);
- assertTrue(String.format(
-"HFile output contains a column family (%s) not present in input
families (%s)", cf,
-configFamilies), configFamilies.contains(cf));
+ assertTrue(configFamilies.contains(cf),
+String.format(
+ "HFile output contains a column family (%s) not present in input
families (%s)", cf,
+ configFamilies));
for (FileStatus hfile : fs.listStatus(cfStatus.getPath())) {
-assertTrue(String.format("HFile %s appears to contain no data.",
hfile.getPath()),
- hfile.getLen() > 0);
+assertTrue(hfile.getLen() > 0,
+ String.format("HFile %s appears to contain no data.",
hfile.getPath()));
// count the number of KVs from all the hfiles
if (expectedKVCount > -1) {
actualKVCount += getKVCountFromHfile(fs, hfile.getPath());
}
}
}
-assertTrue(String.format("HFile output does not contain the input family
'%s'.", family),
- foundFamilies.contains(family));
+assertTrue(foundFamilies.contains(family),
+ String.format("HFile output does not contain the input family '%s'.",
family));
if (expectedKVCount > -1) {
- assertTrue(
+ assertTrue(actualKVCount == expectedKVCount,
String.format("KV count in ouput hfile=<%d> doesn't match with
expected KV count=<%d>",
- actualKVCount, expectedKVCount),
-actualKVCount == expectedKVCount);
+ actualKVCount, expectedKVCount));
Review Comment:
Typo in the assertion message: "KV count in ouput hfile" should be "KV count
in output hfile".
--
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]
Re: [PR] HBASE-29968 Upgrade hbase-mapreduce to use junit5 [hbase]
Copilot commented on code in PR #7996:
URL: https://github.com/apache/hbase/pull/7996#discussion_r3026150155
##
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestImportTsv.java:
##
@@ -453,8 +439,8 @@ protected static Tool doMROnTableTest(HBaseTestingUtility
util, TableName table,
&& "true".equalsIgnoreCase(args.get(ImportTsv.DRY_RUN_CONF_KEY));
if (args.containsKey(ImportTsv.BULK_OUTPUT_CONF_KEY)) {
if (isDryRun) {
-assertFalse(String.format("Dry run mode, %s should not have been
created.",
- ImportTsv.BULK_OUTPUT_CONF_KEY), fs.exists(new
Path(ImportTsv.BULK_OUTPUT_CONF_KEY)));
+assertFalse(fs.exists(new Path(ImportTsv.BULK_OUTPUT_CONF_KEY)),
String.format(
+ "Dry run mode, %s should not have been created.",
ImportTsv.BULK_OUTPUT_CONF_KEY));
} else {
Review Comment:
In the dry-run + bulk-output branch, the existence check uses new
Path(ImportTsv.BULK_OUTPUT_CONF_KEY), which is the configuration key name (e.g.
"importtsv.bulk.output"), not the bulk output path value. This makes the
assertion ineffective (it will not actually verify that the bulk output
directory was not created). Use the configured output path
(args.get(ImportTsv.BULK_OUTPUT_CONF_KEY)) for the Path being checked.
##
hbase-mapreduce/pom.xml:
##
@@ -215,11 +215,6 @@
junit-jupiter-params
test
-
- org.junit.vintage
- junit-vintage-engine
- test
-
Review Comment:
The junit-vintage-engine dependency was removed, but there are still
JUnit4-based tests under hbase-mapreduce/src/test/java (e.g.,
TestPerformanceEvaluation, TestCellBasedHFileOutputFormat2,
TestCellBasedImportExport2, TestCellBasedWALPlayer2). Without the vintage
engine these tests will no longer be discovered/executed by Surefire, reducing
test coverage for this module. Either migrate the remaining JUnit4 tests to
JUnit5 in this PR, or keep junit-vintage-engine until the migration is complete.
```suggestion
org.junit.vintage
junit-vintage-engine
test
```
##
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat2.java:
##
@@ -691,10 +684,10 @@ private void doIncrementalLoadTest(boolean
shouldChangeRegions, boolean shouldKe
}
}
}
- assertEquals("Column family not found in FS.", FAMILIES.length, dir);
+ assertEquals(FAMILIES.length, dir, "Column family not found in FS.");
}
if (writeMultipleTables) {
- assertEquals("Dir for all input tables not created", numTableDirs,
allTables.size());
+ assertEquals(numTableDirs, allTables.size(), "Dir for all input tables
not created");
Review Comment:
assertEquals parameters are reversed here: expected should be
allTables.size() (one dir per table) and actual should be numTableDirs. As
written, failure output will be misleading.
```suggestion
assertEquals(allTables.size(), numTableDirs, "Dir for all input tables
not created");
```
##
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat2.java:
##
@@ -650,9 +643,9 @@ private void doIncrementalLoadTest(boolean
shouldChangeRegions, boolean shouldKe
Table table = util.createTable(tableName, FAMILIES, splitKeys);
RegionLocator r = util.getConnection().getRegionLocator(tableName);
- assertEquals("Should start with empty table", 0, util.countRows(table));
+ assertEquals(0, util.countRows(table), "Should start with empty table");
int numRegions = r.getStartKeys().length;
- assertEquals("Should make " + regionNum + " regions", numRegions,
regionNum);
+ assertEquals(numRegions, regionNum, "Should make " + regionNum + "
regions");
Review Comment:
assertEquals parameters are reversed here: the expected number of regions is
regionNum, while numRegions is the actual value read from the locator. Swapping
them will produce correct failure diagnostics if this assertion fails.
```suggestion
assertEquals(regionNum, numRegions, "Should make " + regionNum + "
regions");
```
--
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]
Re: [PR] HBASE-29968 Upgrade hbase-mapreduce to use junit5 [hbase]
liuxiaocs7 closed pull request #7996: HBASE-29968 Upgrade hbase-mapreduce to use junit5 URL: https://github.com/apache/hbase/pull/7996 -- 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]
Re: [PR] HBASE-29968 Upgrade hbase-mapreduce to use junit5 [hbase]
liuxiaocs7 commented on PR #7996: URL: https://github.com/apache/hbase/pull/7996#issuecomment-4151976487 Spotless check fails due to HBASE-29863, https://github.com/apache/hbase/pull/7823 Let me file an addendum first -- 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]
Re: [PR] HBASE-29968 Upgrade hbase-mapreduce to use junit5 [hbase]
liuxiaocs7 merged PR #7992: URL: https://github.com/apache/hbase/pull/7992 -- 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]
Re: [PR] HBASE-29968 Upgrade hbase-mapreduce to use junit5 [hbase]
liuxiaocs7 merged PR #7861: URL: https://github.com/apache/hbase/pull/7861 -- 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]
[PR] HBASE-29968 Upgrade hbase-mapreduce to use junit5 [hbase]
liuxiaocs7 opened a new pull request, #7996: URL: https://github.com/apache/hbase/pull/7996 - for branch-2 - see: HBASE-29968 -- 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]
Re: [PR] HBASE-29968 Upgrade hbase-mapreduce to use junit5 [hbase]
Copilot commented on code in PR #7992:
URL: https://github.com/apache/hbase/pull/7992#discussion_r2999635329
##
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/MRIncrementalLoadTestBase.java:
##
@@ -110,17 +102,16 @@ public void setUp() throws IOException {
Table table = UTIL.createTable(tableName, FAMILIES, splitKeys);
RegionLocator r = UTIL.getConnection().getRegionLocator(tableName);
- assertEquals("Should start with empty table", 0,
HBaseTestingUtil.countRows(table));
+ assertEquals(0, HBaseTestingUtil.countRows(table), "Should start with
empty table");
int numRegions = r.getStartKeys().length;
- assertEquals("Should make " + regionNum + " regions", numRegions,
regionNum);
+ assertEquals(numRegions, regionNum, "Should make " + regionNum + "
regions");
Review Comment:
In JUnit 5, `assertEquals(expected, actual, message)` reports expected vs
actual values on failure. Here `numRegions` (actual) and `regionNum` (expected)
are reversed, which will produce confusing diagnostics if the assertion fails.
Swap the arguments so `regionNum` is the expected value and `numRegions` is the
actual value.
```suggestion
assertEquals(regionNum, numRegions, "Should make " + regionNum + "
regions");
```
--
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]
[PR] HBASE-29968 Upgrade hbase-mapreduce to use junit5 [hbase]
liuxiaocs7 opened a new pull request, #7992: URL: https://github.com/apache/hbase/pull/7992 - for branch-3 - see HBASE-29968 -- 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]
Re: [PR] HBASE-29968 Upgrade hbase-mapreduce to use junit5 [hbase]
liuxiaocs7 commented on PR #7861: URL: https://github.com/apache/hbase/pull/7861#issuecomment-4140066351 Hi, @Apache9, could you help review this PR first when free, thanks! -- 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]
Re: [PR] HBASE-29968 Upgrade hbase-mapreduce to use junit5 [hbase]
Copilot commented on code in PR #7861:
URL: https://github.com/apache/hbase/pull/7861#discussion_r2981829035
##
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/MRIncrementalLoadTestBase.java:
##
@@ -169,10 +172,10 @@ public void doIncrementalLoadTest() throws Exception {
}
}
}
- assertEquals("Column family not found in FS.", FAMILIES.length, dir);
+ assertEquals(FAMILIES.length, dir, "Column family not found in FS.");
}
if (writeMultipleTables) {
- assertEquals("Dir for all input tables not created", numTableDirs,
allTables.size());
+ assertEquals(numTableDirs, allTables.size(), "Dir for all input tables
not created");
Review Comment:
This `assertEquals` also has expected/actual reversed (`numTableDirs` is the
measured value, `allTables.size()` is the expected count). Swapping the
parameters will make assertion failure output correctly describe what was
expected vs what was observed.
```suggestion
assertEquals(allTables.size(), numTableDirs, "Dir for all input tables
not created");
```
##
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/MRIncrementalLoadTestBase.java:
##
@@ -110,17 +102,16 @@ public void setUp() throws IOException {
Table table = UTIL.createTable(tableName, FAMILIES, splitKeys);
RegionLocator r = UTIL.getConnection().getRegionLocator(tableName);
- assertEquals("Should start with empty table", 0,
HBaseTestingUtil.countRows(table));
+ assertEquals(0, HBaseTestingUtil.countRows(table), "Should start with
empty table");
int numRegions = r.getStartKeys().length;
- assertEquals("Should make " + regionNum + " regions", numRegions,
regionNum);
+ assertEquals(numRegions, regionNum, "Should make " + regionNum + "
regions");
Review Comment:
In JUnit assertions, expected/actual order appears reversed, which makes
failures harder to interpret. For example, this compares `numRegions` (actual)
against `regionNum` (expected) but passes them as expected/actual in the
opposite order. Please swap the arguments so the expected value is `regionNum`
and the actual value is `numRegions` (and similarly for the later
`numTableDirs` vs `allTables.size()` assertion).
```suggestion
assertEquals(regionNum, numRegions, "Should make " + regionNum + "
regions");
```
--
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]
Re: [PR] HBASE-29968 Upgrade hbase-mapreduce to use junit5 [hbase]
liuxiaocs7 commented on PR #7861: URL: https://github.com/apache/hbase/pull/7861#issuecomment-4118517546 master branch has the same result with applying the patch (56+366) ``` default-test [INFO] Results: [INFO] [INFO] Tests run: 56, Failures: 0, Errors: 0, Skipped: 0 [INFO] [INFO] [INFO] --- surefire:3.5.3:test (secondPartTestsExecution) @ hbase-mapreduce --- [INFO] Using auto detected provider org.apache.maven.surefire.junitplatform.JUnitPlatformProvider [INFO] [INFO] Results: [INFO] [WARNING] Tests run: 366, Failures: 0, Errors: 0, Skipped: 12 [INFO] [INFO] [INFO] BUILD SUCCESS [INFO] ``` -- 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]
Re: [PR] HBASE-29968 Upgrade hbase-mapreduce to use junit5 [hbase]
liuxiaocs7 commented on code in PR #7861:
URL: https://github.com/apache/hbase/pull/7861#discussion_r2979957662
##
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/replication/TestVerifyReplicationAdjunct.java:
##
@@ -89,15 +79,14 @@ protected String getClusterKey(HBaseTestingUtil util)
throws Exception {
return util.getClusterKey();
}
- @Before
+ @BeforeEach
public void setUp() throws Exception {
cleanUp();
UTIL2.deleteTableData(peerTableName);
}
- @BeforeClass
+ @BeforeAll
public static void setUpBeforeClass() throws Exception {
-TestReplicationBase.setUpBeforeClass();
Review Comment:
Will be invoked in parent class `TestReplicationBase` automatically, will
refactor this in `hbase-server`
--
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]
Re: [PR] HBASE-29968 Upgrade hbase-mapreduce to use junit5 [hbase]
Copilot commented on code in PR #7861:
URL: https://github.com/apache/hbase/pull/7861#discussion_r2981414057
##
hbase-mapreduce/pom.xml:
##
@@ -191,11 +191,6 @@
junit-jupiter-params
test
-
- org.junit.vintage
- junit-vintage-engine
- test
-
Review Comment:
The `junit-vintage-engine` dependency was removed, but there are still JUnit
4-based tests in this module (e.g., `TestJarFinder` still uses
`org.junit.Test`). Without the Vintage engine those tests will be skipped
entirely.
Suggested fix: either migrate the remaining JUnit 4 tests to JUnit Jupiter
before removing Vintage, or keep Vintage until all JUnit 4 tests are converted.
```suggestion
org.junit.vintage
junit-vintage-engine
test
```
##
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/replication/TestVerifyReplicationAdjunct.java:
##
@@ -352,9 +341,8 @@ public void testVerifyReplicationWithSnapshotSupport()
throws Exception {
TestVerifyReplicationZkClusterKey.checkRestoreTmpDir(CONF2, temPath2, 2);
}
- @AfterClass
+ @AfterAll
public static void tearDownAfterClass() throws Exception {
htable3.close();
-TestReplicationBase.tearDownAfterClass();
}
Review Comment:
`tearDownAfterClass` no longer delegates to
`TestReplicationBase.tearDownAfterClass()`. Since this class also hides the
superclass method, the mini clusters/connections started by
`TestReplicationBase` may never be shut down, leading to leaked resources and
follow-on test failures.
Suggested fix: add `TestReplicationBase.tearDownAfterClass()` after closing
`htable3` (or rename this method to avoid hiding and rely on inherited
`@AfterAll`).
##
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestJarFinder.java:
##
@@ -30,24 +30,18 @@
import java.util.jar.JarInputStream;
import java.util.jar.JarOutputStream;
import java.util.jar.Manifest;
-import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.testclassification.SmallTests;
import org.junit.Assert;
-import org.junit.ClassRule;
import org.junit.Test;
-import org.junit.experimental.categories.Category;
+import org.junit.jupiter.api.Tag;
import org.slf4j.LoggerFactory;
/**
* This file was forked from hadoop/common/branches/branch-2@1350012.
*/
-@Category(SmallTests.class)
+@Tag(SmallTests.TAG)
public class TestJarFinder {
- @ClassRule
- public static final HBaseClassTestRule CLASS_RULE =
-HBaseClassTestRule.forClass(TestJarFinder.class);
-
@Test
public void testJar() throws Exception {
Review Comment:
This test still imports and uses JUnit 4 (`org.junit.Test` /
`org.junit.Assert`). With the removal of the `junit-vintage-engine` in this
module, it will no longer be executed on the JUnit Platform.
Suggested fix: migrate the remaining JUnit 4 annotations/assertions to JUnit
Jupiter (e.g., `org.junit.jupiter.api.Test` and
`org.junit.jupiter.api.Assertions`).
--
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]
Re: [PR] HBASE-29968 Upgrade hbase-mapreduce to use junit5 [hbase]
liuxiaocs7 commented on code in PR #7861:
URL: https://github.com/apache/hbase/pull/7861#discussion_r2979957662
##
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/replication/TestVerifyReplicationAdjunct.java:
##
@@ -89,15 +79,14 @@ protected String getClusterKey(HBaseTestingUtil util)
throws Exception {
return util.getClusterKey();
}
- @Before
+ @BeforeEach
public void setUp() throws Exception {
cleanUp();
UTIL2.deleteTableData(peerTableName);
}
- @BeforeClass
+ @BeforeAll
public static void setUpBeforeClass() throws Exception {
-TestReplicationBase.setUpBeforeClass();
Review Comment:
Will be invoked in parent class `TestReplicationBase` automatically
--
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]
Re: [PR] HBASE-29968 Upgrade hbase-mapreduce to use junit5 [hbase]
liuxiaocs7 commented on code in PR #7861:
URL: https://github.com/apache/hbase/pull/7861#discussion_r2979947685
##
hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationBase.java:
##
@@ -272,6 +276,7 @@ private static void startClusters() throws Exception {
htable2 = connection2.getTable(tableName);
}
+ @BeforeAll
Review Comment:
add this for `TestVerifyReplicationAdjunct`, will remove in `hbase-server`
upgrading!
--
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]
Re: [PR] HBASE-29968 Upgrade hbase-mapreduce to use junit5 [hbase]
liuxiaocs7 commented on PR #7861: URL: https://github.com/apache/hbase/pull/7861#issuecomment-4090633359 `TestVerifyReplicationAdjunct` fails -- 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]
Re: [PR] HBASE-29968 Upgrade hbase-mapreduce to use junit5 [hbase]
liuxiaocs7 commented on PR #7861: URL: https://github.com/apache/hbase/pull/7861#issuecomment-4067183489 wait: https://github.com/apache/hbase/pull/7945 -- 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]
Re: [PR] HBASE-29968 Upgrade hbase-mapreduce to use junit5 [hbase]
liuxiaocs7 commented on PR #7861: URL: https://github.com/apache/hbase/pull/7861#issuecomment-4022562648 wait https://github.com/apache/hbase/pull/7887 -- 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]
[PR] HBASE-29968 Upgrade hbase-mapreduce to use junit5 [hbase]
liuxiaocs7 opened a new pull request, #7861: URL: https://github.com/apache/hbase/pull/7861 - see: HBASE-29968 -- 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]
