1446954 - Stop running tests on automation with stylo-disabled and stylo-vs-gecko tasks

Status: RESOLVED FIXED

(Firefox Build System :: Task Configuration, task)

Comment 1

[Xidorn Quan [:xidorn] UTC+10]

This should not be a style system bug.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1446954: Remove support for running tests on automation without stylo configurations.

You can still run them on a --disable-stylo build, as long as that works
(presumably not for long).

I think I haven't missed anything, but please double-check.

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

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1446954: Remove ImageCacheKey styloToGecko hack.

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

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1446954: Remove support for styloVsGecko reftests.

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

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1446954: Cleanup !stylo and styloVsGecko test expectations.

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

Comment 6

[Nathan Froyd [:froydnj]]

Comment on attachment 8960206 [details]
Bug 1446954: Remove support for running tests on automation without stylo configurations.

https://reviewboard.mozilla.org/r/228960/#review234822

WFM.

::: taskcluster/taskgraph/test/test_try_option_syntax.py:235
(Diff revision 1)
>              {'test': 'gtest', 'platforms': ['linux', 'win32']},
>          ]))
>  
>      def test_u_platforms_pretty(self):
> -        """-u gtest[Ubuntu] selects the linux, linux64, linux64-asan, linux64-stylo-disabled,
> +        """-u gtest[Ubuntu] selects the linux, linux64, linux64-asan
>          and linux64-stylo-sequential platforms for gtest"""

Just to confirm: we're keeping the stylo-sequential platform to ensure that tests still work with sequential stylo?  Or have we totally given up on that front?

Comment 7

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8960207 [details]
Bug 1446954: Remove ImageCacheKey styloToGecko hack.

https://reviewboard.mozilla.org/r/228962/#review234970

Comment 8

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8960208 [details]
Bug 1446954: Remove support for styloVsGecko reftests.

https://reviewboard.mozilla.org/r/228964/#review234972

Comment 9

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8960209 [details]
Bug 1446954: Cleanup !stylo and styloVsGecko test expectations.

https://reviewboard.mozilla.org/r/228966/#review234986

Nice! Thanks for working on this!

I think there are some Gecko-only tests we may want to remove.

There are also mochitests using `isStyledByServo` which can probably also be simplified.

::: layout/reftests/first-line/reftest.list:8
(Diff revision 1)
>  
>  # handling of out-of-flows when ::first-line happens
>  == out-of-flow-1a.html out-of-flow-1-ref.html
>  == out-of-flow-1b.html out-of-flow-1-ref.html
>  == out-of-flow-1c.html out-of-flow-1-ref.html
> -fails-if(!stylo) fails-if(styloVsGecko) == out-of-flow-1d.html out-of-flow-1-ref.html # bug 396645
> +== out-of-flow-1d.html out-of-flow-1-ref.html # bug 396645

It seems the bug has been fixed, and we should remove the bug number here.

::: layout/reftests/font-features/reftest.list:57
(Diff revision 1)
> -skip-if((stylo||styloVsGecko)&&winWidget) == annotations.html annotations-ref.html
> +# XXX Why the skip-if?
> +skip-if(winWidget) == annotations.html annotations-ref.html

Consider filing a bug for this and mark it with the bug number here.

It seems this was initially done in a massive annotating in bug 1380053, and its macOS counterpart was removed in bug 1355721 with no mention on Windows.

::: layout/reftests/forms/legend/reftest.list:4
(Diff revision 1)
>  == legend.html legend-ref.html
>  #bug 1418002 fuzzy-if(skiaContent,1,7) pref(dom.webcomponents.shadowdom.enabled,true) == shadow-dom.html shadow-dom-ref.html
>  == 1273433.html 1273433-ref.html
> -fails-if(styloVsGecko||stylo) == 1339287.html 1339287-ref.html
> +fails == 1339287.html 1339287-ref.html # bug 1446969

This is bug 1383868.

::: layout/reftests/pixel-rounding/reftest.list:182
(Diff revision 1)
>  == border-image-width-4.html border-image-width-0.html
>  == border-image-width-9.html border-image-width-0.html
>  
>  == iframe-1.html iframe-1-ref.html
>  
> -fuzzy-if(stylo||styloVsGecko,128,220) == viewport-units-rounding-1.html viewport-units-rounding-1-ref.html
> +fuzzy-if(stylo,128,220) == viewport-units-rounding-1.html viewport-units-rounding-1-ref.html

Do we need `fuzzy-if(stylo)` now? Should be just `fuzzy()` I suppose?

::: layout/reftests/table-background/reftest.list:7
(Diff revision 1)
> -fuzzy-if(styloVsGecko,1,168) != backgr_border-table-row-group.html empty.html # Bug 1386543
> -fuzzy-if(styloVsGecko,1,204) != backgr_border-table-row.html empty.html # Bug 1386543
> +!= backgr_border-table-row-group.html empty.html # Bug 1386543
> +!= backgr_border-table-row.html empty.html # Bug 1386543

Bug numbers can be removed here as well?

::: layout/reftests/text/reftest.list:330
(Diff revision 1)
>  # osx-font-smoothing - with and without subpixel AA, only under OSX
>  fails-if(!cocoaWidget) != osx-font-smoothing.html osx-font-smoothing-ref.html
>  fails-if(!cocoaWidget) != osx-font-smoothing-2.html osx-font-smoothing-2-notref.html
>  == osx-font-smoothing-2.html osx-font-smoothing-2-ref.html
>  
> -pref(layout.css.text-align-unsafe-value.enabled,true) fails-if(styloVsGecko||stylo) == text-align-unsafe.html text-align-unsafe-ref.html
> +pref(layout.css.text-align-unsafe-value.enabled,true) fails == text-align-unsafe.html text-align-unsafe-ref.html # bug 1446967

bug 1388949

::: layout/reftests/transform/reftest.list:145
(Diff revision 1)
>  == animate-layer-scale-inherit-2.html animate-layer-scale-inherit-2-ref.html
>  == animate-layer-scale-inherit-3.html animate-layer-scale-inherit-1-ref.html
>  # Bug 1301500
>  == dynamic-add-without-change-cb-1.html dynamic-add-without-change-cb-1-ref.html
>  fuzzy-if(d2d,1,5) fuzzy-if(skiaContent,26,208) == table-overflowed-by-animation.html table-overflowed-by-animation-ref.html
> -# Bug 1392161. These tests are failed on Gecko (Bug 1396535). The viewport
> +# Bug 1392161.

Probably worth filing a separate bug for the viewport reftest and mark the failing one with it, given that bug 1392161 has been closed.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

(In reply to Nathan Froyd [:froydnj] from comment #6)
> Just to confirm: we're keeping the stylo-sequential platform to ensure that
> tests still work with sequential stylo?  Or have we totally given up on that
> front?

That's right. It seemed out of scope to remove that one in this bug.

Comment 11

[Emilio Cobos Álvarez (:emilio)]

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #9)
> ::: layout/reftests/transform/reftest.list:145
> (Diff revision 1)
> >  == animate-layer-scale-inherit-2.html animate-layer-scale-inherit-2-ref.html
> >  == animate-layer-scale-inherit-3.html animate-layer-scale-inherit-1-ref.html
> >  # Bug 1301500
> >  == dynamic-add-without-change-cb-1.html dynamic-add-without-change-cb-1-ref.html
> >  fuzzy-if(d2d,1,5) fuzzy-if(skiaContent,26,208) == table-overflowed-by-animation.html table-overflowed-by-animation-ref.html
> > -# Bug 1392161. These tests are failed on Gecko (Bug 1396535). The viewport
> > +# Bug 1392161.
> 
> Probably worth filing a separate bug for the viewport reftest and mark the
> failing one with it, given that bug 1392161 has been closed.

There's bug 1397146, which is the underlying issue, so will use that.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d5b647ddf8e5
Remove support for running tests on automation without stylo configurations. r=froydnj
https://hg.mozilla.org/integration/mozilla-inbound/rev/68011d9bb819
Remove ImageCacheKey styloToGecko hack. r=xidorn
https://hg.mozilla.org/integration/mozilla-inbound/rev/bc054c5c8cc8
Remove support for styloVsGecko reftests. r=xidorn
https://hg.mozilla.org/integration/mozilla-inbound/rev/c5186bbee6e5
Cleanup !stylo and styloVsGecko test expectations. r=xidorn

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e78a0a26858e
followup: Cleanup expectation that snuck after the rebase to reopen the CLOSED TREE. r=me

Comment 14

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

https://hg.mozilla.org/mozilla-central/rev/d5b647ddf8e5
https://hg.mozilla.org/mozilla-central/rev/68011d9bb819
https://hg.mozilla.org/mozilla-central/rev/bc054c5c8cc8
https://hg.mozilla.org/mozilla-central/rev/c5186bbee6e5
https://hg.mozilla.org/mozilla-central/rev/e78a0a26858e