On Fri, 4 Apr 2025 20:57:06 GMT, Andy Goryachev <ango...@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
