Add VR Shell image diff instrumentation tests

Add VR Shell image diff instrumentation tests

Adds basic image diffing capabilities to the VR Shell instrumentation tests
and adds image diffing versions of the enter/exit VR mode tests. Will require
adding new reference images as more devices with different resolutions are
tested (currently only has 2560 x 1440).

BUG=

Committed: https://crrev.com/6314233276ce67d868511195ad213ed1bd0c74ef
Cr-Commit-Position: refs/heads/master@{#435038}

[bsheedy, 2016-10-26 17:28:57]

bsheedy@chromium.org changed reviewers:
+ bauerb@chromium.org, phajdan.jr@chromium.org

[bsheedy, 2016-10-26 17:28:58]

phajdan.jr@: PTAL at the //chrome/test changes

bauerb@: PTAL at the //chrome/android changes

Also, if either of you have any idea why Bitmap.sameAs is returning false when
(as far as I can tell) the images are identical, please enlighten me. I've taken
a look at the native comparison code at
https://cs.corp.google.com/android/frameworks/base/tools/layoutlib/bridge/src....
It looks to me like the only times it it could return false where my custom
method doesn't is if the delegates are null.

[Paweł Hajdan Jr., 2016-10-27 08:11:53]

https://codereview.chromium.org/2455683002/diff/1/chrome/android/javatests/sr...
File
chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/VrShellTest.java
(right):

https://codereview.chromium.org/2455683002/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/VrShellTest.java:134:
SystemClock.sleep(5000);
This is a known flakiness pattern.

Is there a bug on file to make this more robust?

https://codereview.chromium.org/2455683002/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/VrShellTest.java:166:
SystemClock.sleep(2000);
Same here.

[Bernhard Bauer, 2016-10-27 12:44:34]

bauerb@chromium.org changed reviewers:
+ peconn@chromium.org

[Bernhard Bauer, 2016-10-27 12:44:34]

+Peter, who has been working on a framework for image-based rendering tests (see
go/render-tests). You should maybe look into that?

[bsheedy, 2016-10-27 16:42:05]

On 2016/10/27 12:44:34, Bernhard Bauer wrote:
> +Peter, who has been working on a framework for image-based rendering tests
(see
> go/render-tests). You should maybe look into that?

Render tests look like they'll do the job. I'll re-write the tests to make use
of that.

[bsheedy, 2016-10-27 18:54:55]

On 2016/10/27 16:42:05, bsheedy wrote:
> On 2016/10/27 12:44:34, Bernhard Bauer wrote:
> > +Peter, who has been working on a framework for image-based rendering tests
> (see
> > go/render-tests). You should maybe look into that?
> 
> Render tests look like they'll do the job. I'll re-write the tests to make use
> of that.

I'm running into two issues while trying to use render tests, so maybe Peter can
provide a solution.

1. ViewRenderer.renderAndCompare is returning MISMATCH even though the two
images are identical according to ImageMagick and have the same md5 checksum. I
notice that ViewRenderer uses Bitmap.sameAs, so maybe this is related to my
earlier issue of Bitmap.sameAs not working as expected.

2. I tried replacing the sleeps with getInstrumentation().waitForIdleSync()
calls like the example tests in the render test doc, but that randomly causes
the test runner to fail - sometimes it fails before running any tests, other
times it only fails when trying to run the render test. Error log available at
https://paste.googleplex.com/4813738695720960

I know that the --regenerate-goldens option is currently broken on N (which is
what I'm testing on), but maybe N is breaking other things?
in_reply_to: 5676830073815040
send_mail: 1
subject: Add VR Shell image diff instrumentation tests

[PEConn, 2016-10-28 14:47:34]

On 2016/10/27 18:54:55, bsheedy wrote:
> On 2016/10/27 16:42:05, bsheedy wrote:
> > On 2016/10/27 12:44:34, Bernhard Bauer wrote:
> > > +Peter, who has been working on a framework for image-based rendering
tests
> > (see
> > > go/render-tests). You should maybe look into that?
> > 
> > Render tests look like they'll do the job. I'll re-write the tests to make
use
> > of that.
> 
> I'm running into two issues while trying to use render tests, so maybe Peter
can
> provide a solution.
> 
> 1. ViewRenderer.renderAndCompare is returning MISMATCH even though the two
> images are identical according to ImageMagick and have the same md5 checksum.
I
> notice that ViewRenderer uses Bitmap.sameAs, so maybe this is related to my
> earlier issue of Bitmap.sameAs not working as expected.

I'll take a look at this - what device did you use to generate the png?

> 2. I tried replacing the sleeps with getInstrumentation().waitForIdleSync()
> calls like the example tests in the render test doc, but that randomly causes
> the test runner to fail - sometimes it fails before running any tests, other
> times it only fails when trying to run the render test. Error log available at
> https://paste.googleplex.com/4813738695720960
> 
> I know that the --regenerate-goldens option is currently broken on N (which is
> what I'm testing on), but maybe N is breaking other things?
> in_reply_to: 5676830073815040
> send_mail: 1
> subject: Add VR Shell image diff instrumentation tests

It is worth noting that Render Tests aren't enabled just yet. A key prerequisite
is to allow developers to download the results of failed render tests (so they
can develop a feature, run the trybots, download the failed renders and upload
them to the CL if the changes are desired - they don't need to have the same
development device as the test writer). This is coming along soon though
(https://codereview.chromium.org/2439213002/).

Also, '--regenerate-goldens' is only broken because test_runner.py has
difficulty passing flags to devices running android N - the functionality for
generating the goldens is there, you just can't enable it by command line flag.
If you want to generate golden files, you can locally modify
RenderUtils.isGenerateMode() to return true.

[bsheedy, 2016-10-28 16:12:19]

On 2016/10/28 14:47:34, PEConn wrote:
> On 2016/10/27 18:54:55, bsheedy wrote:
> > On 2016/10/27 16:42:05, bsheedy wrote:
> > > On 2016/10/27 12:44:34, Bernhard Bauer wrote:
> > > > +Peter, who has been working on a framework for image-based rendering
> tests
> > > (see
> > > > go/render-tests). You should maybe look into that?
> > > 
> > > Render tests look like they'll do the job. I'll re-write the tests to make
> use
> > > of that.
> > 
> > I'm running into two issues while trying to use render tests, so maybe Peter
> can
> > provide a solution.
> > 
> > 1. ViewRenderer.renderAndCompare is returning MISMATCH even though the two
> > images are identical according to ImageMagick and have the same md5
checksum.
> I
> > notice that ViewRenderer uses Bitmap.sameAs, so maybe this is related to my
> > earlier issue of Bitmap.sameAs not working as expected.
> 
> I'll take a look at this - what device did you use to generate the png?
> 
> > 2. I tried replacing the sleeps with getInstrumentation().waitForIdleSync()
> > calls like the example tests in the render test doc, but that randomly
causes
> > the test runner to fail - sometimes it fails before running any tests, other
> > times it only fails when trying to run the render test. Error log available
at
> > https://paste.googleplex.com/4813738695720960
> > 
> > I know that the --regenerate-goldens option is currently broken on N (which
is
> > what I'm testing on), but maybe N is breaking other things?
> > in_reply_to: 5676830073815040
> > send_mail: 1
> > subject: Add VR Shell image diff instrumentation tests
> 
> It is worth noting that Render Tests aren't enabled just yet. A key
prerequisite
> is to allow developers to download the results of failed render tests (so they
> can develop a feature, run the trybots, download the failed renders and upload
> them to the CL if the changes are desired - they don't need to have the same
> development device as the test writer). This is coming along soon though
> (https://codereview.chromium.org/2439213002/).
> 
> Also, '--regenerate-goldens' is only broken because test_runner.py has
> difficulty passing flags to devices running android N - the functionality for
> generating the goldens is there, you just can't enable it by command line
flag.
> If you want to generate golden files, you can locally modify
> RenderUtils.isGenerateMode() to return true.

I used a 6P for the tests and golden image generation. And yes, I did modify the
source to have it always generate golden images.

[bsheedy, 2016-10-28 18:07:20]

https://codereview.chromium.org/2455683002/diff/1/chrome/android/javatests/sr...
File
chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/VrShellTest.java
(right):

https://codereview.chromium.org/2455683002/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/VrShellTest.java:134:
SystemClock.sleep(5000);
On 2016/10/27 08:11:53, Paweł Hajdan Jr. wrote:
> This is a known flakiness pattern.
> 
> Is there a bug on file to make this more robust?

Looks like getInstrumentation().waitForIdleSync() works as as replacement. Done.

https://codereview.chromium.org/2455683002/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/VrShellTest.java:166:
SystemClock.sleep(2000);
On 2016/10/27 08:11:53, Paweł Hajdan Jr. wrote:
> Same here.

Done.

[bsheedy, 2016-10-28 18:09:33]

Also, it looks like the issues with getInstrumentation().waitForIdleSync()
causing the test runner to fail have been fixed. Either restarting the device or
rebasing onto ToT fixed the issue. Now all that's left is the issue with
Bitmap.sameAs.

[bsheedy, 2016-10-28 18:09:33]

Also, it looks like the issues with getInstrumentation().waitForIdleSync()
causing the test runner to fail have been fixed. Either restarting the device or
rebasing onto ToT fixed the issue. Now all that's left is the issue with
Bitmap.sameAs.

[bsheedy, 2016-11-07 18:33:16]

Quick update: Rebased today, and Bitmap.sameAs appears to be working properly
now (tests are passing), so it looks like something was changed in the past week
that fixed it (not sure if it was the rebase or Android update, though). So,
this change should be ready to go through whenever you guys are happy with the
code.

[PEConn, 2016-11-08 13:57:31]

peconn@chromium.org changed reviewers:
+ jbudorick@chromium.org

[PEConn, 2016-11-08 13:57:32]

+John Budorick

What devices are Daydream ready? I'm assuming the aim for these tests would be
to run on the Testers and trybots (once Render Tests get running fully) and I
don't think any of them use 6Ps (they're mostly Nexus 5s at the moment).

I've added John who knows more about the testing infrastructure, but it'd be
good to either change the golden images to be for devices we test on or
acknowledge that these tests will only ever be run locally.

[bsheedy, 2016-11-08 17:16:08]

On 2016/11/08 13:57:32, PEConn wrote:
> +John Budorick
> 
> What devices are Daydream ready? I'm assuming the aim for these tests would be
> to run on the Testers and trybots (once Render Tests get running fully) and I
> don't think any of them use 6Ps (they're mostly Nexus 5s at the moment).
> 
> I've added John who knows more about the testing infrastructure, but it'd be
> good to either change the golden images to be for devices we test on or
> acknowledge that these tests will only ever be run locally.

Currently, only the Pixel and Pixel XL are actually Daydream-ready, although the
6P is currently treated as one until Pixels are more common for testing. The
plan at the moment is to enable these on our VR FYI bot
(https://build.chromium.org/p/chromium.fyi/builders/Android%20VR%20Tests?numbu...)
that's currently using a 6P. The inclusion of VrShellTest.java into
ChromePublicTest is behind a build flag that's only currently enabled on the FYI
waterfall, and won't be enabled on other waterfalls until a few other things get
sorted out (namely when VR-enabled non-component builds finally work).

We can either wait until then for the N5 images or add them now, although I
don't have ready access to a N5 myself (only Pixel, 6P, and 5X).

[bsheedy, 2016-11-21 17:50:05]

Anything else that needs to be done to get this landed?

[bsheedy, 2016-11-28 22:36:06]

I've added a Nexus 5 reference image, and the test passes (test image and golden
image match) when run locally on a Nexus 5. Does this look ready to land to
PEConn@/phajdan.jr@?

[Bernhard Bauer, 2016-11-29 13:01:06]

lgtm

[PEConn, 2016-11-29 13:02:40]

LGTM!

[bsheedy, 2016-11-29 17:37:44]

The CQ bit was checked by bsheedy@chromium.org

[commit-bot: I haz the power, 2016-11-29 17:37:57]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-29 18:33:06]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1480441064911910,
"parent_rev": "ff09abf919c0031780f0210cb0346d70fc9a34f3", "commit_rev":
"42ac4f2079333566eeef7c263d1af6609c78f9d2"}

[commit-bot: I haz the power, 2016-11-29 18:33:29]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-11-29 18:35:53]

Description was changed from

==========
Add VR Shell image diff instrumentation tests

Adds basic image diffing capabilities to the VR Shell instrumentation tests
and adds image diffing versions of the enter/exit VR mode tests. Will require
adding new reference images as more devices with different resolutions are
tested (currently only has 2560 x 1440).

BUG=
==========

to

==========
Add VR Shell image diff instrumentation tests

Adds basic image diffing capabilities to the VR Shell instrumentation tests
and adds image diffing versions of the enter/exit VR mode tests. Will require
adding new reference images as more devices with different resolutions are
tested (currently only has 2560 x 1440).

BUG=

Committed: https://crrev.com/6314233276ce67d868511195ad213ed1bd0c74ef
Cr-Commit-Position: refs/heads/master@{#435038}
==========

[commit-bot: I haz the power, 2016-11-29 18:35:53]

Patchset 4 (id:??) landed as
https://crrev.com/6314233276ce67d868511195ad213ed1bd0c74ef
Cr-Commit-Position: refs/heads/master@{#435038}