testng-team / testng

TestNG testing framework
https://testng.org
Apache License 2.0
1.98k stars 1.02k forks source link

JPMS support #2727

Open cowwoc opened 2 years ago

cowwoc commented 2 years ago

TestNG Version

7.5

Expected behavior

TestNG runs tests.

Actual behavior

TestNG throws an exception:

[ERROR] java.lang.AssertionError: Couldn't find resource: jquery.min.js
[ERROR]     at org.testng@7.5/org.testng.reporters.jq.Main.generateReport(Main.java:91)
[ERROR]     at org.testng@7.5/org.testng.TestNG.generateReports(TestNG.java:1127)
[ERROR]     at org.testng@7.5/org.testng.TestNG.run(TestNG.java:1065)
[ERROR]     at org.apache.maven.surefire.testng.TestNGExecutor.run(TestNGExecutor.java:136)
[ERROR]     at org.apache.maven.surefire.testng.TestNGDirectoryTestSuite.executeMulti(TestNGDirectoryTestSuite.java:193)
[ERROR]     at org.apache.maven.surefire.testng.TestNGDirectoryTestSuite.execute(TestNGDirectoryTestSuite.java:94)
[ERROR]     at org.apache.maven.surefire.testng.TestNGProvider.invoke(TestNGProvider.java:145)
[ERROR]     at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:428)
[ERROR]     at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:162)
[ERROR]     at org.apache.maven.surefire.booter.ForkedBooter.run(ForkedBooter.java:562)
[ERROR]     at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:548)

The problem only occurs if the code being tested is a Java module (has module-info.java defined). The problem does not occur if module-info.java is not present, but I want to run tests as a module.

Is the issue reproducible on runner?

juherr commented 2 years ago

What if you disable default listeners?

cowwoc commented 2 years ago

@juherr It works fine as a workaround, but has the undesirable side-effect of disabling other remaining listeners that might be enabled by default.

Please let me know when I can test JPMS support without this workaround. Thank you.

wakingrufus commented 1 year ago

is this related to https://github.com/cbeust/testng/issues/2308 ?

krmahadevan commented 1 year ago

@cowwoc - Can you please help share a sample project that can be used to reproduce this issue ?

@wakingrufus - I am not sure if they are related to each other. #2308 is because of the way in which we attempt to locate classes.

midsorbet commented 1 year ago

Are there any plans to modularize the project by adding the module-info.java files?

krmahadevan commented 1 year ago

@bhuntay yes we do. Just havent managed to get around to this. Would you be willing to help create a Pull Request for this?

midsorbet commented 1 year ago

@krmahadevan sure I can take a look

midsorbet commented 1 year ago

Running into some issues with the javax.annotation dependency

> Task :testng-core-api:compileJava FAILED
/home/thinkpad/projects/testng/testng-core-api/src/main/java/org/testng/IInjectorFactory.java:6: error: package javax.annotation is not visible
import javax.annotation.Nullable;
            ^
  (package javax.annotation is declared in the unnamed module, but module org.testng.core.api does not read it)
/home/thinkpad/projects/testng/testng-core-api/src/main/java/org/testng/internal/Utils.java:21: error: package javax.annotation is not visible
import javax.annotation.Nullable;
            ^
  (package javax.annotation is declared in the unnamed module, but module org.testng.core.api does not read it)
/home/thinkpad/projects/testng/testng-core-api/src/main/java/org/testng/reporters/XMLStringBuffer.java:7: error: package javax.annotation is not visible
import javax.annotation.Nullable;
            ^
  (package javax.annotation is declared in the unnamed module, but module org.testng.core.api does not read it)
/home/thinkpad/projects/testng/testng-core-api/src/main/java/org/testng/reporters/XMLUtils.java:8: error: package javax.annotation is not visible
import javax.annotation.Nullable;
            ^
  (package javax.annotation is declared in the unnamed module, but module org.testng.core.api does not read it)
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
4 errors

Hoping that https://github.com/javax-inject/javax-inject/issues/33 fixes the issue or I will try to follow some other approach

cowwoc commented 1 year ago

Sorry. I don't have a project handy to reproduce this. Hopefully others can provide an example the next time this happens to them.

midsorbet commented 1 year ago

@krmahadevan would it be better to switch to jakarta.inject? seems to be a drop-in replacement for javax.inject with JPMS support.

krmahadevan commented 1 year ago

@bhuntay would you know if that would hurt backward compatibility? We dont want to inconvenience our users by having them fix the package structure because of the jakarta annotations. I dont think it should be a problem but please do check and confirm.

midsorbet commented 1 year ago

I think the Nullable/NonNullable annotations should not have an impact. But, I will look into it further before making any such changes.

midsorbet commented 1 year ago

@krmahadevan In the process of migrating a Gradle project to use JPMS, I utilized the Extra Java Module Info Gradle plugin to convert any transitive modules such as javax.inject into automatic modules for the project to consume.

I added module-info.java files to all the main source sets for each subproject in my fork. However, the project doesn't compile due to JPMS restrictions on split packages. For instance, the org.testng.internal package is present in multiple subprojects, which is not allowed by JPMS. We need to address this split package issue before we can proceed with adding the module-info.java files to the project.

juherr commented 1 year ago

The internal package is not an issue. It could be an inner package of the module.

@bhuntay Would you like to propose a pull request fixing that?

midsorbet commented 1 year ago

sure @juherr guess I will leave the core api packages untouched so as to not make it a breaking change

cowwoc commented 11 months ago

For what it's worth, the following works for me:

module org.testng
{
    exports org.testng;
    exports org.testng.annotations;
    exports org.testng.asserts;
    exports org.testng.collections;
    exports org.testng.junit;
    exports org.testng.log;
    exports org.testng.log4testng;
    exports org.testng.reporters;
    exports org.testng.thread;
    exports org.testng.util;
    exports org.testng.xml;
    exports org.testng.internal;

    requires org.slf4j;
    requires java.xml;
    requires java.sql;
    requires jcommander;
    requires bsh;

    uses org.testng.IAnnotationTransformer;
    uses org.testng.IHookable;
    uses org.testng.IMethodInterceptor;
    uses org.testng.ITestNGListener;
    uses org.testng.xml.ISuiteParser;
}

Unfortunately, we need to export org.testng.internal because otherwise the Surefire maven plugin fails with the following error:

java.lang.IllegalAccessError: superinterface check failed: class org.apache.maven.surefire.testng.ConfigurationAwareTestNGReporter (in unnamed module @0x167fdd33) cannot access class org.testng.internal.IResultListener (in module org.testng) because module org.testng does not export org.testng.internal to unnamed module @0x167fdd33

I suggest moving this interface out of the internal package.