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

Andy Goryachev angorya at openjdk.org
Fri Apr 4 21:45:54 UTC 2025


On Fri, 4 Apr 2025 21:39:32 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>>> hmm, not sure if I share the concern (did I understand the concern?)
>>> 
>>> the idea is to have a low-level method `takeScreenshotBase64(prefix, postfix)` that can be used in non-standard situations, but also provide the convenience method that will be used in the tests, `writeScreenshotBase64()`.
>> 
>> That part sounds fine. However, even in non-standard situations, the caller of `takeScreenshotBase64` should not be expected to pass `"data:image/png;base64,"` to that method. It is the method itself that knows that the format is PNG, so that method should provide that part of the string when converting it to a base64 screenshot. So, for example:
>> 
>> 
>>     takeScreenshotBase64(null, null);  <-- returns a data URL of a base64-encoded PNG
>>     takeScreenshotBase64("{ screenshot:", "}"); <-- return the same with the prefix and postfix
>> 
>> 
>> So what I'm trying to say is that `"data:image/png;base64,"` is an integral part of the base64-encoded image, not an optional prefix that a caller must pass in.
>
> One thing that happened recently (in the past 10 years or so) is that JSON became a frequently used format for logs.  It's not perfect, but it's easy to parse.  With that, json-oriented log viewers came.
> 
> We don't have a consistent way to log data (or show data in `.toString()`).  A good log viewer can pretty print an object in the log file, decode a hex- or base-64 encoded string, or even run a query on a log file.
> 
> We are not ready for JSON logs, I admit, but this was the rationale behind the design of this class:
> 
> - a low-level method that returns byte[]
> - a base-64 encoding method that allows for custom prefix/suffix to be able to do a data url or a json
> - a convenience method to use in tests
> 
> Also, considering what you said about gradle and stderr, perhaps `writeScreenshot()` should always emit to stderr.

Not a "good" log viewer, but to illustrate:

![Screenshot 2025-04-04 at 14 41 54](https://github.com/user-attachments/assets/241fe4da-ae03-44bb-9766-30c86d5af94c)

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

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


More information about the openjfx-dev mailing list