8343832: Enhance test summary with number of skipped tests

[Domest0s]

The output for Jtreg v7.5 was enhanced with information about skipped tests.

Use the additional information in the "summary" block printed at the end of make test target.

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

• JDK-8343832: Enhance test summary with number of skipped tests (Enhancement - P4) ⚠️ Issue is already resolved. Consider making this a "backport pull request" by setting the PR title to Backport <hash> with the hash of the original commit. See Backports.

Reviewers

• Magnus Ihse Bursie (@magicus - Reviewer)
• Erik Joelsson (@erikj79 - Reviewer)

Contributors

• Magnus Ihse Bursie <ihse@openjdk.org>

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/22245/head:pull/22245
$ git checkout pull/22245

Update a local copy of the PR:
$ git checkout pull/22245
$ git pull https://git.openjdk.org/jdk.git pull/22245/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 22245

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/22245.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back Domest0s! 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]

@Domest0s 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:

8343832: Enhance test summary with number of skipped tests

Co-authored-by: Magnus Ihse Bursie <ihse@openjdk.org>
Reviewed-by: ihse, erikj

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 117 new commits pushed to the master branch:

• ac76d8d: 8350824: New async logging gtest StallingModePreventsDroppedMessages fails
• eada1ea: 8350855: RISC-V: print offset by assert of patch_offset_in_conditional_branch
• 2af76de: 8350683: Non-C2 / minimal JVM crashes in the build on ppc64 platforms
• ab4b0ef: 8350889: GenShen: Break out of infinite loop of old GC cycles
• 0a4c5a8: 8347804: GenShen: Crash with small GCCardSizeInBytes and small Java heap
• f1398ec: 8350701: [JMH] test foreign.AllocFromSliceTest failed with Exception for size>1024
• 3ae80bf: 8349766: GenShen: Bad progress after degen does not always need full gc
• 2fd7156: 8347426: Invalid value used for enum Cell in iTypeFlow::StateVector::meet_exception
• 939815f: 8347040: C2: assert(!loop->_body.contains(in)) failed
• 8323ddf: 8346659: SnippetTaglet should report an error if provided ambiguous links
• ... and 107 more: https://git.openjdk.org/jdk/compare/0ef1c40991e703592fc79325bda1a6d2fc6caf4e...master

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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@magicus, @erikj79) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

[openjdk]

@Domest0s The following label will be automatically applied to this pull request:

• build

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

[mlbridge]

Webrevs

• 07: Full - Incremental (0912b019)
• 06: Full - Incremental (6af586a2)
• 05: Full - Incremental (4a1a9268)
• 04: Full - Incremental (e4bab4dc)
• 03: Full - Incremental (dda4e2fe)
• 02: Full - Incremental (a2003355)
• 01: Full - Incremental (71cc3a93)
• 00: Full (a1b898b3)

[magicus]

This looks good!

To be clear, I assisted in writing some of this code, so I think a secondary reviewer is in order.

/reviewers 2

[openjdk]

@magicus
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

[Domest0s]

Thanks for reviewing this @magicus . Is there a way to add you as a co-author?

/contributor add @magicus

[openjdk]

@Domest0s
Contributor Magnus Ihse Bursie <ihse@openjdk.org> successfully added.

[openjdk]

⚠️ @Domest0s the full name on your profile does not match the author name in this pull requests' HEAD commit. If this pull request gets integrated then the author name from this pull requests' HEAD commit will be used for the resulting commit. If you wish to push a new commit with a different author name, then please run the following commands in a local repository of your personal fork:

$ git checkout 8343832_enhance_test_summary
$ git commit --author='Preferred Full Name <you@example.com>' --allow-empty -m 'Update full name'
$ git push

[Domest0s]

It should have been obvious to me that other test frameworks like gtest and microbenchmarking also use that same "run-test-report" target-routine to present results and so they also have to define the variable for the number of skipped tests.
IMO, it would make sense for gtest to count disabled test as SKIPPED in a final report.
For the rest of the test frameworks, the number of skipped tests will be set to 0.

Any objections?

[bridgekeeper]

@Domest0s This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[Domest0s]

Don't close it just yet, bot.

[Domest0s]

JDK moved to the newer JTREG 7.5.1 - https://bugs.openjdk.org/browse/JDK-8339238.
I merged the branch with master.

[magicus]

The GHA tests all look failed since .github/scripts/gen-test-summary.sh does not know about this new output format. The fix should be fairly trivial, but a somewhat more interesting question is if the number of skipped tests should be reported to the user, not as an error but as a warning or informational message. On the one hand, it seems prudent to inform users that not all tests where run as requested, but on the other hand, most users likely don't care, they just want to run "tier1" with whatever it includes, and printing the number of skipped tests for all test runs will likely only produce clutter and noise.

Actually, I think I answered myself. :-) We should not inform about skipped tests in GHA.

[magicus]

Actually, my assessment was wrong -- it is not the GHA script that is the cause of failure. Instead, it was the changes in RunTests.gmk that were not complete. They tried to detect if there were any errors by comparing the number of successful tests with the total number of tests, but this is no longer an uphold invariant. Another side effect of this was that even when run locally, tests suites with passed tests would be marked as failed.

It was a bit tricky to describe what was needed for this, so I made a patch at
6784dea. This is currently running in GHA at https://github.com/magicus/jdk/actions/runs/13551422706, but I expect it to pass, since the script behaves properly local on my computer.

[openjdk]

⚠️ @Domest0s This pull request contains merges that bring in commits not present in the target repository. Since this is not a "merge style" pull request, these changes will be squashed when this pull request in integrated. If this is your intention, then please ignore this message. If you want to preserve the commit structure, you must change the title of this pull request to Merge <project>:<branch> where <project> is the name of another project in the OpenJDK organization (for example Merge jdk:master).

[magicus]

Looks good now to me. Technically, I assume it would be good if you get a re-review of the latest commit, since I should not be reviewing my own code.

[Domest0s]

Thank you, @magicus, @erikj79 for helping me with this MR.

[Domest0s]

/integrate

[openjdk]

@Domest0s
Your change (at version 0912b01) is now ready to be sponsored by a Committer.

[LudwikJaniuk]

/sponsor

[openjdk]

Going to push as commit 2019f44.
Since your change was applied there have been 117 commits pushed to the master branch:

• ac76d8d: 8350824: New async logging gtest StallingModePreventsDroppedMessages fails
• eada1ea: 8350855: RISC-V: print offset by assert of patch_offset_in_conditional_branch
• 2af76de: 8350683: Non-C2 / minimal JVM crashes in the build on ppc64 platforms
• ab4b0ef: 8350889: GenShen: Break out of infinite loop of old GC cycles
• 0a4c5a8: 8347804: GenShen: Crash with small GCCardSizeInBytes and small Java heap
• f1398ec: 8350701: [JMH] test foreign.AllocFromSliceTest failed with Exception for size>1024
• 3ae80bf: 8349766: GenShen: Bad progress after degen does not always need full gc
• 2fd7156: 8347426: Invalid value used for enum Cell in iTypeFlow::StateVector::meet_exception
• 939815f: 8347040: C2: assert(!loop->_body.contains(in)) failed
• 8323ddf: 8346659: SnippetTaglet should report an error if provided ambiguous links
• ... and 107 more: https://git.openjdk.org/jdk/compare/0ef1c40991e703592fc79325bda1a6d2fc6caf4e...master

Your commit was automatically rebased without conflicts.

[openjdk]

@LudwikJaniuk @Domest0s Pushed as commit 2019f44.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.