kwin commented on code in PR #114:
URL:
https://github.com/apache/commons-secure-xml/pull/114#discussion_r4237549131
##########
pom.xml:
##########
@@ -391,6 +413,110 @@ limitations under the License.
</execution>
</executions>
</plugin>
+ <!--
+ Writes a fragment manifest into the test output directory,
+ so OsgiIT can install it as an exploded bundle attached to the library.
+ The imports are optional:
+ the test harness (JUnit, Mockito, JAXB) is boot-delegated to the class
path by OsgiIT.
+ -->
+ <plugin>
+ <groupId>org.apache.felix</groupId>
+ <artifactId>maven-bundle-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>test-bundle-manifest</id>
+ <phase>process-test-classes</phase>
+ <goals>
+ <goal>manifest</goal>
+ </goals>
+ <configuration>
+
<outputDirectory>${project.build.testOutputDirectory}</outputDirectory>
+
<manifestLocation>${project.build.testOutputDirectory}/META-INF</manifestLocation>
+ <instructions>
+
<Bundle-SymbolicName>${commons.osgi.symbolicName}.tests</Bundle-SymbolicName>
+ <Fragment-Host>${commons.osgi.symbolicName}</Fragment-Host>
+ <Export-Package>!*</Export-Package>
+ <Private-Package>org.apache.commons.xml.*</Private-Package>
+ <Import-Package>*;resolution:=optional</Import-Package>
+ <!-- The java11 test sources would otherwise raise the osgi.ee
requirement to that of the build JDK. -->
+ <_noee>true</_noee>
+ <!--
+ bnd looks the host up on the class path to drop its packages
from Import-Package, but the host is this module's own jar, not built yet.
+ Harmless: the tests are in the host's packages, so the
fragment imports none of them anyway.
+ -->
+ <_fixupmessages>"Host ${commons.osgi.symbolicName}= for this
fragment/require bundle cannot be found on the
classpath";is:=ignore</_fixupmessages>
+ </instructions>
+ </configuration>
+ </execution>
+ </executions>
+ </plugin>
+ <!--
+ OsgiIT runs the unit tests inside an OSGi framework against the
packaged bundle, once per framework.
+ Each framework jar embeds its own copy of the OSGi API,
+ so each execution keeps exactly one of them on the class path.
+ Saxon is dropped as in test-stockjdk,
+ so provider lookups reach the JDK's built-in implementations, as they
do for a bundle.
+ -->
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-failsafe-plugin</artifactId>
+ <!-- The same permissive JDK defaults as the surefire executions. -->
+ <configuration>
+ <systemPropertyVariables>
+ <javax.xml.accessExternalDTD>all</javax.xml.accessExternalDTD>
+
<javax.xml.accessExternalSchema>all</javax.xml.accessExternalSchema>
+
<javax.xml.accessExternalStylesheet>all</javax.xml.accessExternalStylesheet>
+ </systemPropertyVariables>
+ </configuration>
+ <executions>
+ <execution>
+ <id>test-osgi-felix</id>
+ <goals>
+ <goal>integration-test</goal>
+ <goal>verify</goal>
+ </goals>
+ <configuration>
+ <includes>
+ <include>**/OsgiIT.java</include>
+ </includes>
+
<reportsDirectory>${project.build.directory}/failsafe-reports/felix</reportsDirectory>
+
<summaryFile>${project.build.directory}/failsafe-reports/felix/failsafe-summary.xml</summaryFile>
+ <classpathDependencyExcludes>
Review Comment:
I know this pattern is used elsewhere as well, however instead of tweaking
classpath (which is hard to reproduce e.g. in Eclipse) I would appreciate to
just parameterize via classname passed via system property (here the
FrameworkFactory impl).
##########
src/test/java/org/apache/commons/xml/osgi/OsgiIT.java:
##########
@@ -0,0 +1,246 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.xml.osgi;
+
+import static org.junit.jupiter.api.Assertions.assertAll;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.lang.reflect.InvocationTargetException;
+import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.Enumeration;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.ServiceLoader;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.DynamicTest;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Order;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestFactory;
+import org.junit.jupiter.api.TestMethodOrder;
+import org.junit.platform.engine.FilterResult;
+import org.junit.platform.engine.discovery.DiscoverySelectors;
+import org.junit.platform.engine.support.descriptor.MethodSource;
+import org.junit.platform.launcher.PostDiscoveryFilter;
+import org.junit.platform.launcher.TagFilter;
+import org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder;
+import org.junit.platform.launcher.core.LauncherFactory;
+import org.junit.platform.launcher.listeners.SummaryGeneratingListener;
+import org.junit.platform.launcher.listeners.TestExecutionSummary;
+import org.osgi.framework.Bundle;
+import org.osgi.framework.BundleContext;
+import org.osgi.framework.Constants;
+import org.osgi.framework.FrameworkEvent;
+import org.osgi.framework.launch.Framework;
+import org.osgi.framework.launch.FrameworkFactory;
+import org.osgi.framework.wiring.BundleWiring;
+
+/**
+ * Runs the unit test suite inside an OSGi framework, against the built bundle.
+ *
+ * <p>
+ * The tests catch class-loading assumptions that only hold on a flat
classpath: an OSGi bundle class loader sees its own classes, the packages it
imports and
+ * {@code java.*}, but not JDK-internal packages such as {@code com.sun.*}.
The built jar is installed with its generated manifest, so the bundle also
proves
+ * that its {@code Import-Package} header resolves.
+ * </p>
+ * <p>
+ * The test output directory, which the build gives a fragment manifest, is
attached to that bundle, so the tests share its class loader and keep their
+ * package-private access. The test harness (JUnit, Mockito, JAXB) is not
under test and is boot-delegated to the class path. Only the JDK's built-in JAXP
+ * implementations are used, as in the {@code test-stockjdk} execution:
provider lookups cannot see other bundles without a mediator such as SPI Fly.
+ * </p>
+ * <p>
+ * The framework is whichever single {@link FrameworkFactory} the class path
provides; the build runs this class once with Apache Felix and once with Eclipse
+ * Equinox.
+ * </p>
+ */
+@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
+class OsgiIT {
+
+ /**
+ * Factory methods that reach the JDK's built-in implementations without a
provider lookup: {@code SecureSchemaFactory} also initializes the
+ * {@code LSResourceResolver} floor.
+ */
+ private static final String[][] DEFAULT_FACTORIES =
{{"SecureDocumentBuilderFactory", "newDefaultInstance"},
+ {"SecureSAXParserFactory", "newDefaultInstance"},
{"SecureSchemaFactory", "newDefaultInstance"}, {"SecureTransformerFactory",
"newDefaultInstance"},
+ {"SecureXPathFactory", "newDefaultInstance"},
{"SecureXMLInputFactory", "newDefaultFactory"}};
+
+ /**
+ * Package roots the framework delegates to the class path: the test
harness only, never the library or JDK internals.
+ */
+ private static final String[] HARNESS_PACKAGES = {"org.junit",
"org.opentest4j", "org.apiguardian", "org.mockito", "net.bytebuddy",
"org.objenesis",
+ "javax.xml.bind", "com.sun.xml.bind", "org.glassfish.jaxb",
"com.sun.istack", "javax.activation", "jakarta.activation"};
+
+ /**
+ * Symbolic name of the library bundle.
+ */
+ private static final String HOST_SYMBOLIC_NAME =
"org.apache.commons.xml.secure";
+
+ /**
+ * Test classes that cannot run inside a framework: {@code
ShadingFootprintTest} reads {@code target/classes} directly.
+ */
+ private static final List<String> NOT_IN_OSGI =
Collections.singletonList("org.apache.commons.xml.secure.ShadingFootprintTest");
+
+ /**
+ * Test methods that cannot run inside a framework.
+ *
+ * <p>
+ * They name a test class as the JAXP factory, which JAXP loads through
the context class loader: that is the class path, which holds a second copy of
the
+ * test classes.
+ * </p>
+ */
+ private static final List<String> METHODS_NOT_IN_OSGI =
Arrays.asList("SecureDocumentBuilderFactoryTest#newDocumentBuilderWrapsDeclaredExceptions",
+ "SecureSAXParserFactoryTest#newXmlReaderWrapsDeclaredExceptions");
+
+ private static Framework framework;
+
+ private static Bundle host;
+
+ private static Path requiredPath(final String property) {
+ final String value = System.getProperty(property);
+ assertNotNull(value, "System property '" + property + "' must be set
by the build");
+ return Paths.get(value).toAbsolutePath();
+ }
+
+ @BeforeAll
+ static void startFramework() throws Exception {
+ final List<FrameworkFactory> factories = new ArrayList<>();
+ ServiceLoader.load(FrameworkFactory.class).forEach(factories::add);
+ assertEquals(1, factories.size(), () -> "Expected exactly one OSGi
framework on the class path, found " + factories);
+ final Path buildJar = requiredPath("buildJar");
+ final Path work = requiredPath("osgiWorkDirectory");
+ Files.createDirectories(work);
+ final Map<String, String> config = new HashMap<>();
+ config.put(Constants.FRAMEWORK_STORAGE,
work.resolve("storage").toString());
+ config.put(Constants.FRAMEWORK_STORAGE_CLEAN,
Constants.FRAMEWORK_STORAGE_CLEAN_ONFIRSTINIT);
+ config.put(Constants.FRAMEWORK_BUNDLE_PARENT,
Constants.FRAMEWORK_BUNDLE_PARENT_FRAMEWORK);
+ // A trailing ".*" only matches subpackages, so each root is listed
twice.
+ // sun.reflect, jdk.internal.reflect: the JDK defines generated
reflection accessors under the bundle loader.
+ // leaked: XSLTC rejects the host of Felix bundle:// URLs (for example
"5.0"), so the XML fixtures resolve as file: URLs instead.
+ config.put(Constants.FRAMEWORK_BOOTDELEGATION,
Stream.concat(Stream.of(HARNESS_PACKAGES).flatMap(p -> Stream.of(p, p + ".*")),
+ Stream.of("sun.reflect", "jdk.internal.reflect",
"leaked")).collect(Collectors.joining(",")));
+ // Both frameworks can fall back to the parent loader on their own;
disable it so only the explicit list above is delegated.
+ config.put("felix.bootdelegation.implicit", "false");
+ config.put("osgi.compatibility.bootdelegation", "false");
+ config.put("osgi.context.bootdelegation", "false");
+ framework = factories.get(0).newFramework(config);
+ framework.start();
Review Comment:
Looks ok to me.
--
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]
