Skip to content

test: improve osgi tests #3213

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
krmahadevan merged 1 commit into from
Mar 6, 2025
Merged

test: improve osgi tests #3213

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions testng-test-osgi/src/test/java/org/testng/test/osgi/DefaultTestngOsgiOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,23 @@ public static ModifiableCompositeOption defaultTestngOsgiOptions() {
systemProperty("org.ops4j.pax.url.mvn.localRepository")
.value(System.getProperty("testng.org.ops4j.pax.url.mvn.localRepository")),
mavenBundle("org.testng", "testng").versionAsInProject(),
mavenBundle("org.jcommander", "jcommander").versionAsInProject(),
mavenBundle("org.assertj", "assertj-core").versionAsInProject(),
mavenBundle("net.bytebuddy", "byte-buddy").versionAsInProject(),
mavenBundle("com.google.inject", "guice").versionAsInProject(),
mavenBundle("org.yaml", "snakeyaml").versionAsInProject(),
mavenBundle("com.google.guava", "guava").versionAsInProject(),
mavenBundle("com.google.guava", "failureaccess").versionAsInProject(),
mavenBundle("com.google.guava", "listenablefuture").versionAsInProject(),
mavenBundle("com.google.code.findbugs", "jsr305").versionAsInProject(),
mavenBundle("org.apache.servicemix.bundles", "org.apache.servicemix.bundles.aopalliance")
.versionAsInProject(),
mavenBundle("org.checkerframework", "checker-qual").versionAsInProject(),
mavenBundle("com.google.errorprone", "error_prone_annotations").versionAsInProject(),
mavenBundle("com.google.j2objc", "j2objc-annotations").versionAsInProject(),
mavenBundle(
"org.apache.aries.spifly", "org.apache.aries.spifly.dynamic.framework.extension")
.versionAsInProject(),
systemProperty("logback.configurationFile")
.value(System.getProperty("logback.configurationFile")),
mavenBundle("org.slf4j", "slf4j-api").versionAsInProject(),
Expand Down
37 changes: 29 additions & 8 deletions testng-test-osgi/src/test/java/org/testng/test/osgi/PlainOsgiTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,20 @@
import static org.ops4j.pax.exam.CoreOptions.options;
import static org.testng.test.osgi.DefaultTestngOsgiOptions.defaultTestngOsgiOptions;

import java.lang.reflect.Method;
import java.io.ByteArrayInputStream;
import java.nio.charset.StandardCharsets;
import org.ops4j.pax.exam.Configuration;
import org.ops4j.pax.exam.Option;
import org.ops4j.pax.exam.spi.reactors.ExamReactorStrategy;
import org.ops4j.pax.exam.spi.reactors.PerMethod;
import org.ops4j.pax.exam.testng.listener.PaxExam;
import org.testng.IModuleFactory;
import org.testng.TestNG;
import org.testng.annotations.Listeners;
import org.testng.annotations.Test;
import org.testng.internal.Version;
import org.testng.internal.YamlParser;
import org.testng.xml.XmlSuite;

/**
* The purpose of the class is to ensure {@code postgresql} bundle activation does not fail in case
Expand All @@ -25,17 +31,32 @@ public Option[] config() {
return options(defaultTestngOsgiOptions());
}

// TODO: Enable this test once the PR https://github.com/ops4j/org.ops4j.pax.exam2/pull/1112
// gets merged and there's a new release done.
@Test(enabled = false)
@Test
public void versionShouldStartWithDigit() throws Exception {
Class<?> versionClass = Class.forName("org.testng.internal.Version");
Method getVersionStringMethod = versionClass.getMethod("getVersionString");
Object version = getVersionStringMethod.invoke(null);
String version = String.valueOf(Version.getVersionString());

assertThat(version)
.matches(
(v) -> String.valueOf(v).length() > 0 && Character.isDigit(String.valueOf(v).charAt(0)),
(v) -> !v.isEmpty() && Character.isDigit(v.charAt(0)),
"Version.getVersionString() should start with a digit but was " + version);
}

@Test
public void guiceModuleFactoryLoads() {
assertThat(IModuleFactory.class.getMethods()).isNotEmpty();
}

@Test
public void jcommanderLoads() {
assertThat(TestNG.class.getFields()).isNotEmpty();
}

@Test
public void yamlLoads() {
YamlParser parser = new YamlParser();
ByteArrayInputStream input =
new ByteArrayInputStream("name: My_Suite\n".getBytes(StandardCharsets.UTF_8));
XmlSuite suite = parser.parse("test.yml", input, false);
assertThat(suite).extracting("name").isEqualTo("My_Suite");
}
}
28 changes: 23 additions & 5 deletions testng-test-osgi/testng-test-osgi-build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,40 @@ val testngRepository by configurations.creating {
dependencies {
testngRepository(projects.testng)

testImplementation(projects.testng)
testImplementation(projects.testng) {
capabilities {
requireFeature("guice")
}
}
testImplementation(projects.testng) {
capabilities {
requireFeature("yaml")
}
}

testImplementation("javax:javaee-api:8.0.1") {
// The dependency was not obvious, logged https://github.com/cbeust/testng/issues/2578
because("PaxExam uses javax.transaction.NotSupportedException and other classes")
}
testImplementation("org.ops4j.pax.exam:pax-exam-container-native:4.13.1")
testImplementation("org.ops4j.pax.exam:pax-exam-container-native:4.14.0")
// pax-exam is not yet compatible with junit5
// see https://github.com/ops4j/org.ops4j.pax.exam2/issues/886
testImplementation("org.ops4j.pax.exam:pax-exam-testng:4.13.5")
testImplementation("org.ops4j.pax.exam:pax-exam-link-mvn:4.13.5")
testImplementation("org.ops4j.pax.exam:pax-exam-testng:4.14.0")
testImplementation("org.ops4j.pax.exam:pax-exam-link-mvn:4.14.0")
testImplementation("org.ops4j.pax.url:pax-url-aether:2.6.12")
testImplementation("org.apache.felix:org.apache.felix.framework:7.0.5")
testImplementation("ch.qos.logback:logback-core:1.4.11")
testImplementation("ch.qos.logback:logback-classic:1.4.11")
Comment on lines 41 to 42
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Verify logback version constraints

Note that there's a dependency constraint to keep logback below 1.3.0 as mentioned in a previous review comment. The current version 1.4.11 exceeds this constraint.

🏁 Script executed:

#!/bin/bash
# Check if the current logback version conflicts with project requirements

echo "Checking logback version constraints..."
echo "Current version in build file: 1.4.11"
echo "Previous review comment indicated a constraint to stay below 1.3.0"

# Search for any logback-related issues in the code
echo "Checking for logback imports in the codebase:"
rg -l "import.*logback" --type java

# Check for possible compatibility issues with slf4j
echo "Checking for slf4j version to ensure compatibility with logback 1.4.x:"
rg -A 3 "slf4j.*version" --type kotlin

Length of output: 1032

Action Required: Logback Version Conflict

The dependency in testng-test-osgi/testng-test-osgi-build.gradle.kts still uses Logback 1.4.11, which violates the previously established constraint that Logback must remain below version 1.3.0. The output of the verification script confirms that this version is in use, and no updates elsewhere in the codebase indicate an intentional change of constraint.

  • File: testng-test-osgi/testng-test-osgi-build.gradle.kts (lines 41-42)
  • Issue: Using Logback 1.4.11 instead of a version below 1.3.0.
  • Action: Please update the Logback dependencies to a version that conforms with the project constraints or provide an updated rationale and adjustments across the project (including compatibility checks with SLF4J as seen in testng/testng-build.gradle.kts).

testRuntimeOnly("org.assertj:assertj-core:3.23.1")
testRuntimeOnly("org.apache.servicemix.bundles:org.apache.servicemix.bundles.aopalliance:1.0_6") {
because("Guice requires org.aopalliance.intercept package in osgi, however, aopalliance:aopalliance has no osgi headers")
}
testRuntimeOnly("com.google.errorprone:error_prone_annotations:2.36.0") {
because("It is needed for Guava, only recent version of error_prone_annotations have osgi headers")
}
testRuntimeOnly("org.apache.aries.spifly:org.apache.aries.spifly.dynamic.framework.extension:1.3.7") {
because("slf4j-api 2.0 requires osgi.serviceloader.processor, see https://stackoverflow.com/a/77867804")
}
}

// <editor-fold defaultstate="collapsed" desc="Pass dependency versions to pax-exam container">
Expand Down Expand Up @@ -83,5 +102,4 @@ tasks.test {
"-Dtestng.org.ops4j.pax.url.mvn.localRepository=file:${paxLocalCacheRepository.get().asFile.absolutePath}@id=pax-repo"
)
})
ignoreFailures = true
}
11 changes: 6 additions & 5 deletions testng/testng-build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,12 @@ tasks.mergedJar {
"Bundle-Description" to project.description,
"Bundle-Version" to project.version.toString().removeSuffix("-SNAPSHOT"),
"Import-Package" to """
bsh.*;version="[2.0.0,3.0.0)";resolution:=optional,
com.beust.jcommander.*;version="[1.7.0,3.0.0)";resolution:=optional,
com.google.inject.*;version="[1.2,1.3)";resolution:=optional,
org.yaml.*;version="[1.6,2.0)";resolution:=optional,
*;resolution:=optional
com.beust.jcommander;version="1.83",
org.slf4j;version="2.0",
com.google.inject;version="1.4";resolution:=optional,
org.yaml.snakeyaml;version="2.0";resolution:=optional,
org.yaml.snakeyaml.nodes;version="2.0";resolution:=optional,
org.yaml.snakeyaml.constructor;version="2.0";resolution:=optional
""".trimIndent().replace("\n", ""),
"Export-Package" to """
org.testng
Expand Down
Loading