RFR: 8328716: [TestBug] Screen capturing utility for failed tests [v6]

[Kevin Rushforth]

On Fri, 4 Apr 2025 20:57:06 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> tests/system/src/test/java/test/util/ScreenshotCapture.java line 93:
>> 
>>> 91:      */
>>> 92:     public static void writeScreenshot(PrintStream out) {
>>> 93:         out.println(ScreenshotCapture.takeScreenshotBase64("Screenshot:\ndata:image/png;base64,", null));
>> 
>> With the change I am suggesting to `takeScreenshotBase64 `, this would become:
>> 
>> 
>>         out.println("Screenshot:");
>>         out.println(takeScreenshotBase64());
>> 
>> 
>> That makes the `takeScreenshotBase64` API cleaner with a better separation of concerns.
>
> The reason I have prefix and postfix instead of what you are proposing is that in my case only one event is written to the output.  This might be important when both stderr and stdout are redirected to the same log file, in which case it might produce an interleaved result.
> 
> Also, prefix/postfix allow for more flexibility, such as outputting a JSON segment like
> 
> { screenshot:"..." }
> 
> or any other representation depending on the situation.

To be clear, I agree that the caller is the one who should create the representation you show above. What I object to is having the caller pass in the first part of the data URL. That's not a clean separation of concerns.

As for whether to keep the prefix / postfix _without_ the data URL part, I don't care as much. I don't see it as needed, but it's fine if you want to keep it.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1746#discussion_r2029433416