Bug 285778 – shouldBe in js-test-pre.js is far too noisy when successful

• Component: New Bugs
• Status: RESOLVED INVALID

[Jonathan Bedard]

When two values are equal in the `shouldBe()` function, we output the value of the matched elements. That's extremely noisy when those values are pixel positions on a page determined by how the OS renders elements. We should be able to leverage the utilities of js-test-pre.js without enshrining specific pixel positions into layout test expectations.

[Radar WebKit Bug Importer]

<rdar://problem/142712323>

[Jonathan Bedard]

https://bugs.webkit.org/show_bug.cgi?id=285778 is the bug which inspired me to take a look at this.

[Alexey Proskuryakov]

I think that this is just a test improperly using shouldBe(), and there is no change to be made here.

[Sam Sneddon [:gsnedders]]

(In reply to Jonathan Bedard from comment #2)
> https://bugs.webkit.org/show_bug.cgi?id=285778 is the bug which inspired me
> to take a look at this.

This should, I believe, be https://bugs.webkit.org/show_bug.cgi?id=280424, similar to the See Also.

(In reply to Jonathan Bedard from comment #0)
> When two values are equal in the `shouldBe()` function, we output the value
> of the matched elements. That's extremely noisy when those values are pixel
> positions on a page determined by how the OS renders elements. We should be
> able to leverage the utilities of js-test-pre.js without enshrining specific
> pixel positions into layout test expectations.

In general, this isn't too bad, provided there's no content which depends on how the OS renders content. We used to have guidelines in the form of https://trac.webkit.org/wiki/Writing%20Layout%20Tests%20for%20DumpRenderTree#Howtowriteportablepixeltests for pixel tests, and these largely apply here too.

I'd need to check, but looking at https://github.com/WebKit/WebKit/blob/21753b6e53c734d6b8969a631e2429ac41c3e219/LayoutTests/fast/table/col-and-colgroup-offsets.html makes it seem very reliant on font rendering, and font height especially. I wonder if even `table { font: 16px/1 Ahem }` would be enough to get it rendering the same cross platform, because I don't think there's anything else OS-dependent?

That said, offsetTop/offsetHeight where the positioned parent is the root always seems a bit risky — given a small change (even the description being filled in!) can change everything.

We also have very few tests with platform specific expectations doing this, which suggests it isn't a particular problem:

```
LayoutTests % rg '(PASS|FAIL).*offset(Height|Top|Width|Left)' platform --glob '*-expected.txt' --glob '!imported' --files-with-matches | sort -t/ -k3 -k1
platform/gtk/fast/forms/textarea-metrics-expected.txt
platform/ios/fast/shadow-dom/trusted-event-scoped-flags-expected.txt
platform/wpe/fast/sub-pixel/inline-block-with-padding-expected.txt
platform/glib/fast/table/col-and-colgroup-offsets-expected.txt
platform/ios/fast/table/col-and-colgroup-offsets-expected.txt
platform/mac/fast/table/col-and-colgroup-offsets-expected.txt
platform/wpe/fast/text/font-interstitial-invisible-width-while-loading-expected.txt
platform/ios/ios/fast/flexbox/vertical-box-form-controls-expected.txt
```

[Alexey Proskuryakov]

The point is that tests should be putting the expected expression in quote marks when evaluating it causes this problem. There is no other acceptable solution - we don't have macros in JavaScript, and removing the expectation from output is not good for analyzing results (we wouldn't be able to tell how much off it is without the expected value printed in the diff).