[GitHub] [accumulo] EdColeman commented on a change in pull request #1667: Fix #1663 - Missing metrics files running CI tests

[GitBox <gi...@apache.org>]

EdColeman commented on a change in pull request #1667:
URL: https://github.com/apache/accumulo/pull/1667#discussion_r464580459



##########
File path: test/pom.xml
##########
@@ -270,6 +270,15 @@
           </execution>
         </executions>
       </plugin>
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-jar-plugin</artifactId>
+        <configuration>
+          <excludes>
+            <exclude>**/hadoop-metrics2-accumulo.properties</exclude>

Review comment:
       I was under the impression that the maven-jar-plugin would be called during assembly (package?)  If you build / test under maven, the property file will still be there.  So, any testing that uses classes / files under ..../target/...  should still work.
   
   You are correct though, if you tried to run it tests using just the jars - unless you provided a metrics configuration file with metrics output configured, then the tests that check those files would fail. If you go to the trouble to run the tests outside of maven using a command line, then if you got that far, adding a suitable config file may not be much of a hurdle and forces you to provide a configuration that you want for your needs.
   
   One issue is that the metrics config file is specifying ./target/name as a know location for the output files - this was so that mvn clean would remove them and they would not be left hanging around. 
   
   I suppose that if there was another well know location that would always be suitable for writes, then that could be used instead.  Sounds like just specified /tmp, but that's making an assumption that /tmp would always be suitable - for example would docker / containers / aws / ... prefer not to use /tmp but something else?  And then there's the clean-up - would need something besides (or modification) mvn clean.  Sure, /tmp may eventually get cleaned, but seems good practice not to depend on that.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org