Copilot commented on code in PR #8210:
URL: https://github.com/apache/hbase/pull/8210#discussion_r3210985276
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHBaseWalOnEC.java:
##########
@@ -82,37 +74,39 @@ public static void setUpBeforeClass() throws Exception {
try (FSDataOutputStream out = fs.create(new Path("/canary"))) {
// If this comes back as having hflush then some test setup assumption
is wrong.
// Fail the test so that a developer has to look and triage
- assertFalse("Did not enable EC!",
out.hasCapability(StreamCapabilities.HFLUSH));
+ assertFalse(out.hasCapability(StreamCapabilities.HFLUSH), "Did not
enable EC!");
}
} catch (NoSuchMethodException e) {
// We're not testing anything interesting if EC is not available, so
skip the rest of the test
- Assume.assumeNoException("Using an older version of hadoop; EC not
available.", e);
+ Assumptions.assumeTrue(false, "Using an older version of hadoop; EC not
available.");
}
Review Comment:
If the Hadoop version lacks the EC APIs, this `@BeforeAll` has already
started a MiniDFSCluster before hitting the assumption. Aborting the test at
that point can leak the cluster (and threads/files) because `@AfterAll` will
not run when `@BeforeAll` aborts. Consider restructuring so you check for
required methods before starting the cluster, or ensure the MiniDFSCluster is
shut down in the catch/finally path before calling Assumptions.*.
--
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]
