Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LayoutTests] Rebaseline text tests with pixel refs #28071

Conversation

JonWBedard
Copy link
Member

@JonWBedard JonWBedard commented May 2, 2024

67855f6

[LayoutTests] Rebaseline text tests with pixel refs
rdar://127396069
https://bugs.webkit.org/show_bug.cgi?id=273595

Reviewed by Sam Sneddon.

There are a number of text based tests which have pixel refs. Since these tests explicitly
dumpText, they cannot also be pixel tests.

* LayoutTests/TestExpectations:
* LayoutTests/imported/w3c/web-platform-tests/css/css-break/relpos-inline-hit-testing-expected.html: Removed.
* LayoutTests/imported/w3c/web-platform-tests/css/css-break/relpos-inline-hit-testing-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/intrinsic-size/col-wrap-004-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/intrinsic-size/col-wrap-004-expected.xht: Removed.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/intrinsic-size/col-wrap-005-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/intrinsic-size/col-wrap-005-expected.xht: Removed.
* LayoutTests/imported/w3c/web-platform-tests/css/css-tables/colspan-001-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-tables/colspan-001-expected.xht: Removed.
* LayoutTests/imported/w3c/web-platform-tests/css/css-tables/colspan-002-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-tables/colspan-002-expected.xht: Removed.
* LayoutTests/imported/w3c/web-platform-tests/css/css-tables/colspan-003-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-tables/colspan-003-expected.xht: Removed.
* LayoutTests/imported/w3c/web-platform-tests/css/css-tables/percent-height-overflow-auto-in-restricted-block-size-cell-expected.html: Removed.
* LayoutTests/imported/w3c/web-platform-tests/css/css-tables/percent-height-overflow-auto-in-restricted-block-size-cell-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/mathml/relations/css-styling/width-height-004-expected.html: Removed.
* LayoutTests/imported/w3c/web-platform-tests/mathml/relations/css-styling/width-height-004-expected.txt: Added.
* LayoutTests/platform/ios/TestExpectations:
* LayoutTests/platform/mac/TestExpectations:

Canonical link: https://commits.webkit.org/278402@main

b07c0a1

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ§ͺ wpe-wk2
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ§ͺ api-wpe
βœ… πŸ§ͺ ios-wk2-wpt βœ… πŸ§ͺ mac-wk1 βœ… πŸ›  wpe-skia
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk2 βœ… πŸ›  gtk
βœ… πŸ›  tv ❌ πŸ§ͺ mac-AS-debug-wk2 βœ… πŸ§ͺ gtk-wk2
βœ… πŸ›  tv-sim βœ… πŸ§ͺ mac-wk2-stress βœ… πŸ§ͺ api-gtk
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch
βœ… πŸ›  watch-sim

@JonWBedard JonWBedard self-assigned this May 2, 2024
@JonWBedard JonWBedard added the New Bugs Unclassified bugs are placed in this component until the correct component can be determined. label May 2, 2024
@@ -14,7 +14,7 @@

// Setup for WebKit JavaScript tests
if (self.testRunner) {
testRunner.dumpAsText();
testRunner.dumpAsText(testRunner.shouldDumpPixels());
Copy link
Contributor

@Ahmad-S792 Ahmad-S792 May 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for my knowledge asking - LayoutTest use this but WPT in imported folder use LayoutTests/imported/w3c/web-platform-tests/resources/testharnessreport.js?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the WPT tests use this too when run under run-webkit-tests, it's how WPT tests know how to interact with WebKitTestRunner.

@ryanhaddad ryanhaddad requested a review from gsnedders May 3, 2024 00:03
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 3, 2024
@gsnedders
Copy link
Contributor

So it turns out that upstream WPT doesn't lint for this: https://github.com/web-platform-tests/wpt/blob/c5ced76d2189c00405d2c153f406fdcd8f91781d/tools/lint/lint.py#L451-L469

This is another thing we should fix, and just fix the tests.

@JonWBedard
Copy link
Member Author

Clearly, this doesn't work, testRunner.shouldDumpPixels() isn't a thing. I'll be working on a solution.

@gsnedders
Copy link
Contributor

There's a number of moving parts here, and a number of places where we could fix this.

@JonWBedard JonWBedard removed the merging-blocked Applied to prevent a change from being merged label May 3, 2024
@JonWBedard JonWBedard force-pushed the eng/run-webkit-tests-testharnessreport-js-breaks-ref-tests branch from 05e5de1 to b07c0a1 Compare May 3, 2024 20:19
@JonWBedard JonWBedard changed the title [run-webkit-tests] testharnessreport.js breaks ref tests [LayoutTests] Rebaseline text tests with pixel refs May 3, 2024
Copy link
Contributor

@gsnedders gsnedders left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems fine; we should have a longer-term solution to avoid these being added to WPT in the first place, and make sure we haven't missed anything from this PR.

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 4, 2024
@JonWBedard JonWBedard added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels May 6, 2024
rdar://127396069
https://bugs.webkit.org/show_bug.cgi?id=273595

Reviewed by Sam Sneddon.

There are a number of text based tests which have pixel refs. Since these tests explicitly
dumpText, they cannot also be pixel tests.

* LayoutTests/TestExpectations:
* LayoutTests/imported/w3c/web-platform-tests/css/css-break/relpos-inline-hit-testing-expected.html: Removed.
* LayoutTests/imported/w3c/web-platform-tests/css/css-break/relpos-inline-hit-testing-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/intrinsic-size/col-wrap-004-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/intrinsic-size/col-wrap-004-expected.xht: Removed.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/intrinsic-size/col-wrap-005-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/intrinsic-size/col-wrap-005-expected.xht: Removed.
* LayoutTests/imported/w3c/web-platform-tests/css/css-tables/colspan-001-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-tables/colspan-001-expected.xht: Removed.
* LayoutTests/imported/w3c/web-platform-tests/css/css-tables/colspan-002-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-tables/colspan-002-expected.xht: Removed.
* LayoutTests/imported/w3c/web-platform-tests/css/css-tables/colspan-003-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-tables/colspan-003-expected.xht: Removed.
* LayoutTests/imported/w3c/web-platform-tests/css/css-tables/percent-height-overflow-auto-in-restricted-block-size-cell-expected.html: Removed.
* LayoutTests/imported/w3c/web-platform-tests/css/css-tables/percent-height-overflow-auto-in-restricted-block-size-cell-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/mathml/relations/css-styling/width-height-004-expected.html: Removed.
* LayoutTests/imported/w3c/web-platform-tests/mathml/relations/css-styling/width-height-004-expected.txt: Added.
* LayoutTests/platform/ios/TestExpectations:
* LayoutTests/platform/mac/TestExpectations:

Canonical link: https://commits.webkit.org/278402@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/run-webkit-tests-testharnessreport-js-breaks-ref-tests branch from b07c0a1 to 67855f6 Compare May 6, 2024 14:04
@webkit-commit-queue
Copy link
Collaborator

Committed 278402@main (67855f6): https://commits.webkit.org/278402@main

Reviewed commits have been landed. Closing PR #28071 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 67855f6 into WebKit:main May 6, 2024
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label May 6, 2024
@JonWBedard JonWBedard deleted the eng/run-webkit-tests-testharnessreport-js-breaks-ref-tests branch May 6, 2024 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Bugs Unclassified bugs are placed in this component until the correct component can be determined.
Projects
None yet
6 participants