On Mon, 31 Mar 2025 14:58:11 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> tests/system/src/test/java/test/util/ScreenCaptureTestWatcher.java line 47:
>>
>>> 45: * <p>
>>> 46: * This facility takes a screenshot of any failed test, then logs the
>>> base64-encoded screenshot
>>> 47: * to {@code stderr}.
>>
>> This should not be on by default, but should be "opt in" on a System
>> property. I recommend defining a gradle property to do this and map it to
>> that System property, much like we do for "UNSTABLE_TEST" and others.
>
> The intent is not to annotate each test, but rather to use this tool to debug
> the issues in an intermittently failing test.
Hmm. That isn't what I thought we were doing. I thought the idea was to
annotate most (if not all all) screen capture tests, qualified by a system
property, enable that property in our CI test runs, so that when we do get
intermittent failures, we'll be able to take a look at them.
We _could_ to what you propose, but in that case I wouldn't want _any_ tests
annotated as part of this or any other PR. Once we have this annotation on any
test in our repo, then my objection about not having it on by default needs to
be addressed, at which point you might as well annotate more tests, since we
have seen occasional failures on many of them.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1746#discussion_r2021320825
