Bug 202649 - Web Inspector: inspector/layers/layers-for-node.html and inspector/timeline/line-column.html are flaky
Summary: Web Inspector: inspector/layers/layers-for-node.html and inspector/timeline/...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Yury Semikhatsky
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-10-07 13:10 PDT by Yury Semikhatsky
Modified: 2019-10-08 16:04 PDT (History)
4 users (show)

See Also:


Attachments
Patch (7.19 KB, patch)
2019-10-07 13:18 PDT, Yury Semikhatsky
no flags Details | Formatted Diff | Diff
Patch (7.08 KB, patch)
2019-10-07 14:13 PDT, Yury Semikhatsky
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yury Semikhatsky 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
Comment 1 Yury Semikhatsky 2019-10-07 13:18:52 PDT
Created attachment 380350 [details]
Patch
Comment 2 Devin Rousso 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({
            ...
        });
    });
```
Comment 3 Yury Semikhatsky 2019-10-07 14:13:40 PDT
Created attachment 380361 [details]
Patch
Comment 4 Yury Semikhatsky 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.
Comment 5 Devin Rousso 2019-10-08 15:01:32 PDT
Comment on attachment 380361 [details]
Patch

rs=me
Comment 6 WebKit Commit Bot 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>
Comment 7 WebKit Commit Bot 2019-10-08 16:03:10 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2019-10-08 16:04:21 PDT
<rdar://problem/56096739>