1766990 - Reftest analyzer output broken for android wrench tests

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Jamie Nicol [:jnicol]]

Since updating the version of cargo-apk we use to build wrench in bug 1615148, the reftest analyzer no longer works for failing reftests.

This is because on Android logcat truncates long lines, and the base64 encoded images we output are too long so they get truncated. The version of android-glue used by the old cargo-apk automatically split long lines whilst redirecting stdout and stderr to the logcat. We then stitched them back together again.

The equivalent ndk_glue crate used by the new cargo-apk does not do this, it simply forwards stdout and stderr to logcat line by line, regardless of their length.

We could patch ndk_glue to match the old behaviour. Or split the long strings ourselves before we we println them. But I think this has always been unreliable. I've seen broken images in the reftest analyzer before and I'm pretty sure this is the cause.

I'd rather we redirect stdout and stderr to a file instead, and pull that file from the device when parsing the log at the end. We should still redirect to logcat as well so that most of the information shows up there (but won't be usable in the reftest-analyzer)

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1615148

Comment 2

[Jamie Nicol [:jnicol]]

Attachment: Bug 1766990 - Redirect stdout and stderr to file for wrench tests on Android. r?#gfx-reviewers

On Android long lines are truncated by logcat, meaning the base64
images output by wrench's reftest harness get truncated. The glue code
used by the old version of cargo-apk automatically split long lines,
which we then stitched back together again when parsing the
log. However, the glue used by the new cargo-apk no longer does this.

While we could manually split the long strings ourselves, this method
has always been unreliable. Instead, this patch makes us additionally
write stdout and stderr to a file as well as logcat. The test runner
script then pulls this file from the device and scrapes it rather than
the logcat.

Depends on D145075

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5c65f68ac342
Redirect stdout and stderr to file for wrench tests on Android. r=gfx-reviewers,nical

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1615148

Comment 5

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/5c65f68ac342

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:jnicol, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 7

[Jamie Nicol [:jnicol]]

No need to uplift this, it just means the reftest analyzer doesn't work properly on wrench reftests.