1386384 - Stylo: Regenerate DevTools CSS test fixtures when Stylo is default

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P5)

Description

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

DevTools saves test fixture that records exact CSS error messages.

Stylo produces slightly different data for these, so they aren't an exact match.

I think the simplest path is to store a separate copy of the data for use with Stylo.

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Ah, it turns out the real tests that use this fixture data don't currently run on Mozilla CI anyway, so the only thing being tested is that the fixture data is unchanged.

For now, I'll skip the fixture check test (devtools/client/webconsole/new-console-output/test/fixtures/stub-generators/browser_webconsole_check_stubs_css_message.js).

Once Stylo is the default, we can regenerate the fixtures and enable the test again.

Comment 2

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Bug 1386384 - Update DevTools CSS stubs for Stylo.

Two specific changes have occurred with Stylo:

* `lineText` is no longer set because it caused performance regressions to
  compute it and it is currently unused by DevTools.
* `columnNumber` is computed differently with Stylo.  It's currently the
  beginning of property, instead of the end.  Bug 1378861 is filed for future
  improvements to this info.

Review commit: https://reviewboard.mozilla.org/r/176954/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/176954/

Comment 3

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=44e491410f1024089c522ed4633a9275a2d25d8c

Comment 4

[Brian Grinstead [:bgrins]]

Comment on attachment 8905167 [details]
Bug 1386384 - Update DevTools CSS stubs for Stylo.

https://reviewboard.mozilla.org/r/176954/#review181960

Stylo is riding the train to release, right?  So we don't need to special case this test for beta/release?

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8905167 [details]
Bug 1386384 - Update DevTools CSS stubs for Stylo.

https://reviewboard.mozilla.org/r/176954/#review181960

Correct!

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3adbca67c67a
Update DevTools CSS stubs for Stylo. r=bgrins

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out for dt failures like https://treeherder.mozilla.org/logviewer.html#?job_id=129022789&repo=autoland

Seems exclusive to the stylo-disabled variants.

https://hg.mozilla.org/integration/autoland/rev/60a28a7012ed

Comment 8

[Brian Grinstead [:bgrins]]

I guess we should skip the test in non stylo builds now

Comment 9

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Ah, I guess I should have seen that coming... :S

Comment 10

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8905167 [details]
Bug 1386384 - Update DevTools CSS stubs for Stylo.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/176954/diff/1-2/

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/08f5c5e4f3b1
Update DevTools CSS stubs for Stylo. r=bgrins

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/08f5c5e4f3b1