936720 - Move animated image operator tests from reftests to mochitests

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Seth Fowler [:seth] [:s2h]]

Bug 896684 is a top orange that involves the current tests for animated GIF/PNG operators (found in image/tests/reftest/animated) intermittently failing to complete the test animation. This only happens on the B2G emulator, which runs in a VM. The problem is that the reftest expects the animation to be complete after 1 second, and whether because of wonky VM timing or just random slowness in the test environment, we don't always hit that deadline.

Other tests in the codebase have solved this problem in similar cases by using a mochitest instead of a reftest and taking snapshots repeatedly until either the test passes or the test harness times out. This means that in most cases the test finishes as quickly as possible, but in the rare case where the test takes more time we keep retrying until it passes.

We should do the same thing for these tests. One or two new mochitests need to be added that perform the same task as the existing animation operator tests, and then the old tests need to be removed.

Comment 1

[Seth Fowler [:seth] [:s2h]]

Attachment: (Part 1) - Add a new mochitest for animated image operators.

Here's the new test.

Comment 2

[Seth Fowler [:seth] [:s2h]]

Attachment: (Part 2) - Remove reftests for animated image operators.

And here we remove the old tests.

Comment 3

[Seth Fowler [:seth] [:s2h]]

Here's a try job to verify that the new mochitest works:

https://tbpl.mozilla.org/?tree=Try&rev=b7018fd0f20b

Comment 4

[Seth Fowler [:seth] [:s2h]]

Here's a single-platform try job to verify that removing the reftests didn't break anything:

https://tbpl.mozilla.org/?tree=Try&rev=068fd5847440

Comment 5

[Timothy Nikkel (:tnikkel)]

Comment on attachment 829663 [details] [diff] [review]
(Part 2) - Remove reftests for animated image operators.

Leave both sets (the old reftests and the new mochitests) to run in parallel for a bit to make sure we have all the issues sorted out? ie land this a little later

Comment 6

[Seth Fowler [:seth] [:s2h]]

Thanks for the review, Timothy! As per our discussion, it'd be a good idea to add the sync image decode flag when we do our drawing in the test. I'll add that before landing.

Comment 7

[Seth Fowler [:seth] [:s2h]]

(In reply to Timothy Nikkel (:tn) from comment #5)
> Leave both sets (the old reftests and the new mochitests) to run in parallel
> for a bit to make sure we have all the issues sorted out? ie land this a
> little later

Sure, sounds good.

Comment 8

[Seth Fowler [:seth] [:s2h]]

(In reply to Seth Fowler [:seth] from comment #6)
> Thanks for the review, Timothy! As per our discussion, it'd be a good idea
> to add the sync image decode flag when we do our drawing in the test. I'll
> add that before landing.

Actually this is the default; you need to explicitly request async decode. See here:

https://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp#4462

Comment 9

[Timothy Nikkel (:tnikkel)]

Oh, good to know.

Comment 10

[Seth Fowler [:seth] [:s2h]]

Pushed part 1:

https://hg.mozilla.org/integration/mozilla-inbound/rev/957acbe8ca22

Setting needinfo to remind myself to push part 2 once part 1 has cooked for a while.

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/957acbe8ca22

Comment 12

[Seth Fowler [:seth] [:s2h]]

All looks well. Pushed part 2:

https://hg.mozilla.org/integration/mozilla-inbound/rev/8a3b4b9b786c

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8a3b4b9b786c

Comment 14

[Seth Fowler [:seth] [:s2h]]

Per discussion with Ryan, we should uplift these as far back as they can go. To ESR24 ideally. (These are test-only patches, and the tests in question are ancient.)

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4afd3190ee4e
https://hg.mozilla.org/releases/mozilla-beta/rev/fbf07e6b7eba
https://hg.mozilla.org/releases/mozilla-esr24/rev/e658ac971548

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/fbf07e6b7eba