285778 2025-01-10 15:28:11 -0800 shouldBe in js-test-pre.js is far too noisy when successful 2025-01-13 10:21:04 -0800 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED INVALID https://bugs.webkit.org/show_bug.cgi?id=280424 InRadar P2 Normal --- 1 jbedard webkit-unassigned ap webkit-bug-importer oldest_to_newest 2086477 0 jbedard 2025-01-10 15:28:11 -0800 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. 2086479 1 webkit-bug-importer 2025-01-10 15:28:33 -0800 <rdar://problem/142712323> 2086480 2 jbedard 2025-01-10 15:28:57 -0800 https://bugs.webkit.org/show_bug.cgi?id=285778 is the bug which inspired me to take a look at this. 2086759 3 ap 2025-01-13 08:15:10 -0800 I think that this is just a test improperly using shouldBe(), and there is no change to be made here. 2086800 4 gsnedders 2025-01-13 10:10:48 -0800 (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 ``` 2086804 5 ap 2025-01-13 10:21:04 -0800 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).