Bug 202649

Summary: Web Inspector: inspector/layers/layers-for-node.html and inspector/timeline/line-column.html are flaky
Product: WebKit Reporter: Yury Semikhatsky <yurys>
Component: Web InspectorAssignee: Yury Semikhatsky <yurys>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, hi, inspector-bugzilla-changes, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
See Also: https://bugs.webkit.org/show_bug.cgi?id=137131
https://bugs.webkit.org/show_bug.cgi?id=203171
Attachments:
Description Flags
Patch
none
Patch none

Yury Semikhatsky
Reported 2019-10-07 13:10:49 PDT
The two tests became very flaky after replacing Timer based queue with RunLoop one in https://trac.webkit.org/changeset/250655/webkit. The change seems to have revealed existing flaw in the tests as it essentially modified the timing of the event dispatches. inspector/layers/layers-for-node.html inspector/timeline/line-column.html History: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=inspector%2Flayers%2Flayers-for-node.html%20inspector%2Ftimeline%2Fline-column.html Diff: https://build.webkit.org/results/Apple%20Mojave%20Release%20WK1%20(Tests)/r250672%20(8069)/inspector/layers/layers-for-node-diff.txt https://build.webkit.org/results/Apple%20Mojave%20Release%20WK1%20(Tests)/r250672%20(8069)/inspector/timeline/line-column-diff.txt
Attachments
Patch (7.19 KB, patch)
2019-10-07 13:18 PDT, Yury Semikhatsky 		no flags
DetailsFormatted DiffDiff
Patch (7.08 KB, patch)
2019-10-07 14:13 PDT, Yury Semikhatsky 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Yury Semikhatsky
Comment 1 2019-10-07 13:18:52 PDT
Created attachment 380350 [details] Patch
Devin Rousso
Comment 2 2019-10-07 13:53:25 PDT
Comment on attachment 380350 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=380350&action=review rs=me > LayoutTests/inspector/layers/layers-for-node.html:25 > + layersChanged = InspectorProtocol.awaitEvent({event: "LayerTree.layerTreeDidChange"}) Frankly, given that `enableLayerTreeAgent` is called synchronously at the start of the test, perhaps we should just inline this at the top of `test` as a sort of "make sure we get at least one of these before continuing with the rest of the test". > LayoutTests/inspector/layers/layers-for-node.html:36 > + layersChanged.then(() => step({ Style: if the body of the arrow function is more than one line (or the value isn't intended as the return value), we add the `{ ... }` to the arrow function. ```js layersChanged.then(() => { step({ ... }); }); ```
Yury Semikhatsky
Comment 3 2019-10-07 14:13:40 PDT
Created attachment 380361 [details] Patch
Yury Semikhatsky
Comment 4 2019-10-07 14:14:24 PDT
Comment on attachment 380350 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=380350&action=review >> LayoutTests/inspector/layers/layers-for-node.html:25 >> + layersChanged = InspectorProtocol.awaitEvent({event: "LayerTree.layerTreeDidChange"}) > > Frankly, given that `enableLayerTreeAgent` is called synchronously at the start of the test, perhaps we should just inline this at the top of `test` as a sort of "make sure we get at least one of these before continuing with the rest of the test". Done. >> LayoutTests/inspector/layers/layers-for-node.html:36 >> + layersChanged.then(() => step({ > > Style: if the body of the arrow function is more than one line (or the value isn't intended as the return value), we add the `{ ... }` to the arrow function. > ```js > layersChanged.then(() => { > step({ > ... > }); > }); > ``` Moved this logic out of the method.
Devin Rousso
Comment 5 2019-10-08 15:01:32 PDT
Comment on attachment 380361 [details] Patch rs=me
WebKit Commit Bot
Comment 6 2019-10-08 16:03:08 PDT
Comment on attachment 380361 [details] Patch Clearing flags on attachment: 380361 Committed r250872: <https://trac.webkit.org/changeset/250872>
WebKit Commit Bot
Comment 7 2019-10-08 16:03:10 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 8 2019-10-08 16:04:21 PDT
<rdar://problem/56096739>
Note You need to log in before you can comment on or make changes to this bug.