Skip to content

Add a Test-Sequence header to the webkitpy <-> WKTR/DRT server protocol #49978

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Add a Test-Sequence header to the webkitpy <-> WKTR/DRT server protocol #49978

wants to merge 1 commit into from

Conversation

gsnedders
Copy link
Member

@gsnedders gsnedders commented Aug 28, 2025 •
edited by webkit-early-warning-system
Loading

b5b0a0d

Add a Test-Sequence header to the webkitpy <-> WKTR/DRT server protocol
https://bugs.webkit.org/show_bug.cgi?id=298004

Reviewed by NOBODY (OOPS!).

This keeps count of the number of run test commands sent on both sides
of the protocol, allowing some basic synchronization.

It is less ideal than setting some abstract UUID (because
incrementing accidentally is possible), but this is landable in a much
lower-impact way that doesn't break the communication protocol,
allowing older WKTR/DRT builds (e.g., from build archives) to still
work with current webkitpy.

We report failures as CRASH, because a driver synchronization issue is
a significant one. Of course, by the point this occurs, it's reporting
a _following_ test as CRASH (and almost certainly the _immediately_
following test), and not the one that caused the synchronization issue
to start with.

Explanation of why this fixes the bug (OOPS!).

b5b0a0d

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows Apple Internal
❌ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ❌ 🛠 wpe ✅ 🛠 win ⏳ 🛠 ios-apple
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2 ✅ 🧪 win-tests ✅ 🛠 mac-apple
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ❌ 🧪 api-wpe ✅ 🛠 vision-apple
❌ 🧪 webkitpy ✅ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ❌ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ❌ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ❌ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ❌ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@gsnedders gsnedders self-assigned this Aug 28, 2025
@gsnedders gsnedders added the Tools / Tests Tools in the Tools directory, build issues, test infrastructure, and bugs in test cases label Aug 28, 2025
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Aug 28, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 8b56f40)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows Apple Internal
❌ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win ✅ 🛠 ios-apple
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2   🧪 win-tests ✅ 🛠 mac-apple
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe ✅ 🛠 vision-apple
❌ 🧪 webkitpy ✅ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Aug 28, 2025
@gsnedders
Copy link
Member Author

The only notable failures this finds are on mac-wk1, fonts/monospace.html (test sequence mismatch; history), and scrollbars/scrollevent-iframe-no-scrolling-wheel.html having the strangest diff (where both old and new look wrong):

--- /Volumes/Data/worker/macOS-Sonoma-Release-WK1-Tests-EWS/build/layout-test-results/scrollbars/scrollevent-iframe-no-scrolling-wheel-expected.txt
+++ /Volumes/Data/worker/macOS-Sonoma-Release-WK1-Tests-EWS/build/layout-test-results/scrollbars/scrollevent-iframe-no-scrolling-wheel-actual.txt
@@ -1,3 +1,3 @@
-mouseScrollByXandYContinuously: Unable to locate target view for current mouse location.Content-Type: text/plain
+mouseScrollByXandYContinuously: Unable to locate target view for current mouse location.Test-Sequence: 495
 Not scrolled by WheelEvent: SUCCESS

For fonts/monospace.html, in the original run we have on worker 7:

18:54:57.241 62808 worker/7 fast/zooming/client-rect-in-fixed-zoomed.html passed
18:54:57.262 62808 worker/7 fast/zooming/client-rects-with-css-and-page-zoom.html passed
18:54:57.277 62808 worker/7 fast/zooming/currentCSSZoom-body-fixed-regression-test.html passed
18:54:57.309 62808 worker/7 fast/zooming/uiscript-zooming.html passed
18:54:57.309 62808 worker/7 finished test group
18:54:57.329 62808 worker/7 fonts/cursive.html passed
18:54:57.408 62808 worker/7 fonts/default.html passed
18:54:57.450 62808 worker/7 fonts/fantasy.html passed
18:54:57.477 62808 worker/7 fonts/font-cache-crash.html passed
18:54:57.582 62808 worker/7 fonts/font-cache-memory-pressure-crash.html passed
18:54:57.584 62808 worker/7 Test sequence mismatch for fonts/monospace.html: expected 1766, got 1765
18:54:57.850 62808 worker/7 Couldn't quit 62944 with SIGTERM, sending SIGKILL.
18:54:57.914 62808 worker/7 killed pid 62944
18:54:57.915 62808 worker/7 killing driver
18:54:57.915 62808 worker/7 fonts/monospace.html failed:
18:54:57.915 62808 worker/7 test sequence mismatch: expected 1766, got 1765
18:54:58.950 62808 worker/7 fonts/sans-serif.html passed
18:54:58.971 62808 worker/7 fonts/serif.html passed
18:54:59.020 62808 worker/7 fonts/unicode-character-font-crash.html passed
18:54:59.113 62808 worker/7 fonts/use-typo-metrics-1.html passed
18:54:59.206 62808 worker/7 fonts/use-typo-metrics-2.html passed
18:54:59.233 62808 worker/7 fonts/valid-standard-font.html passed
18:54:59.234 62808 worker/7 finished test group
18:55:02.067 62808 worker/7 worker/7 http/tests/canvas/webgl/origin-clean-conformance.html output stderr lines:
18:55:02.067 62808 worker/7   2025-08-27 18:55:02.022 DumpRenderTree[63068:417851] *** WARNING: Method convertPointToBase: in class NSView is deprecated on 10.7 and later. It should not be used in new applications.

And on the re-run on worker 17:

19:13:30.986 66864 worker/17 fast/xpath/py-dom-xpath/functions.html passed
19:13:31.037 66864 worker/17 fast/xpath/py-dom-xpath/nodetests.html passed
19:13:31.066 66864 worker/17 fast/xpath/py-dom-xpath/paths.html passed
19:13:31.186 66864 worker/17 fast/xpath/py-dom-xpath/predicates.html passed
19:13:31.186 66864 worker/17 finished test group
19:13:31.222 66864 worker/17 fonts/cursive.html passed
19:13:31.242 66864 worker/17 fonts/default.html passed
19:13:31.270 66864 worker/17 fonts/fantasy.html passed
19:13:31.288 66864 worker/17 fonts/font-cache-crash.html passed
19:13:31.384 66864 worker/17 fonts/font-cache-memory-pressure-crash.html passed
19:13:31.384 66864 worker/17 Test sequence mismatch for fonts/monospace.html: expected 291, got 290
19:13:31.727 66864 worker/17 Couldn't quit 67018 with SIGTERM, sending SIGKILL.
19:13:31.783 66864 worker/17 killed pid 67018
19:13:31.784 66864 worker/17 killing driver
19:13:31.809 66864 worker/17 fonts/monospace.html failed:
19:13:31.809 66864 worker/17 test sequence mismatch: expected 291, got 290
19:13:32.396 66864 worker/17 fonts/sans-serif.html passed
19:13:32.417 66864 worker/17 fonts/serif.html passed
19:13:32.484 66864 worker/17 fonts/unicode-character-font-crash.html passed
19:13:32.549 66864 worker/17 fonts/use-typo-metrics-1.html passed
19:13:32.596 66864 worker/17 fonts/use-typo-metrics-2.html passed
19:13:32.607 66864 worker/17 fonts/valid-standard-font.html passed
19:13:32.607 66864 worker/17 finished test group
19:13:35.238 66864 worker/17 http/tests/contentfiltering/allow-after-redirect.html passed
19:13:35.402 66864 worker/17 http/tests/contentfiltering/block-after-redirect.html passed

This suggests fonts/font-cache-memory-pressure-crash.html needs gardened, at least on mac-wk1.

@gsnedders
Add a Test-Sequence header to the webkitpy <-> WKTR/DRT server protocol
b5b0a0d 
https://bugs.webkit.org/show_bug.cgi?id=298004

Reviewed by NOBODY (OOPS!).

This keeps count of the number of run test commands sent on both sides
of the protocol, allowing some basic synchronization.

It is less ideal than setting some abstract UUID (because
incrementing accidentally is possible), but this is landable in a much
lower-impact way that doesn't break the communication protocol,
allowing older WKTR/DRT builds (e.g., from build archives) to still
work with current webkitpy.

We report failures as CRASH, because a driver synchronization issue is
a significant one. Of course, by the point this occurs, it's reporting
a _following_ test as CRASH (and almost certainly the _immediately_
following test), and not the one that caused the synchronization issue
to start with.

Explanation of why this fixes the bug (OOPS!).
@gsnedders gsnedders removed the merging-blocked Applied to prevent a change from being merged label Sep 16, 2025
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Sep 16, 2025 •
edited
Loading

EWS run on current version of this PR (hash b5b0a0d)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows Apple Internal
❌ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ❌ 🛠 wpe ✅ 🛠 win ⏳ 🛠 ios-apple
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2 ✅ 🧪 win-tests ✅ 🛠 mac-apple
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ❌ 🧪 api-wpe ✅ 🛠 vision-apple
❌ 🧪 webkitpy ✅ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ❌ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ❌ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ❌ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ❌ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexgcastro alexgcastro Awaiting requested review from alexgcastro alexgcastro will be requested when the pull request is marked ready for review alexgcastro is a code owner

@carlosgcampos carlosgcampos Awaiting requested review from carlosgcampos carlosgcampos will be requested when the pull request is marked ready for review carlosgcampos is a code owner

@nikolaszimmermann nikolaszimmermann Awaiting requested review from nikolaszimmermann nikolaszimmermann will be requested when the pull request is marked ready for review nikolaszimmermann is a code owner

@magomez magomez Awaiting requested review from magomez magomez will be requested when the pull request is marked ready for review magomez is a code owner

@JonWBedard JonWBedard Awaiting requested review from JonWBedard JonWBedard will be requested when the pull request is marked ready for review JonWBedard is a code owner

Assignees

@gsnedders gsnedders

Labels
merging-blocked Applied to prevent a change from being merged Tools / Tests Tools in the Tools directory, build issues, test infrastructure, and bugs in test cases
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gsnedders @webkit-early-warning-system @webkit-ews-buildbot