From e7a6ff9a5147959f4ac25f01767798c1ec3a1cd1 Mon Sep 17 00:00:00 2001 From: Julian Descottes Date: Fri, 22 Jul 2022 20:15:52 +0000 Subject: [PATCH] Bug 1778985 - [devtools] Add `wait` option to `getRuleViewProperty` r=Honza,nchevobbe Update the test helper getRuleViewProperty to support an async version via a `wait` option. When passed, the helper will keep polling until there is a valid ruleviewproperty which matches the arguments. This can avoid race issues when the API is used too early. In this changeset we also start using this API in all tests which either: - used to manually poll getRuleViewProperty - were disabled on linux for getRuleViewProperty issues - are currently intermittent because of getRuleViewProperty Differential Revision: https://phabricator.services.mozilla.com/D152286 --- .../inspector/rules/test/browser_part1.ini | 2 - ..._rules_colorpicker-and-image-tooltip_01.js | 9 +++-- ...wser_rules_colorpicker-swatch-displayed.js | 4 +- ...r_rules_colorpicker-works-with-css-vars.js | 4 +- .../rules/test/browser_rules_cycle-angle.js | 13 +++++-- ...rowser_rules_edit-display-grid-property.js | 4 +- devtools/client/inspector/test/shared-head.js | 39 ++++++++++++------- 7 files changed, 50 insertions(+), 25 deletions(-) diff --git a/devtools/client/inspector/rules/test/browser_part1.ini b/devtools/client/inspector/rules/test/browser_part1.ini index bab725777d59..b4caa3dc4aa6 100644 --- a/devtools/client/inspector/rules/test/browser_part1.ini +++ b/devtools/client/inspector/rules/test/browser_part1.ini @@ -109,10 +109,8 @@ skip-if = (os == 'win' && bits == 32 && debug) # bug 1609313 [browser_rules_cubicbezier-revert-on-ESC.js] [browser_rules_custom.js] [browser_rules_cycle-angle.js] -skip-if = (os == "linux") # Bug 1767684 [browser_rules_cycle-color.js] [browser_rules_edit-display-grid-property.js] -skip-if = (os == "linux") # Bug 1356214 [browser_rules_edit-property-cancel.js] [browser_rules_edit-property-click.js] [browser_rules_edit-property-commit.js] diff --git a/devtools/client/inspector/rules/test/browser_rules_colorpicker-and-image-tooltip_01.js b/devtools/client/inspector/rules/test/browser_rules_colorpicker-and-image-tooltip_01.js index 6ca36a110d84..6108402e00f3 100644 --- a/devtools/client/inspector/rules/test/browser_rules_colorpicker-and-image-tooltip_01.js +++ b/devtools/client/inspector/rules/test/browser_rules_colorpicker-and-image-tooltip_01.js @@ -20,10 +20,11 @@ add_task(async function() { await addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI)); const { view } = await openRuleView(); - // Bug 1767679 - Frequent intermittents on linux. - const property = await waitFor(() => - getRuleViewProperty(view, "body", "background") - ); + // Bug 1767679 - Use { wait: true } to avoid frequent intermittents on linux. + const property = await getRuleViewProperty(view, "body", "background", { + wait: true, + }); + const value = property.valueSpan; const swatch = value.querySelectorAll(".ruleview-colorswatch")[0]; const url = value.querySelector(".theme-link"); diff --git a/devtools/client/inspector/rules/test/browser_rules_colorpicker-swatch-displayed.js b/devtools/client/inspector/rules/test/browser_rules_colorpicker-swatch-displayed.js index d04e8f7e4bec..d87b048e825c 100644 --- a/devtools/client/inspector/rules/test/browser_rules_colorpicker-swatch-displayed.js +++ b/devtools/client/inspector/rules/test/browser_rules_colorpicker-swatch-displayed.js @@ -90,7 +90,9 @@ add_task(async function() { selector ); - const prop = getRuleViewProperty(view, selector, propertyName).valueSpan; + const prop = ( + await getRuleViewProperty(view, selector, propertyName, { wait: true }) + ).valueSpan; const swatches = prop.querySelectorAll(".ruleview-colorswatch"); ok(swatches.length, "Swatches found in the property"); diff --git a/devtools/client/inspector/rules/test/browser_rules_colorpicker-works-with-css-vars.js b/devtools/client/inspector/rules/test/browser_rules_colorpicker-works-with-css-vars.js index 20ca786a104e..c58c935a3364 100644 --- a/devtools/client/inspector/rules/test/browser_rules_colorpicker-works-with-css-vars.js +++ b/devtools/client/inspector/rules/test/browser_rules_colorpicker-works-with-css-vars.js @@ -41,7 +41,9 @@ async function testColorPickerAppearsOnColorSwatchActivation( property, withKeyboard = false ) { - const value = getRuleViewProperty(view, "body", property).valueSpan; + const value = ( + await getRuleViewProperty(view, "body", property, { wait: true }) + ).valueSpan; const swatch = value.querySelector(".ruleview-colorswatch"); const cPicker = view.tooltips.getTooltip("colorPicker"); diff --git a/devtools/client/inspector/rules/test/browser_rules_cycle-angle.js b/devtools/client/inspector/rules/test/browser_rules_cycle-angle.js index 29816fc35fd2..36d9ab3487af 100644 --- a/devtools/client/inspector/rules/test/browser_rules_cycle-angle.js +++ b/devtools/client/inspector/rules/test/browser_rules_cycle-angle.js @@ -26,7 +26,10 @@ add_task(async function() { async function checkAngleCycling(inspector, view) { await selectNode(".turn", inspector); - const container = getRuleViewProperty(view, ".turn", "filter").valueSpan; + + const container = ( + await getRuleViewProperty(view, ".turn", "filter", { wait: true }) + ).valueSpan; const valueNode = container.querySelector(".ruleview-angle"); const win = view.styleWindow; @@ -59,7 +62,9 @@ async function checkAngleCycling(inspector, view) { async function checkAngleCyclingPersist(inspector, view) { await selectNode(".deg", inspector); - let container = getRuleViewProperty(view, ".deg", "filter").valueSpan; + let container = ( + await getRuleViewProperty(view, ".deg", "filter", { wait: true }) + ).valueSpan; let valueNode = container.querySelector(".ruleview-angle"); const win = view.styleWindow; @@ -79,7 +84,9 @@ async function checkAngleCyclingPersist(inspector, view) { // We have to query for the container and the swatch because // they've been re-generated - container = getRuleViewProperty(view, ".deg", "filter").valueSpan; + container = ( + await getRuleViewProperty(view, ".deg", "filter", { wait: true }) + ).valueSpan; valueNode = container.querySelector(".ruleview-angle"); is( valueNode.textContent, diff --git a/devtools/client/inspector/rules/test/browser_rules_edit-display-grid-property.js b/devtools/client/inspector/rules/test/browser_rules_edit-display-grid-property.js index 330204f93639..6c81490c606e 100644 --- a/devtools/client/inspector/rules/test/browser_rules_edit-display-grid-property.js +++ b/devtools/client/inspector/rules/test/browser_rules_edit-display-grid-property.js @@ -29,7 +29,9 @@ add_task(async function() { } = getHighlighterTestHelpers(inspector); await selectNode("#grid", inspector); - const container = getRuleViewProperty(view, "#grid", "display").valueSpan; + const container = ( + await getRuleViewProperty(view, "#grid", "display", { wait: true }) + ).valueSpan; let gridToggle = container.querySelector(".js-toggle-grid-highlighter"); info("Toggling ON the CSS grid highlighter from the rule-view."); diff --git a/devtools/client/inspector/test/shared-head.js b/devtools/client/inspector/test/shared-head.js index 29289066bb74..243c078c0e01 100644 --- a/devtools/client/inspector/test/shared-head.js +++ b/devtools/client/inspector/test/shared-head.js @@ -560,7 +560,7 @@ function getRuleViewRule(view, selectorText, index = 0) { /** * Get references to the name and value span nodes corresponding to a given - * selector and property name in the rule-view + * selector and property name in the rule-view. * * @param {CssRuleView} view * The instance of the rule-view panel @@ -568,25 +568,38 @@ function getRuleViewRule(view, selectorText, index = 0) { * The selector in the rule-view to look for the property in * @param {String} propertyName * The name of the property + * @param {Object=} options + * @param {Boolean=} options.wait + * When true, returns a promise which waits until a valid rule view + * property can be retrieved for the provided selectorText & propertyName. + * Defaults to false. * @return {Object} An object like {nameSpan: DOMNode, valueSpan: DOMNode} */ -function getRuleViewProperty(view, selectorText, propertyName) { - let prop; +function getRuleViewProperty(view, selectorText, propertyName, options = {}) { + if (options.wait) { + return waitFor(() => + _syncGetRuleViewProperty(view, selectorText, propertyName) + ); + } + return _syncGetRuleViewProperty(view, selectorText, propertyName); +} +function _syncGetRuleViewProperty(view, selectorText, propertyName) { const rule = getRuleViewRule(view, selectorText); - if (rule) { - // Look for the propertyName in that rule element - for (const p of rule.querySelectorAll(".ruleview-property")) { - const nameSpan = p.querySelector(".ruleview-propertyname"); - const valueSpan = p.querySelector(".ruleview-propertyvalue"); + if (!rule) { + return null; + } - if (nameSpan.textContent === propertyName) { - prop = { nameSpan: nameSpan, valueSpan: valueSpan }; - break; - } + // Look for the propertyName in that rule element + for (const p of rule.querySelectorAll(".ruleview-property")) { + const nameSpan = p.querySelector(".ruleview-propertyname"); + const valueSpan = p.querySelector(".ruleview-propertyvalue"); + + if (nameSpan.textContent === propertyName) { + return { nameSpan: nameSpan, valueSpan: valueSpan }; } } - return prop; + return null; } /**