On Tue, 1 Apr 2025 11:16:01 GMT, Renjith Kannath Pariyangad
<rkannathp...@openjdk.org> wrote:
>> Hi Reviewers,
>>
>> Added screen capture in case of test failure using Robot.
>>
>> Please review and let me know your suggestion if any.
>
> Renjith Kannath Pariyangad has updated the pull request incrementally with
> one additional commit since the last revision:
>
> Updated copyright year
Changes requested by aivanov (Reviewer).
test/jdk/javax/swing/Popup/TaskbarPositionTest.java line 218:
> 216:
> 217: // for debugging purpose, saves screen capture when test fails.
> 218: private static void saveScreenCapture(Rectangle bounds) {
The comment is redundant.
I propose to move this method to the bottom of the class — it's the least
important piece of code in the entire test class.
Modify the `saveScreenCapture` method to save a screenshot of all the screens.
test/jdk/javax/swing/Popup/TaskbarPositionTest.java line 221:
> 219: BufferedImage image = robot.createScreenCapture(bounds);
> 220: try {
> 221: ImageIO.write(image,"png", new File("Screenshot.png"));
Suggestion:
ImageIO.write(image, "png", new File("Screenshot.png"));
Space after the comma.
test/jdk/javax/swing/Popup/TaskbarPositionTest.java line 234:
> 232: saveScreenCapture(checkBounds);
> 233: throw new RuntimeException("Popup not visible");
> 234: }
I don't really like this approach, you're modifying several places where an
exception can be thrown, but there are other places.
I suggest adding a catch section into the `main` method:
robot.waitForIdle();
} catch (Throwable t) {
saveScreenCapture(screens);
throw t;
} finally {
SwingUtilities.invokeAndWait(() -> {
This way you capture screenshot in one place and it automatically covers *all
the cases* where an error is thrown, which avoids adding explicit calls to save
the screenshots.
You may pass `robot` too.
I think you should make a screenshot of all the screens.
-------------
PR Review: https://git.openjdk.org/jdk/pull/24286#pullrequestreview-2739457459
PR Review Comment: https://git.openjdk.org/jdk/pull/24286#discussion_r2026773666
PR Review Comment: https://git.openjdk.org/jdk/pull/24286#discussion_r2026774176
PR Review Comment: https://git.openjdk.org/jdk/pull/24286#discussion_r2026768384
