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.

Sign up for GitHub

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

8353138: Screen capture for test TaskbarPositionTest.java, failure case #24286

Closed
wants to merge 7 commits into from
+25 −5
Closed

8353138: Screen capture for test TaskbarPositionTest.java, failure case #24286

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9cd5925
8353138: Screen capture for test TaskbarPositionTest.java, failure case
Renjithkannath Mar 28, 2025
cec51c1
Updated copyright year
Renjithkannath Apr 1, 2025
b5dc54a
Updated based on suggesion
Renjithkannath Apr 3, 2025
fd8f1f0
Updated screen capture function
Renjithkannath Apr 3, 2025
3cd8068
Moved new function to bottom
Renjithkannath Apr 4, 2025
a24b531
Fixed spacing
Renjithkannath Apr 4, 2025
16e7171
Moved function to last
Renjithkannath Apr 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion test/jdk/javax/swing/Popup/TaskbarPositionTest.java
View file
Original file line number Diff line number Diff line change
@@ -37,7 +37,11 @@
import java.awt.event.KeyEvent;
import java.awt.event.MouseAdapter;
import java.awt.event.MouseEvent;
import java.awt.image.BufferedImage;
import java.io.File;
import java.io.IOException;

import javax.imageio.ImageIO;
import javax.swing.AbstractAction;
import javax.swing.JComboBox;
import javax.swing.JFrame;
@@ -74,6 +78,7 @@ public class TaskbarPositionTest implements ActionListener {
private static JFrame frame;
Copy link
Member

Choose a reason for hiding this comment

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

Nit: I don't think * @run main TaskbarPositionTest is needed. But it's fine as it is too, don't have any strong feelings

Copy link
Member

Choose a reason for hiding this comment

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

It may not work without explicit @run because the test contains @build tags.

@run is implied if there are no other commands, such as @compile or @build, but it's not the case in this test.

Copy link
Member

Choose a reason for hiding this comment

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

Nit: copyright year

private static JPopupMenu popupMenu;
private static JPanel panel;
private static Robot robot;
Copy link
Member

Choose a reason for hiding this comment

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

Do you think it would be better to have Robot initialisation in this way? IMO it will make the robot final, so there is no way to overwrite it in the future as well as it would be easier to debug and not be initialised in the main method of the class.

    private static final Robot robot;

    static {
        try {
            robot = new Robot();
        } catch (AWTException e) {
            throw new RuntimeException(e);
        }
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since its a test, I didn't expect any override of this class in future, my intention is add this feature with minimal change.

Copy link
Member

Choose a reason for hiding this comment

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

I agree with Renjith.

If you call saveScreenCapture once in the main method as I suggest, you can pass robot to saveScreenCapture, in which case robot can remain local variable.


private static JComboBox<String> combo1;
private static JComboBox<String> combo2;
@@ -209,25 +214,38 @@ private void maybeShowPopup(MouseEvent e) {
}
}

// for debugging purpose, saves screen capture when test fails.
private static void saveScreenCapture(Rectangle bounds) {
BufferedImage image = robot.createScreenCapture(bounds);
try {
ImageIO.write(image,"png", new File("Screenshot.png"));
} catch (IOException e) {
e.printStackTrace();
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure it's the best idea to hide the error and just print it into stdout. Wouldn't it be better to just throw the error, what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think its a good idea

  1. IOException was not the original intention of this test and its only for additional information
  2. If we throw the error we will miss the actual exception, potentially that was the next instruction.

Copy link
Member

@aivanov-jdk aivanov-jdk Apr 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

No, we don't care about IOException. It can occur only after the test already failed, and it's important to preserve the original exception.

}
}

/**
* Tests if the popup is on the screen.
*/
private static void isPopupOnScreen(JPopupMenu popup, Rectangle checkBounds) {
if (!popup.isVisible()) {
saveScreenCapture(checkBounds);
throw new RuntimeException("Popup not visible");
}
Dimension dim = popup.getSize();
Point pt = popup.getLocationOnScreen();
Rectangle bounds = new Rectangle(pt, dim);

if (!SwingUtilities.isRectangleContainingRectangle(checkBounds, bounds)) {
saveScreenCapture(checkBounds);
throw new RuntimeException("Popup is outside of screen bounds "
+ checkBounds + " / " + bounds);
}
}

private static void isComboPopupOnScreen(JComboBox<?> comboBox) {
if (!comboBox.isPopupVisible()) {
saveScreenCapture(screenBounds);
throw new RuntimeException("ComboBox popup not visible");
}
JPopupMenu popupMenu = (JPopupMenu) comboBox.getUI().getAccessibleChild(comboBox, 0);
@@ -339,7 +357,7 @@ public static void main(String[] args) throws Throwable {

try {
// Use Robot to automate the test
Robot robot = new Robot();
robot = new Robot();
robot.setAutoDelay(50);

SwingUtilities.invokeAndWait(TaskbarPositionTest::new);