RFR: 8353138: Screen capture for test TaskbarPositionTest.java, failure case [v2]

[Alexey Ivanov]

On Tue, 1 Apr 2025 12:02:49 GMT, Renjith Kannath Pariyangad <rkannathpari at openjdk.org> wrote:

>> test/jdk/javax/swing/Popup/TaskbarPositionTest.java line 81:
>> 
>>> 79:     private static JPopupMenu popupMenu;
>>> 80:     private static JPanel panel;
>>> 81:     private static Robot robot;
>> 
>> 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);
>>         }
>>     }
>
> Since its a test, I didn't expect any override of this class in future, my intention is add this feature with minimal change.

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.

>> test/jdk/javax/swing/Popup/TaskbarPositionTest.java line 223:
>> 
>>> 221:             ImageIO.write(image,"png", new File("Screenshot.png"));
>>> 222:         } catch (IOException e) {
>>> 223:             e.printStackTrace();
>> 
>> 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?
>
> 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.

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

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/24286#discussion_r2026772299
PR Review Comment: https://git.openjdk.org/jdk/pull/24286#discussion_r2026769752