gemmellr commented on a change in pull request #3929:
URL: https://github.com/apache/activemq-artemis/pull/3929#discussion_r798477756
##########
File path: tests/integration-tests/pom.xml
##########
@@ -557,7 +557,10 @@
<exclude>**/ReplicatedJMSFailoverTest.java</exclude>
<exclude>org.apache.activemq/tests/util/*.java</exclude>
</excludes>
- <argLine>-Djgroups.bind_addr=::1 ${activemq-surefire-argline}
${its-surefire-extra-args}
-Dorg.apache.activemq.SERIALIZABLE_PACKAGES="java.lang,javax.security,java.util,org.apache.activemq,org.fusesource.hawtbuf"</argLine>
+ <argLine>-Djgroups.bind_addr=::1 ${activemq-surefire-argline}
${its-surefire-extra-args}
+
-Dorg.apache.activemq.SERIALIZABLE_PACKAGES="java.lang,javax.security,java.util,org.apache.activemq,org.fusesource.hawtbuf"
+
-Dbuild.lib="${activemq.basedir}/artemis-distribution/target/apache-artemis-${project.version}-bin/apache-artemis-${project.version}/lib"
Review comment:
I dont think the integration-tests module has a ordered dependency on
the artemis-distribution module to ensure it is always built before the tests
are run. Even if it did, it wouldnt necessarily mean it was packaged, so this
property value is questionable.
##########
File path:
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/ManifestTest.java
##########
@@ -31,36 +24,44 @@
import org.junit.Assert;
import org.junit.Test;
+import javax.jms.ConnectionMetaData;
+import java.io.File;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Properties;
+import java.util.jar.Attributes;
+import java.util.jar.JarFile;
+import java.util.jar.Manifest;
+
public class ManifestTest extends ActiveMQTestBase {
private static final Logger log = Logger.getLogger(ManifestTest.class);
-
-
+ private static List<String> jarFiles = new ArrayList<>(Arrays.asList(
+ "artemis-jms-client-", "artemis-jms-server-"));
@Test
public void testManifestEntries() throws Exception {
Properties props = System.getProperties();
String userDir = props.getProperty("build.lib");
-
log.trace("userDir is " + userDir);
- // The jar must be there
- File file = new File("build/jars", "activemq-core.jar");
- Assert.assertTrue(file.exists());
-
- // Open the jar and load MANIFEST.MF
- JarFile jar = new JarFile(file);
- Manifest manifest = jar.getManifest();
-
ActiveMQServer server =
ActiveMQServers.newActiveMQServer(createBasicConfig());
-
ConnectionMetaData meta = new
ActiveMQConnectionMetaData(server.getVersion());
- // Compare the value from ConnectionMetaData and MANIFEST.MF
- Attributes attrs = manifest.getMainAttributes();
+ for (String jarFile : jarFiles) {
+ // The jar must be there
+ File file = new File(userDir, jarFile + meta.getProviderVersion() +
".jar");
+ Assert.assertTrue(file.exists());
Review comment:
The jar may not have been packaged before the integration tests are run
(the tests are always run against the classes when built in the same reactor,
not jars). That, coupled with the reactor ordering issue from my earlier
comment, make this assertion questionable. It would really need to be an Assume
check instead I think.
The smoke tests module does depend on the actual distribution module, and
already fails during execution if it doesnt find it built locally, so I think
it may be a better place for this test which requires a packaged jar. You then
wounldt even need the 'build.lib' property to be defined and the test could use
a specific path (as e.g other smoke tests do).
--
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]
