JunRuiLee commented on PR #9856:
URL: https://github.com/apache/paimon/pull/9856#issuecomment-5684611047

   ### Code review
   
   Found 2 issues:
   
   1. The new module-level surefire `integration-tests` override silently stops 
17 pre-existing tests from running. Root `pom.xml` sets `test.unit.pattern` to 
`**/*Test.*`, so `default-test` only matches `*Test`-suffixed classes and the 
`integration-tests` execution (`<!--execute all the integration tests-->`, 
`<include>**/*.*</include>`) is the only one that ever matched the four 
`Test`-prefixed classes in this module: `TestJindoCacheEnable` (9), 
`TestJindoDlfAccessTracking` (4), `TestJindoMultiPartUploadCommitter` (2), 
`TestHadoopCompliantFileIODeleteHook` (2). Two independent causes: the child 
`<includes>` replaces the parent's rather than appending (no 
`combine.children="append"`, unlike the shade `<includes>` 16 lines below), and 
`<groups>plugin-isolation</groups>` excludes these untagged classes regardless. 
CI runs `goal=verify` for the `core` suite without excluding `paimon-jindo`, so 
this drops on every run.
   
   
https://github.com/apache/paimon/blob/ca5fe46c4df914aa85cd4de20eb28b95ee762712/paimon-filesystems/paimon-jindo/pom.xml#L257-L262
   
   2. `unpack-oss-runtime` strips only signatures and `META-INF/maven/**`, so 
third-party license metadata is copied into the plugin directory. The packaged 
jar carries 
`paimon-plugin-jindo-oss/META-INF/{DEPENDENCIES,LICENSE,LICENSE.txt,MANIFEST.MF,NOTICE,NOTICE.txt,jdom-info.xml}`,
 and because every runtime dependency unpacks into one directory these are 
last-writer-wins: the nested `NOTICE` contains only HttpCore's, `NOTICE.txt` 
only Commons Codec's, and `MANIFEST.MF` reports `Automatic-Module-Name: 
io.opentracing.api` — for a closure that also bundles aliyun-sdk-oss, gson, 
opentracing, jettison, ini4j and jdom2. This is what #2468 reported and #2523 
fixed for the sibling plugin directories (after #2429 cancelled 0.6 RC1 over 
the same class of defect); `paimon-oss` and `paimon-s3` exclude `META-INF/**` 
and re-admit only `META-INF/services/**,META-INF/versions/**`. Nothing here 
needs the nested entries — the plugin dir has no `META-INF/services` or 
`META-INF/versions`, and the r
 esources the new tests assert on (`versioninfo.properties`, 
`common.properties`, `oss.properties`) sit at the plugin-dir root. 
`JarFileChecker` cannot catch it: `getNumLicenseFilesOutsideMetaInfDirectory` 
filters on `!path.contains("META-INF")` and `getNumNestedMetaInfDirectories` 
only matches `^/?META-INF/versions/[^/]+/META-INF/?$`.
   
   
https://github.com/apache/paimon/blob/ca5fe46c4df914aa85cd4de20eb28b95ee762712/paimon-filesystems/paimon-jindo/pom.xml#L219-L221
   
   Sibling precedent: 
https://github.com/apache/paimon/blob/ca5fe46c4df914aa85cd4de20eb28b95ee762712/paimon-filesystems/paimon-oss/pom.xml#L96-L108
   
   🤖 Generated with [Claude Code](https://claude.ai/code)
   
   <sub>- If this code review was useful, please react with 👍. Otherwise, react 
with 👎.</sub>


-- 
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]

Reply via email to