On Fri, 4 Apr 2025 20:30:55 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> Andy Goryachev has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> javadoc
>
> 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.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1746#discussion_r2029425946
