1683153 - Refactor CSS Grid Highlighter tests to use generic highlighter test helpers

Status: RESOLVED FIXED

(DevTools :: Inspector, task, P3)

Description

[Razvan Caliman [:rcaliman]]

Follow up for Bug 1572652 where we refactored CSS Grid highlighters to use generic highlighter events ("highlighter-shown", "highlighter-hidden"), but did not update all tests to use the generic test helpers introduced in https://phabricator.services.mozilla.com/D85858.

For uniformity, the tests in https://searchfox.org/mozilla-central/source/devtools/client/inspector/grids/test should be migrated to use the new helpers and drop the custom events ("grid-highlighter-shown", "grid-highlighter-hidden"), as well as the shims for getting the parentGridHighlighters map.

Example snippet:

const HIGHLIGHTER_TYPE = inspector.highlighters.TYPES.GRID;
const { waitForHighlighterTypeShown, waitForHighlighterTypeHidden } = getHighlighterTestHelpers(inspector);

const onHighlighterShown = waitForHighlighterTypeShown(HIGHLIGHTER_TYPE)
// do action to show highlighter (ex: click toggle), then wait for the promise to resolve
const { nodeFront, highlighter } = await onHighlighterShown();

const onHighlighterHidden = waitForHighlighterTypeHidden(HIGHLIGHTER_TYPE)
// do action to hide highlighter, then wait for the promise to resolve
const { nodeFront, highlighter } = await onHighlighterHidden();

Comment 1

[Chris Peterson [:cpeterson]]

Tracking dt-fission-m3-reserve bugs for Fission MVP.

Comment 2

[Chris Peterson [:cpeterson]]

Moving "dt-fission-m3-reserve" bugs to "dt-fission-future" because they don't block Fission MVP.

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1683153 - [devtools] Remove HighlightersOverlay.parentGridHighlighters. r=#devtools.

Migrate tests that were using this to actually check the rendered highlighters
in the DOM.
We add a "isParent" option to the grid highlighters so we can set it as an
attribute in the DOM and run some assertions in tests.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ea85a86cf6e1
[devtools] Remove HighlightersOverlay.parentGridHighlighters. r=devtools-reviewers,ochameau.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ea981fe545ac
[devtools] Remove HighlightersOverlay.parentGridHighlighters. r=devtools-reviewers,ochameau.

Comment 6

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/ea981fe545ac

Comment 7

[Acasandrei Beatrice (needinfo me)]

(In reply to Pulsebot from comment #4)

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ea85a86cf6e1
[devtools] Remove HighlightersOverlay.parentGridHighlighters.
r=devtools-reviewers,ochameau.

Perfherder has detected a browsertime performance change from push ea85a86cf6e194196dbce43c1369a4c053787da4.

If you have any questions, please reach out to a performance sheriff. Alternatively, you can find help on Slack by joining #perf-help, and on Matrix you can find help by joining #perftest.

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
4%	speedometer3 TodoMVC-WebComponents/Adding100Items/Sync	android-hw-a55-14-0-aarch64-shippable	webrender	26.50 -> 25.57
3%	speedometer3 TodoMVC-WebComponents/CompletingAllItems/Sync	android-hw-a55-14-0-aarch64-shippable	webrender	11.31 -> 10.91
3%	speedometer3 TodoMVC-WebComponents/total	android-hw-a55-14-0-aarch64-shippable	webrender	77.29 -> 75.04

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a performance sheriff to do that for you.

You can run all of these tests on try with ./mach try perf --alert 44783

The following documentation link provides more information about this command.

Comment 8

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Acasandrei Beatrice (needinfo me) from comment #7)

(In reply to Pulsebot from comment #4)

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ea85a86cf6e1
[devtools] Remove HighlightersOverlay.parentGridHighlighters.
r=devtools-reviewers,ochameau.

it seems very unlikely that this patch add an impact on those tests, especially on this platform: my change would only impact page for which devtools was opened, and for android that means remote debugging, which is very unlikely.
Looking at the chart I couldn't see which patch is responsible for this improvement though

Comment 9

[Acasandrei Beatrice (needinfo me)]

Thanks for the quick reply, it looks like this improvement might also come from 13e8841536d7e704ea379dbf622cecebfc7f859a Bug 1841946. I've marked my previous comment as obsolete.