xiangfu0 commented on code in PR #19885:
URL: https://github.com/apache/pinot/pull/19885#discussion_r4288269713
##########
pinot-plugins/assembly-descriptor/src/it/projects/simple-assembly/pom.xml:
##########
@@ -34,18 +34,35 @@
<build>
<pluginManagement>
<plugins>
+ <!-- Use the plugin versions of the outer Pinot build so that they
resolve from its local repository
+ (maven.repo.local.tail in the invoker configuration) instead of
being downloaded for every IT run. -->
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<version>@version.maven-compiler-plugin@</version>
</plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-jar-plugin</artifactId>
+ <version>@version.maven-jar-plugin@</version>
+ </plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-resources-plugin</artifactId>
+ <version>@version.maven-resources-plugin@</version>
+ </plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <version>@version.maven-surefire-plugin@</version>
+ </plugin>
</plugins>
</pluginManagement>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-assembly-plugin</artifactId>
- <version>3.7.1</version>
+ <version>@version.maven-assembly-plugin@</version>
Review Comment:
MINOR: This switches the assembly plugin from the pinned `3.7.1` to the
parent-managed `@version.maven-assembly-plugin@`, which resolves to `3.8.0` in
apache-40. That is consistent with the PR's goal of reusing the outer build's
versions, but it is an implicit version bump — worth a line in the PR
description so reviewers know the descriptor is now exercised against 3.8.0.
##########
pinot-plugins/assembly-descriptor/src/it/projects/simple-assembly/verify.groovy:
##########
@@ -24,7 +24,17 @@ def expected = [
"simple-assembly/commons-lang3-${commonslang3_version}.jar"
] as Set
-def entries = new
File(basedir,'target/simple-assembly-0.0.1-SNAPSHOT-plugin.zip').with {
+def zip = new File(basedir, 'target/simple-assembly-0.0.1-SNAPSHOT-plugin.zip')
+if (!zip.isFile()) {
+ // The invoker runs this script even when the IT build itself failed, and
the exception thrown here then replaces
+ // the invoker's own "build exited with code N" message in the console.
Include the end of the IT build log so that
+ // the console shows the real error instead of just a missing-file exception.
+ def buildLog = new File(basedir, 'build.log')
+ def logTail = buildLog.isFile() ?
buildLog.readLines().takeRight(100).join('\n') : '(no build.log found)'
Review Comment:
MINOR (nit): `readLines()` loads the entire `build.log` into memory before
`takeRight(100)`. That is perfectly fine for an IT log, so no change is needed
— just noting that only the last 100 lines are actually used.
--
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]
