Closed Bug 1288165 Opened 8 years ago Closed 8 years ago

browser_parsable_css.js should fail if entries in the whitelist of the test are unused

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 50
Tracking Status
firefox50 --- fixed

People

(Reporter: jaws, Assigned: jaws)

Details

Attachments

(1 file)

      No description provided.
Comment on attachment 8772910 [details]
Bug 1288165 - browser_parsable_css.js should fail if entries in the whitelist of the test are unused.

https://reviewboard.mozilla.org/r/65602/#review62594

::: browser/base/content/test/general/browser_parsable_css.js:15
(Diff revision 1)
>   * ALL need to match an error in order for that error not to cause a test
>   * failure. */
> -const kWhitelist = [
> +let whitelist = [
>    // CodeMirror is imported as-is, see bug 1004423.
> -  {sourceName: /codemirror\.css$/i},
> +  {sourceName: /codemirror\.css$/i,
> +   devtoolsOnly: true},

Can we just use 1 boolean? Something like `isFromDevTools`, and set it to true/false depending on whether it is/isn't devtools?

Then later on...

::: browser/base/content/test/general/browser_parsable_css.js:250
(Diff revision 1)
>    is(errors.length, 0, "All the styles (" + allPromises.length + ") loaded without errors.");
>  
> +  // Confirm that all whitelist rules have been used.
> +  for (let item of whitelist) {
> +    if (!item.used &&
> +        ((isDevtools && item.devtoolsOnly) ||

... you could just check `isDevtools == item.isFromDevtools` ?
Attachment #8772910 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8772910 [details]
Bug 1288165 - browser_parsable_css.js should fail if entries in the whitelist of the test are unused.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65602/diff/1-2/
Attachment #8772910 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8772910 [details]
Bug 1288165 - browser_parsable_css.js should fail if entries in the whitelist of the test are unused.

https://reviewboard.mozilla.org/r/65602/#review62818

LGTM assuming green on try.
Attachment #8772910 - Flags: review?(gijskruitbosch+bugs) → review+
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4a8156e12677
browser_parsable_css.js should fail if entries in the whitelist of the test are unused. r=Gijs
https://hg.mozilla.org/mozilla-central/rev/4a8156e12677
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: