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

Use reflection to access attach API internal APIs #2962

Merged
merged 1 commit into from
Sep 25, 2018

Conversation

pdbain-ibm
Copy link
Contributor

Remove unnecessary compiler options related to exposing the APIs.

Fixes
#2938.

@llxia I ran a personal build with sanity tests on Java 10. Would you kindly start builds to cover extended and Java 8. Thank you.

Signed-off-by: Peter Bain peter_bain@ca.ibm.com

@llxia
Copy link
Contributor

llxia commented Sep 20, 2018

Jenkins test all plinux all

@llxia
Copy link
Contributor

llxia commented Sep 20, 2018

Jenkins test sanity xlinux jdk11

@pdbain-ibm
Copy link
Contributor Author

This needs more work:

===============================================
Running test AttachAPISanity_SE80_0 ...
===============================================
AttachAPISanity_SE80_0 Start Time: Thu Sep 20 16:50:02 2018 Epoch Time (ms): 1537462202936
test with NoOptions
[IncludeExcludeTestAnnotationTransformer] [INFO] exclude file is /home/jenkins/jenkins-agent/workspace/PullRequest-Sanity-JDK8-linux_ppc-64_cmprssptrs_le-OpenJ9/openj9/test/TestConfig/../../jvmtest/TestConfig/resources/excludes/latest_exclude_8.txt
...
... TestNG 6.14.2 by Cédric Beust (cedric@beust.com)
...

[TargetManager] [ERROR] starting

[TargetManager] [ERROR] 0 failed

[TargetManager] [ERROR] Throwable: com.ibm.tools.attach.target.AttachHandler.getVmId(java.lang.String)

[TargetManager] [ERROR] java.lang.NoSuchMethodException: com.ibm.tools.attach.target.AttachHandler.getVmId(java.lang.String)

[TargetManager] [ERROR] 	at java.lang.Class.newNoSuchMethodException(Class.java:562)

[TargetManager] [ERROR] 	at java.lang.Class.throwExceptionOrReturnNull(Class.java:1195)

[TargetManager] [ERROR] 	at java.lang.Class.getMethodHelper(Class.java:1259)

[TargetManager] [ERROR] 	at java.lang.Class.getMethod(Class.java:1187)

[TargetManager] [ERROR] 	at org.openj9.test.attachAPI.TargetVM.main(TargetVM.java:46)

[TargetManager] [ERROR] starting

[TargetManager] [ERROR] 0 failed

[TargetManager] [ERROR] Throwable: com.ibm.tools.attach.target.AttachHandler.getVmId(java.lang.String)

[TargetManager] [ERROR] java.lang.NoSuchMethodException: com.ibm.tools.attach.target.AttachHandler.getVmId(java.lang.String)

@pdbain-ibm pdbain-ibm changed the title Use reflection to access attach API internal APIs WIP Use reflection to access attach API internal APIs Sep 21, 2018
@pdbain-ibm
Copy link
Contributor Author

Fixed and re-tested. @llxia would you kindly run sanity and extended builds on whichever platform is convenient?
Thank you.

org.eclipse.jdt.core.compiler.debug.sourceFile=generate
org.eclipse.jdt.core.compiler.problem.assertIdentifier=error
org.eclipse.jdt.core.compiler.problem.enumIdentifier=error
org.eclipse.jdt.core.compiler.source=1.8
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to check in this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. I included that by accident and have removed it.
Could we add .settings and other auto-generated files to the .gitignore file?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, I think we can.

Remove unnecessary compiler options related to exposing the APIs.

Signed-off-by: Peter Bain <peter_bain@ca.ibm.com>
@pdbain-ibm pdbain-ibm changed the title WIP Use reflection to access attach API internal APIs Use reflection to access attach API internal APIs Sep 25, 2018
@llxia
Copy link
Contributor

llxia commented Sep 25, 2018

Jenkins test all plinux all

@llxia
Copy link
Contributor

llxia commented Sep 25, 2018

Jenkins test sanity win jdk11

@llxia
Copy link
Contributor

llxia commented Sep 25, 2018

windows failure is a known issue: #2955

@llxia llxia merged commit c5cfc61 into eclipse-openj9:master Sep 25, 2018
pdbain-ibm added a commit to pdbain-ibm/openj9 that referenced this pull request Feb 8, 2019
Add methods to check for attach API initialization, get process ID, and get
virtual machine ID.

Use reflection to avoid compile-time dependency in test code on OpenJ9 internal
classes Replace some existing references to AttachHandler in favour of the new
methods.

This is further to "Use reflection to access attach API internal APIs" eclipse-openj9#2962.
This allows tests to remove references to attach API implementation classes and
compile without requiring `--add-exports
java.base/com.ibm.tools.attach.target`.

This is preparation for tests of new features in eclipse-openj9#4655.

[ci skip]

Signed-off-by: Peter Bain <peter_bain@ca.ibm.com>
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.

3 participants