On Tue, 1 Apr 2025 20:11:46 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> There is more than one way to sk^H^H pet the cat. >> >> We could use a property to disable (or rather, enable) the screenshots, and >> only enable the capture during the debugging session. This will prevent us >> from catching those hard-to-reproduce intermittent tests that fail only >> occasionally. >> >> The other concern is that in the case of some infrastructure >> misconfiguration (for example, a sudden loss of screen capture permission) >> would result in log file size explosion, and I don't think we have >> `tailwatch`-like facility to halt the test run when this happens. >> >> I think the debug-only annotation is a meaningful compromise, but I would >> very much welcome other ideas. >> >> (I am ok with removing the annotation from two tests that currently use it). > > Yes, there are two main approaches we could take: > > Option 1: A utility that is available for developers to add to one or more > specific tests in a branch in their personal fork when debugging failures in > those tests. We would not add it to any test in the mainline repo in this > case. Since it is a developer-only utility, it isn't necessary to have a > system property to enable it. > > Option 2: A utility that is added to all robot tests in the repo, with a flag > to enable it (off by default). > > There are pros / cons of each approach. The second approach is more likely to > catch a failing test that fails rarely and would easily allow us to enable it > and run it on a new platform (e.g., for testing a new version of macOS or > Ubuntu), although it is also more prone to the "runaway" screenshot problem > unless we can come up with a good way to limit it. > > I left a couple usability questions that are independent of this. It probably > makes sense to address those first and then come back to this. I would choose the Option 1 (debug utility), with an addition of a static `ScreenCapture.takeScreenshot()`. I don't think there is a need to add this tool to the hundreds of tests which work just fine. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1746#discussion_r2023655635