deepakpanda93 commented on code in PR #19494:
URL: https://github.com/apache/hudi/pull/19494#discussion_r3712535714
##########
hudi-hadoop-common/src/test/java/org/apache/hudi/common/model/TestHoodiePartitionMetadata.java:
##########
@@ -89,6 +107,116 @@ public void
testTextFormatMetaFile(Option<HoodieFileFormat> format) throws IOExc
assertEquals(3, readMetadata.getPartitionDepth());
Review Comment:
Agreed, and done — `diff` confirmed the two copies were byte identical,
which made it worth doing properly rather than leaving for the next log
assertion test to copy a third time.
It now lives at
`hudi-common/src/test/java/org/apache/hudi/common/testutils/HoodieTestLogAppender.java`.
That module works for both call sites because `hudi-hadoop-common` already
declares `hudi-common` with `<type>test-jar</type>`, and
`org.apache.hudi.common.testutils` is where the existing shared harnesses sit.
Two things worth noting on the way it landed:
- The name matters. Surefire collects `Test*.java`, so a standalone
`TestLogAppender.java` would have been picked up as a test class.
`HoodieTestLogAppender` avoids that, and the class javadoc says why so nobody
renames it back.
- I also moved the attach and detach boilerplate into the utility, since
both call sites were repeating the same four lines of `LogManager.getLogger` /
`start` / `addAppender` and the matching teardown. Each site is now `new
HoodieTestLogAppender().attachTo(RetryHelper.class)` with `appender.detach()`
in the `finally`.
This does mean the PR touches two modules and adds a test utility class,
which is a little wider than the bug fix itself. Happy to split the extraction
into a follow up if a committer would rather keep this one minimal.
Verified after the change: `TestRetryHelper` 4/4,
`TestHoodiePartitionMetadata` 7/7, checkstyle and rat clean on both modules.
Also rebased onto master.
--
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]
