Bug 260546

Summary:	REGRESSION:267076@main [ macOS ] [ Debug ] imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-details-element/toggleEvent.html is a flaky test failure
Product:	WebKit	Reporter:	Dawn Morningstar <Morningstar>
Component:	DOM	Assignee:	Dawn Morningstar <Morningstar>
Status:	NEW ---
Severity:	Normal	CC:	mike, webkit-bot-watchers-bugzilla, webkit-bug-importer
Priority:	P2	Keywords:	InRadar
Version:	WebKit Nightly Build
Hardware:	Unspecified
OS:	Unspecified

Description Dawn Morningstar 2023-08-22 14:22:38 PDT

imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-details-element/toggleEvent.html 
is a flaky text failure after 267076@main where toggleEvent was enabled and the failure expectation was removed.
This failure is only on debug, and is nearly constant, but the flakes make rebaslining not the solution.

HISTORY:
https://results.webkit.org/?suite=layout-tests&test=imported%2Fw3c%2Fweb-platform-tests%2Fhtml%2Fsemantics%2Finteractive-elements%2Fthe-details-element%2FtoggleEvent.html&style=debug

DIFF:
--- /Volumes/Data/worker/Apple-Ventura-Debug-WK2-GPUProcess-Tests/build/layout-test-results/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-details-element/toggleEvent-expected.txt
+++ /Volumes/Data/worker/Apple-Ventura-Debug-WK2-GPUProcess-Tests/build/layout-test-results/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-details-element/toggleEvent-actual.txt
@@ -3,7 +3,7 @@
 PASS Removing open from 'details' should fire a toggle event at the 'details' element, with 'oldState: open' and 'newState: closed'
 PASS Adding open to 'details' (display:none) should fire a toggle event at the 'details' element, with 'oldState: closed' and 'newState: open'
 PASS Adding open to 'details' (no children) should fire a toggle event at the 'details' element, with 'oldState: closed' and 'newState: open'
-PASS Calling open twice on 'details' fires only one toggle event, with 'oldState: closed' and 'newState: open'
+FAIL Calling open twice on 'details' fires only one toggle event, with 'oldState: closed' and 'newState: open' assert_true: expected true got false
 PASS Calling setAttribute('open', '') from 'details' should fire a toggle event at the 'details' element, with 'oldState: closed' and 'newState: open'
 PASS Calling removeAttribute('open') from 'details' should fire a toggle event at the 'details' element, with 'oldState: open' and 'newState: closed'
 PASS Setting open=true to opened 'details' element should not fire a toggle event at the 'details' element

DIFF-URL:
https://build.webkit.org/results/Apple-Ventura-Debug-WK2-GPUProcess-Tests/267082@main%20(3372)/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-details-element/toggleEvent-diff.txt 

This is reproducible at TOT on a debug spade with: rwt --root [buildtoTest] imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-details-element/toggleEvent.html --iterations 100

Comment 1 Radar WebKit Bug Importer 2023-08-22 14:25:25 PDT

<rdar://problem/114281621>

Comment 2 Dawn Morningstar 2023-08-22 14:27:01 PDT

Asking Tim how to proceed since Michael is in a much different time zone.

Comment 3 sideshowbarker 2023-08-22 15:41:31 PDT

Yeah it had been marked [ Pass Failure ] in LayoutTests/TestExpectations but I removed that in my commit — see https://github.com/WebKit/WebKit/commit/74637c383d415af9b4e4ca2ce7fbb3268e6ead85#diff-0b438e462819a7b0d06250189152c7bcfb07b7c39acd73938bdfccc7b25b8058

In hindsight, I probably shouldn’t have done that — because that test is flaky locally as well — specifically, only one particular subtest.

Should I do another PR which adds it back to LayoutTests/TestExpectations as [ Pass Failure ]?

Comment 4 Dawn Morningstar 2023-08-22 15:44:23 PDT

(In reply to Michael[tm] Smith from comment #3)
> Yeah it had been marked [ Pass Failure ] in LayoutTests/TestExpectations but
> I removed that in my commit — see
> https://github.com/WebKit/WebKit/commit/
> 74637c383d415af9b4e4ca2ce7fbb3268e6ead85#diff-
> 0b438e462819a7b0d06250189152c7bcfb07b7c39acd73938bdfccc7b25b8058
> 
> In hindsight, I probably shouldn’t have done that — because that test is
> flaky locally as well — specifically, only one particular subtest.
> 
> Should I do another PR which adds it back to LayoutTests/TestExpectations as
> [ Pass Failure ]?

Michael, No worries! I noticed the expectation was removed, I can handle adding it back, but I was wondering if I should only add it for Debug since it's now passing elsewhere.

Comment 5 sideshowbarker 2023-08-22 15:49:12 PDT

(In reply to Dawn Flores from comment #4)
> Michael, No worries! I noticed the expectation was removed, I can handle
> adding it back, but I was wondering if I should only add it for Debug since
> it's now passing elsewhere.

That part maybe we need for Tim to weigh in on. In my local testing for the patch, I was only building Debug builds — I never tested with any Release builds. So I can’t say that I’ve ever seen the flakiness in a local Release build (though I definitely did with my local Debug builds).

Comment 6 Tim Nguyen (:ntim) 2023-08-23 09:14:46 PDT

Dawn, can we mark it as [ Pass Failure ] on [ Debug ] only for now?

Comment 7 Dawn Morningstar 2023-08-25 14:00:30 PDT

Pull request: https://github.com/WebKit/WebKit/pull/17081

Comment 8 EWS 2023-08-25 14:05:26 PDT

Test gardening commit 267298@main (8df3f2fc4f5d): <https://commits.webkit.org/267298@main>

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