969972 - browser_styleeditor_autocomplete.js will be permaorange when 29 merges to beta on March 17th

Status: RESOLVED FIXED

(DevTools :: Style Editor, defect)

Description

[Phil Ringnalda (:philor)]

Looks like the test thinks it knows how many things will be returned by autocomplete, when that depends on whether layout.css.font-features.enabled is true or not, and whether that's true or not depends on whether the RELEASE_BUILD define is set, and when we merge to beta and change the version number from 29.0a2 to 29.0, RELEASE BUILD gets set and you get permaorange like https://tbpl.mozilla.org/?tree=Try&rev=bc445a9dbee6&jobname=browser-chrome.

Even without RELEASE_BUILD troubles (and that'll be true of pretty much every CSS prop that gets added, it'll be ifdef'ed off at first), expecting CSS hackers to know that along with needing to make their tests pass, there's a single browser-chrome test that thinks it knows how many exist and needs to be told that another exists seems pretty fragile.

Comment 1

[Girish Sharma [:Optimizer]]

The test tests that the number of autocomplete entries are not incorrect. How else should I test that ?

Comment 2

[Girish Sharma [:Optimizer]]

(Also, the same is true for various other autocomplete tests in markup view and rule view)

Comment 3

[Panos Astithas (he/him) [:past] (please ni?)]

We used to have the same issue in debugger and console tests. It shouldn't test for the actual number of entries, since it's not supposed to verify the correctness of low-level APIs (we should, and hopefully do, have other tests for that). It should rather make sure that some well-known entries are present, or perhaps that the number of entries displayed on the popup matches the number of entries returned by the backend.

Comment 4

[Justin Wood (:Callek)]

saw this called out in IRC by :philor about how it will turn perma-orange on beta on March 17 and thus will get turned off, and thus will not be able to catch regressions in any beta-uplifts related to this code.

Flag needinfo on the author of the test and everyone who reviewed any changes to this file. I suggest we get this fixed as a priority, and a good way to fix logic was presented in Oct as well

Comment 5

[Justin Wood (:Callek)]

* s/In October/2 weeks ago/  (bah at misreading number-only-date)

Comment 6

[Girish Sharma [:Optimizer]]

Attachment: fix

Now using suggestions numbers by querying platform API instead if hard coded numbers.
try push : https://tbpl.mozilla.org/?tree=Try&rev=67292faf5770

Philor, can you push this through a similar try as in comment 0 to verify ?

Comment 7

[Phil Ringnalda (:philor)]

https://tbpl.mozilla.org/?tree=Try&rev=c14fafb16fa4

Comment 8

[Girish Sharma [:Optimizer]]

The above try is green too.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/28a0756d4d31

This is a test-only change, so it can land on Aurora without approval. Please put [checkin-needed-aurora] on the whiteboard after this gets merged to m-c and I'll take care of it :)

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/28a0756d4d31

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d78ce87f31a6