Skip to content
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.

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

Changes needed for TestNG to work for OpenJDK tests #1018

Merged
merged 3 commits into from
Apr 11, 2016

Conversation

shurymury
Copy link
Contributor

With Jigsaw, some tests need to be executed with only java.base module present. Some of the OpenJDK tests use TestNG.

This change includes minimal fixes to allow those tests to work.

@@ -43,6 +41,8 @@
import org.testng.internal.thread.graph.SuiteWorkerFactory;
import org.testng.junit.JUnitTestFinder;
import org.testng.log4testng.Logger;
import org.testng.remote.SuiteDispatcher;
Copy link
Contributor

Choose a reason for hiding this comment

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

the remote package was moved to https://github.com/testng-team/testng-remote in PR #953
is there any special reason you added it back?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ugh, sorry.

Lemme fix that.

@shurymury
Copy link
Contributor Author

I have reverted the un-intentional changes. My apologies.

I do, however, see that now JDK tests are failing with java.lang.NoClassDefFoundError: com/beust/jcommander/ParameterException - as of now I do not know the reason.
My hack applied to some older version of TestNG worked fine.

@juherr
Copy link
Member

juherr commented Apr 6, 2016

👍 For the PR. Could you just update the CHANGES file to warn about the api change?

About your issue, are you running the tests with IntelliJ? Someone else has the same problem: http://stackoverflow.com/questions/36443730/java-lang-noclassdeffounderror-though-testng-is-not-imported

@shurymury
Copy link
Contributor Author

  1. on java.lang.NoClassDefFoundError: com/beust/jcommander/ParameterException:

    I have, for now, added jcommander classes into the testng.jar which lives in the jtreg.home/lib. OpenJDK tests work for me right now. That is a separate issue.
  2. no - I am not running with IDEA.

    The only way I know to reproduce the original problem is to run some of the OpenJDK tests such as:

    jtreg.home/bin/jtreg -jdk:jdk9.home -javaoptions:"-limitmods java.base,java.management" java/util/concurrent/ConcurrentHashMap/ConcurrentAssociateTest.java

    You have to use a JDK built from sources after jigsaw integration:
    http://hg.openjdk.java.net/jdk9/jdk9/rev/f900d5afd9c8

    If I run that without my hack, it is failing with
    java.lang.NoClassDefFoundError: org/xml/sax/SAXException
  3. on changing the CHANGES

    Am I supposed to do that manually?

@juherr
Copy link
Member

juherr commented Apr 6, 2016

  1. Ok, maybe a dependency issue.
  2. Ok
  3. Yes

@cbeust
Copy link
Collaborator

cbeust commented Apr 7, 2016

This ClassNotFoundException is weird, I verified that JCommander is listed as a compile dependency in both build.gradle and Build.kt.

@shurymury
Copy link
Contributor Author

I have updated the repo with an extra line in CHANGES.txt. Did that on Fri, I think.

What would happen next?

Shura

On Apr 6, 2016, at 6:18 AM, Julien Herr [email protected] wrote:

Ok, maybe a dependency issue.
Ok
Yes

You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub #1018 (comment)

@juherr
Copy link
Member

juherr commented Apr 11, 2016

👍

Next step: waiting @cbeust approval and the merge :)

@cbeust
Copy link
Collaborator

cbeust commented Apr 11, 2016

Looks like we're down to just the removal of initLogger. Since this builds, I'm guessing TestNG doesn't use it and since it's in internals, I guess we're fine.

Merging!

@cbeust cbeust merged commit a9d5318 into testng-team:master Apr 11, 2016
@cbeust
Copy link
Collaborator

cbeust commented Apr 11, 2016

Next step for you @shurymury: use the TestNG SNAPSHOT to confirm you no longer need your custom version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants