1530462 - float-retry-push-image.html makes bogus/flaky positioning assumption

Assignee

Description

•

5 years ago

Test float-retry-push-image.html is flaky (and is currently annotated as being extremely fuzzy, except on webrender).

Its expectations are bogus because its shape-outside is a diagonal line that crosses the central point of a rectangle, and it's undefined whether or not the that central pixel is "shaded" and available for floats or not (which creates a positioning ambiguity).

It looks like we can fix this by adjusting the diagonal slightly to be at e.g. 50.01% rather than at 50%. (At least, that fixes the test locally.)

Daniel Holbert [:dholbert]

Assignee

Comment 1

•

5 years ago

(In reply to Daniel Holbert [:dholbert] from comment #0)

Test float-retry-push-image.html is flaky (and is currently annotated as being extremely fuzzy, except on webrender)

Its annotations are a bit inconsistent actually -- we have this "fuzzy everywhere" annotation in the reftest version:

fuzzy(0-255,0-500) == float-retry-push-image.html float-retry-push-ref.html
https://searchfox.org/mozilla-central/rev/b10ae6b7a50d176a813900cbe9dc18c85acd604b/layout/reftests/w3c-css/submitted/shapes1/reftest.list#109

...vs. this one in the WPT reimport version (this is where we have an "except webrender"):

if webrender and (os == "win"): PASS
FAIL
https://searchfox.org/mozilla-central/rev/b10ae6b7a50d176a813900cbe9dc18c85acd604b/testing/web-platform/meta/css/vendor-imports/mozilla/mozilla-central-reftests/shapes1/float-retry-push-image.html.ini#2

Anyway, the webrender exception is itself unreliable, per https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=229865751&repo=mozilla-inbound&lineNumber=20240

Let's just fix the test.

Daniel Holbert [:dholbert]

Assignee

Comment 2

•

5 years ago

Attached file Bug 1530462: Adjust reftest 'float-retry-push-image.html' to allow for rounding error on linear-gradient edge. r?bradwerth — Details

And while we're at it, update the reference case to have "Reference" in
its title, so that the reference case and testcases are easier to distinguish
when viewing them side-by-side in several tabs.

Daniel Holbert [:dholbert]

Assignee

Comment 3

•

5 years ago

The patch makes the test pass locally on my machine. Let's see if it fixes it elsewhere, too... Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d15321afc6dee0fcc56b0585082874010777421e

Note: the WPT copy of this test (the copy in testing/web-platform) will still be broken for the time being, but it'll become fixed after this makes the roundtrip through upstream WPT and gets reimported back into the tree.

As I understand it, this is the least-bumpy way to fix tests that exist in these two locations in our tree.

Daniel Holbert [:dholbert]

Assignee

Comment 4

•

5 years ago

Darn, Try shows that we're still flaky on some platforms even with the adjustment. (Possibly due to those platforms having HiDPI displays? not sure. I know Windows uses 125% pixel scaling by default, which I could imagine causing subtle weirdness here.)

New proposal: let's adjust the test to bake in 1px of vertical flexibility, by:

making the float 1px taller
adding a clip-path to its container, to only paint the region that we expect would be covered in an idealized scenario.

This allows the float to be positioned either at the "ideal" position or 1px upwards from it, and the test will still pass either way.

Phabricator Automation

Updated

•

5 years ago

Attachment #9046423 - Attachment description: Bug 1530462: Adjust reftest 'float-retry-push-image.html' to avoid rounding ambiguity and produce more consistent results. r?bradwerth → Bug 1530462: Adjust reftest 'float-retry-push-image.html' to allow for rounding error on linear-gradient edge. r?bradwerth

Daniel Holbert [:dholbert]

Assignee

Comment 5

•

5 years ago

Try looks good with the new clipping approach discussed in comment 4:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=155e7aeb41090fe58e0916dcd27716f06e1520ff&selectedJob=230445681

(The Linux x64 QuantumRender R8 oranges there are for an unrelated test, and seem intermittent or like an unrelated regression.)

Brad, does the updated patch look good to you? (I can't request re-review in phabricator, unfortunately.)

Daniel Holbert [:dholbert]

Assignee

Updated

•

5 years ago

Flags: needinfo?(bwerth)

Brad Werth [:bradwerth]

Comment 6

•

5 years ago

Yes, that's a good fix. I marked the patch as approved.

Flags: needinfo?(bwerth)

Daniel Holbert [:dholbert]

Assignee

Comment 7

•

5 years ago

Thank you!

Pulsebot

Comment 8

•

5 years ago

Pushed by dholbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e6333b7c1331
Adjust reftest 'float-retry-push-image.html' to allow for rounding error on linear-gradient edge. r=bradwerth

Natalia Csoregi [:nataliaCs]

Comment 9

•

5 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/e6333b7c1331

Status: ASSIGNED → RESOLVED

Closed: 5 years ago

status-firefox67: affected → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla67

David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)

Comment 10

•

5 years ago

WPT pull at https://github.com/web-platform-tests/wpt/pull/15581 ... which has a failure I don't understand.

Brad Werth [:bradwerth]

Comment 11

•

5 years ago

(In reply to David Baron :dbaron: 🏴󠁵󠁳󠁣󠁡󠁿 ⌚UTC-8 (if account gets disabled due to email bounces, ask a bugzilla admin to reenable it) from comment #10)

WPT pull at https://github.com/web-platform-tests/wpt/pull/15581 ... which has a failure I don't understand.

The key issue appears to be:

ERROR:lint:css/vendor-imports/mozilla/mozilla-central-reftests/shapes1/float-retry-push-image.html:51: Whitespace at EOL (TRAILING WHITESPACE)

Daniel Holbert [:dholbert]

Assignee

Comment 12

•

5 years ago

Aha, thanks (& sorry about that). I can fix that with a quick push to inbound.

Pulsebot

Comment 13

•

5 years ago

Pushed by dholbert@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8d678e501c5a
followup: remove end-of-line whitespace in testcase. (no review, trivial test-only change)

David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)

Comment 14

•

5 years ago

There were 2 failures: the lint failure in https://travis-ci.com/web-platform-tests/wpt/jobs/181034954 is easily understandable; I don't understand the wpt-firefox-nightly-stability failure at https://tools.taskcluster.net/groups/FEO4R2y5SQWQHSd0r4elHA/tasks/aBa71QnzTce7CWoawaeXLg/details I still don't understand.

Daniel Holbert [:dholbert]

Assignee

Comment 15

•

5 years ago

•

Edited

That one seems to be a harness issue during the "test-verify" testrun process.

The log is here:
https://taskcluster-artifacts.net/aBa71QnzTce7CWoawaeXLg/0/public/logs/live_backing.log

It trips over "NoSuchWindowException: Browsing context has been discarded" while trying to run the test 10 times. I'm guessing it might be intermittent?

TEST-START | /css/vendor-imports/mozilla/mozilla-central-reftests/shapes1/float-retry-push-circle.html
Run 0/10
PID 272 | 1551290483252	Marionette	INFO	Testing http://web-platform.test:8000/css/vendor-imports/mozilla/mozilla-central-reftests/shapes1/float-retry-push-circle.html == http://web-platform.test:8000/css/vendor-imports/mozilla/mozilla-central-reftests/shapes1/float-retry-push-ref.html
PID 272 | JavaScript error: resource:///modules/sessionstore/SessionStore.jsm, line 2383: TypeError: tabbrowser.getTabForBrowser is not a function
PID 272 | JavaScript error: resource:///modules/sessionstore/SessionStore.jsm, line 2383: TypeError: tabbrowser.getTabForBrowser is not a function
TEST-PASS | /css/vendor-imports/mozilla/mozilla-central-reftests/shapes1/float-retry-push-circle.html | took 663ms
TEST-START | /css/vendor-imports/mozilla/mozilla-central-reftests/shapes1/float-retry-push-circle.html
Run 1/10
PID 272 | 1551290483441	Marionette	INFO	No differences allowed
Error running command reset with arguments ():
Traceback (most recent call last):
  File "/home/test/web-platform-tests/tools/wptrunner/wptrunner/testrunner.py", line 100, in run
    rv = commands[command](*args)
  File "/home/test/web-platform-tests/tools/wptrunner/wptrunner/testrunner.py", line 113, in reset
    self.executor.reset()
  File "/home/test/web-platform-tests/tools/wptrunner/wptrunner/executors/executormarionette.py", line 784, in reset
    self.implementation.reset(**self.implementation_kwargs)
  File "/home/test/web-platform-tests/tools/wptrunner/wptrunner/executors/executormarionette.py", line 869, in reset
    self.setup(screenshot)
  File "/home/test/web-platform-tests/tools/wptrunner/wptrunner/executors/executormarionette.py", line 865, in setup
    self.executor.protocol.marionette._send_message("reftest:setup", data)
  File "/home/test/web-platform-tests/_venv/lib/python2.7/site-packages/marionette_driver/decorators.py", line 26, in _
    return func(*args, **kwargs)
  File "/home/test/web-platform-tests/_venv/lib/python2.7/site-packages/marionette_driver/marionette.py", line 741, in _send_message
    self._handle_error(err)
  File "/home/test/web-platform-tests/_venv/lib/python2.7/site-packages/marionette_driver/marionette.py", line 765, in _handle_error
    raise errors.lookup(error)(message, stacktrace=stacktrace)
NoSuchWindowException: Browsing context has been discarded
stacktrace:
	WebDriverError@chrome://marionette/content/error.js:179:5
	NoSuchWindowError@chrome://marionette/content/error.js:411:5
	assert.that/<@chrome://marionette/content/assert.js:417:13
	assert.open@chrome://marionette/content/assert.js:175:25
	setup@chrome://marionette/content/reftest.js:74:33
	GeckoDriver.prototype.setupReftest@chrome://marionette/content/driver.js:3529:17
	Async*despatch@chrome://marionette/content/server.js:289:40
	async*execute@chrome://marionette/content/server.js:262:16
	async*onPacket/<@chrome://marionette/content/server.js:235:20
	async*onPacket@chrome://marionette/content/server.js:236:9
	_onJSONObjectReady/<@chrome://marionette/content/transport.js:492:20

[...snip...]

SUITE-END | took 5s
Closing logging queue
queue closed
## All results ##

### /css/vendor-imports/mozilla/mozilla-central-reftests/shapes1/float-retry-push-circle.html ###
| Subtest |            Results            | Messages |
|---------|-------------------------------|----------|
|         | **PASS: 1/10, MISSING: 9/10** |          |

## Unstable results ##

Cosmin Sabou [:CosminS]

Comment 16

•

5 years ago

bugherder

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