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

JDK-8266254: Update to use jtreg 6 #4315

Closed

Conversation

jonathan-gibbons
Copy link
Contributor

@jonathan-gibbons jonathan-gibbons commented Jun 2, 2021

Please review the change to update to using jtreg 6.

The primary change is to the jib-profiles.js file, which specifies the version of jtreg to use, for those systems that rely on this file. In addition, the requiredVersion has been updated in the various TEST.ROOT files.

All the tests that could be updated ahead of time have been updated. There are a few tests remaining that need to be done at this time, because of the change in the module name for TestNG 7.3. It changed from a default of testng to an explicit org.testng.


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Issues

  • JDK-8266254: Update to use jtreg 6 ⚠️ Issue is not open.
  • JDK-8265020: tests must be updated for new TestNG module name ⚠️ Issue is not open.

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/4315/head:pull/4315
$ git checkout pull/4315

Update a local copy of the PR:
$ git checkout pull/4315
$ git pull https://git.openjdk.java.net/jdk pull/4315/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 4315

View PR using the GUI difftool:
$ git pr show -t 4315

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/4315.diff

@bridgekeeper
Copy link

bridgekeeper bot commented Jun 2, 2021

👋 Welcome back jjg! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk openjdk bot added the rfr Pull request is ready for review label Jun 2, 2021
@openjdk
Copy link

openjdk bot commented Jun 2, 2021

@jonathan-gibbons The following labels will be automatically applied to this pull request:

  • build
  • compiler
  • core-libs
  • hotspot

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

@openjdk openjdk bot added hotspot hotspot-dev@openjdk.org build build-dev@openjdk.org core-libs core-libs-dev@openjdk.org compiler compiler-dev@openjdk.org labels Jun 2, 2021
@jonathan-gibbons
Copy link
Contributor Author

/issue add jdk-8265020

@openjdk
Copy link

openjdk bot commented Jun 2, 2021

@jonathan-gibbons
Adding additional issue to issue list: 8265020: tests must be updated for new TestNG module name.

@mlbridge
Copy link

mlbridge bot commented Jun 2, 2021

Webrevs

@openjdk
Copy link

openjdk bot commented Jun 2, 2021

@jonathan-gibbons This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8266254: Update to use jtreg 6
8265020: tests must be updated for new TestNG module name

Reviewed-by: lancea, erikj, mchung, naoto, alanb, iris, chegar

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 31 new commits pushed to the master branch:

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

@openjdk openjdk bot added the ready Pull request is ready to be integrated label Jun 2, 2021
Copy link
Member

@naotoj naotoj left a comment

Choose a reason for hiding this comment

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

Some of the modified files have copyright year left unchanged. 2021 needs to be appended.

Copy link
Contributor

@AlanBateman AlanBateman left a comment

Choose a reason for hiding this comment

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

Looks good, I had expected we would have more tests depending on the automatic module.

@jonathan-gibbons
Copy link
Contributor Author

/integrate

@openjdk openjdk bot closed this Jun 2, 2021
@openjdk
Copy link

openjdk bot commented Jun 2, 2021

@jonathan-gibbons Warning! Your commit did not result in any changes! No push attempt will be made.

@erikj79
Copy link
Member

erikj79 commented Jun 3, 2021

This PR wasn't handled properly when integrated. The change looks like it was pushed correctly and the bug was updated, so we are only missing some book keeping in the PR. I've filed https://bugs.openjdk.java.net/browse/SKARA-1069.

@mlbridge
Copy link

mlbridge bot commented Sep 1, 2021

Mailing list message from Matthias Klose on build-dev:

On 6/2/21 6:25 PM, Jonathan Gibbons wrote:

what's the status of this? Trying to run the tests for 17+29 using jtreg 6+1, I
get:

+ jtreg -conc:auto -verbose:summary -automatic -retain:none -ignore:quiet
-agentvm -timeout:5 -workDir:/tmp//hotspot/JTwork
-reportDir:jtreg-test-output//hotspot//0/JTreport
-jdk:build/bootcycle-build/images/jdk
-exclude:test/hotspot/jtreg/ProblemList.txt -dir:test/hotspot/jtreg
:hotspot_compiler :hotspot_gc :hotspot_runtime :hotspot_serviceability
Error: Unexpected exception occurred! java.lang.NumberFormatException: For input
string: "6+1"
java.lang.NumberFormatException: For input string: "6+1"
at
java.base/java.lang.NumberFormatException.forInputString(NumberFormatException.java:67)
at java.base/java.lang.Integer.parseInt(Integer.java:668)
at java.base/java.lang.Integer.parseInt(Integer.java:786)
at com.sun.javatest.regtest.tool.Version.parseDottedInts(Version.java:166)
at com.sun.javatest.regtest.tool.Version.compareTo(Version.java:185)
at com.sun.javatest.regtest.tool.Tool.run(Tool.java:1131)
at com.sun.javatest.regtest.tool.Tool.run(Tool.java:1079)
at com.sun.javatest.regtest.tool.Tool.main(Tool.java:148)
at com.sun.javatest.regtest.Main.main(Main.java:58)
+ exit_code=6

Matthias

@mlbridge
Copy link

mlbridge bot commented Sep 1, 2021

Mailing list message from Aleksey Shipilev on build-dev:

Hi Matthias,

On 7/1/21 1:28 PM, Matthias Klose wrote:

what's the status of this? Trying to run the tests for 17+29 using jtreg 6+1, I
get:

I saw a similar problem with some custom jtreg builds. IIRC, that happens when jtreg6 source is
built with "old" make/build-all.sh, not with the new "make/build.sh".

I know this version works for me with current jdk17 and jdk:
https://builds.shipilev.net/jtreg/jtreg-6+1.zip

--
Thanks,
-Aleksey

@jonathan-gibbons jonathan-gibbons deleted the 8266254.jtreg6 branch October 20, 2021 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build build-dev@openjdk.org compiler compiler-dev@openjdk.org core-libs core-libs-dev@openjdk.org hotspot hotspot-dev@openjdk.org ready Pull request is ready to be integrated rfr Pull request is ready for review
8 participants