Auto merge of #17984 - Manishearth:rm-testing, r=SimonSapin

Remove style/testing feature

We added this because a year ago we had no reliable Gecko CI. This meant that Gecko-only properties needed to be tested *somehow*, and we solved that by making it so that for unit tests we compile all properties, not just the servo ones.

This was useful back then, but I don't think we need this anymore. We have reliable Gecko CI, and all the gecko-only stuff we tested is adequately handled by the properties-database parsing mochitests. It's a bit of annoying cruft that just complicates things; we probably should remove it.

r? @emilio or @SimonSapin

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17984)
<!-- Reviewable:end -->

components/servo/Cargo.toml

@@ -16,7 +16,6 @@ default = ["webdriver", "max_log_level"]
 max_log_level = ["log/release_max_level_info"]
 webdriver = ["webdriver_server"]
 energy-profiling = ["profile_traits/energy-profiling"]
-testing = ["style/testing"]
 debugmozjs = ["script/debugmozjs"]
 
 [dependencies]

components/style/Cargo.toml

@@ -26,7 +26,6 @@ servo = ["serde", "heapsize", "heapsize_derive",
          #"arrayvec/use_union"
 
          "servo_url"]
-testing = []
 gecko_debug = ["nsstring_vendor/gecko_debug"]
 
 [dependencies]

components/style/build.rs

@@ -75,7 +75,6 @@ fn generate_properties() {
         .arg(&script)
         .arg(product)
         .arg("style-crate")
-        .arg(if cfg!(feature = "testing") { "testing" } else { "regular" })
         .status()
         .unwrap();
     if !status.success() {

components/style/properties/build.py

@@ -21,17 +21,16 @@
 
 
 def main():
-    usage = "Usage: %s [ servo | gecko ] [ style-crate | html ] [ testing | regular ]" % sys.argv[0]
-    if len(sys.argv) < 4:
+    usage = "Usage: %s [ servo | gecko ] [ style-crate | html ]" % sys.argv[0]
+    if len(sys.argv) < 3:
         abort(usage)
     product = sys.argv[1]
     output = sys.argv[2]
-    testing = sys.argv[3] == "testing"
 
     if product not in ["servo", "gecko"] or output not in ["style-crate", "geckolib", "html"]:
         abort(usage)
 
-    properties = data.PropertiesData(product=product, testing=testing)
+    properties = data.PropertiesData(product=product)
     template = os.path.join(BASE, "properties.mako.rs")
     rust = render(template, product=product, data=properties, __file__=template)
     if output == "style-crate":

components/style/properties/data.py

@@ -304,18 +304,8 @@ def __init__(self, name, inherited, gecko_name=None, additional_methods=None):
 
 
 class PropertiesData(object):
-    """
-        The `testing` parameter means that we're running tests.
-
-        In this situation, the `product` value is ignored while choosing
-        which shorthands and longhands to generate; and instead all properties for
-        which code exists for either servo or stylo are generated. Note that we skip
-        this behavior when the style crate is being built in gecko mode, because we
-        need manual glue for such properties and we don't have it.
-    """
-    def __init__(self, product, testing):
+    def __init__(self, product):
         self.product = product
-        self.testing = testing and product != "gecko"
         self.style_structs = []
         self.current_style_struct = None
         self.longhands = []
@@ -338,9 +328,9 @@ def add_prefixed_aliases(self, property):
             for prefix in property.extra_prefixes:
                 property.alias.append('-%s-%s' % (prefix, property.name))
 
-    def declare_longhand(self, name, products="gecko servo", disable_when_testing=False, **kwargs):
+    def declare_longhand(self, name, products="gecko servo", **kwargs):
         products = products.split()
-        if self.product not in products and not (self.testing and not disable_when_testing):
+        if self.product not in products:
             return
 
         longhand = Longhand(self.current_style_struct, name, **kwargs)
@@ -354,10 +344,9 @@ def declare_longhand(self, name, products="gecko servo", disable_when_testing=Fa
 
         return longhand
 
-    def declare_shorthand(self, name, sub_properties, products="gecko servo",
-                          disable_when_testing=False, *args, **kwargs):
+    def declare_shorthand(self, name, sub_properties, products="gecko servo", *args, **kwargs):
         products = products.split()
-        if self.product not in products and not (self.testing and not disable_when_testing):
+        if self.product not in products:
             return
 
         sub_properties = [self.longhands_by_name[s] for s in sub_properties]

components/style/properties/helpers/animated_properties.mako.rs

@@ -34,7 +34,7 @@ use std::cmp;
 #[cfg(feature = "gecko")] use fnv::FnvHashMap;
 use style_traits::ParseError;
 use super::ComputedValues;
-#[cfg(any(feature = "gecko", feature = "testing"))]
+#[cfg(feature = "gecko")]
 use values::Auto;
 use values::{CSSFloat, CustomIdent, Either};
 use values::animated::{ToAnimatedValue, ToAnimatedZero};

components/style/properties/longhand/box.mako.rs

@@ -711,7 +711,6 @@ ${helpers.predefined_type("animation-delay",
         "computed::ScrollSnapPoint::none()",
         animation_value_type="discrete",
         products="gecko",
-        disable_when_testing=True,
         spec="Nonstandard (https://www.w3.org/TR/2015/WD-css-snappoints-1-20150326/#scroll-snap-points)",
     )}
 % endfor
@@ -1832,8 +1831,7 @@ ${helpers.predefined_type("-moz-binding", "UrlOrNone", "Either::Second(None_)",
                           boxed="True" if product == "gecko" else "False",
                           animation_value_type="none",
                           gecko_ffi_name="mBinding",
-                          spec="Nonstandard (https://developer.mozilla.org/en-US/docs/Web/CSS/-moz-binding)",
-                          disable_when_testing="True")}
+                          spec="Nonstandard (https://developer.mozilla.org/en-US/docs/Web/CSS/-moz-binding)")}
 
 ${helpers.single_keyword("-moz-orient",
                           "inline block horizontal vertical",
@@ -1915,7 +1913,6 @@ ${helpers.predefined_type("shape-outside", "basic_shape::FloatAreaShape",
 <%helpers:longhand name="touch-action"
                    products="gecko"
                    animation_value_type="discrete"
-                   disable_when_testing="True"
                    spec="https://compat.spec.whatwg.org/#touch-action">
     use gecko_bindings::structs;
     use std::fmt;

components/style/properties/longhand/font.mako.rs

@@ -1507,7 +1507,7 @@ ${helpers.single_keyword_system("font-kerning",
     }
 </%helpers:longhand>
 
-#[cfg(any(feature = "gecko", feature = "testing"))]
+#[cfg(feature = "gecko")]
 macro_rules! exclusive_value {
     (($value:ident, $set:expr) => $ident:ident) => {
         if $value.intersects($set) {
@@ -2256,7 +2256,7 @@ https://drafts.csswg.org/css-fonts-4/#low-level-font-variation-settings-control-
 <%helpers:longhand name="-moz-script-size-multiplier" products="gecko" animation_value_type="none"
                    predefined_type="Number" gecko_ffi_name="mScriptSizeMultiplier"
                    spec="Internal (not web-exposed)"
-                   internal="True" disable_when_testing="True">
+                   internal="True">
     use values::computed::ComputedValueAsSpecified;
     pub use self::computed_value::T as SpecifiedValue;
 
@@ -2281,7 +2281,7 @@ https://drafts.csswg.org/css-fonts-4/#low-level-font-variation-settings-control-
 <%helpers:longhand name="-moz-script-level" products="gecko" animation_value_type="none"
                    predefined_type="Integer" gecko_ffi_name="mScriptLevel"
                    spec="Internal (not web-exposed)"
-                   internal="True" disable_when_testing="True" need_clone="True">
+                   internal="True" need_clone="True">
     use std::fmt;
     use style_traits::ToCss;
 
@@ -2380,7 +2380,7 @@ ${helpers.single_keyword("-moz-math-variant",
 <%helpers:longhand name="-moz-script-min-size" products="gecko" animation_value_type="none"
                    predefined_type="Length" gecko_ffi_name="mScriptMinSize"
                    spec="Internal (not web-exposed)"
-                   internal="True" disable_when_testing="True">
+                   internal="True">
     use app_units::Au;
     use gecko_bindings::structs::NS_MATHML_DEFAULT_SCRIPT_MIN_SIZE_PT;
     use values::specified::length::{AU_PER_PT, FontBaseSize, NoCalcLength};

components/style/properties/longhand/position.mako.rs

@@ -416,7 +416,6 @@ ${helpers.predefined_type("object-position",
         spec="https://drafts.csswg.org/css-grid/#propdef-grid-template-areas"
         products="gecko"
         animation_value_type="discrete"
-        disable_when_testing="True"
         boxed="True">
     use std::collections::HashMap;
     use std::fmt;

components/style/properties/shorthand/font.mako.rs

@@ -8,15 +8,15 @@
 <%helpers:shorthand name="font"
                     sub_properties="font-style font-variant-caps font-weight font-stretch
                                     font-size line-height font-family
-                                    ${'font-size-adjust' if product == 'gecko' or data.testing else ''}
-                                    ${'font-kerning' if product == 'gecko' or data.testing else ''}
-                                    ${'font-variant-alternates' if product == 'gecko' or data.testing else ''}
-                                    ${'font-variant-east-asian' if product == 'gecko' or data.testing else ''}
-                                    ${'font-variant-ligatures' if product == 'gecko' or data.testing else ''}
-                                    ${'font-variant-numeric' if product == 'gecko' or data.testing else ''}
-                                    ${'font-variant-position' if product == 'gecko' or data.testing else ''}
-                                    ${'font-language-override' if product == 'gecko' or data.testing else ''}
-                                    ${'font-feature-settings' if product == 'gecko' or data.testing else ''}"
+                                    ${'font-size-adjust' if product == 'gecko' else ''}
+                                    ${'font-kerning' if product == 'gecko' else ''}
+                                    ${'font-variant-alternates' if product == 'gecko' else ''}
+                                    ${'font-variant-east-asian' if product == 'gecko' else ''}
+                                    ${'font-variant-ligatures' if product == 'gecko' else ''}
+                                    ${'font-variant-numeric' if product == 'gecko' else ''}
+                                    ${'font-variant-position' if product == 'gecko' else ''}
+                                    ${'font-language-override' if product == 'gecko' else ''}
+                                    ${'font-feature-settings' if product == 'gecko' else ''}"
                     spec="https://drafts.csswg.org/css-fonts-3/#propdef-font">
     use parser::Parse;
     use properties::longhands::{font_family, font_style, font_weight, font_stretch};
@@ -31,7 +31,7 @@
                                 variant_ligatures variant_numeric \
                                 variant_position feature_settings".split()
     %>
-    % if product == "gecko" or data.testing:
+    % if product == "gecko":
         % for prop in gecko_sub_properties:
             use properties::longhands::font_${prop};
         % endfor
@@ -112,7 +112,7 @@
             % endfor
             line_height: line_height.unwrap_or(LineHeight::normal()),
             font_family: family,
-            % if product == "gecko" or data.testing:
+            % if product == "gecko":
                 % for name in gecko_sub_properties:
                     font_${name}: font_${name}::get_initial_specified_value(),
                 % endfor
@@ -146,7 +146,7 @@
                 }
             % endif
 
-            % if product == "gecko" or data.testing:
+            % if product == "gecko":
                 % for name in gecko_sub_properties:
                     if self.font_${name} != &font_${name}::get_initial_specified_value() {
                         return Ok(());
@@ -226,16 +226,16 @@
 
 <%helpers:shorthand name="font-variant"
                     sub_properties="font-variant-caps
-                                    ${'font-variant-alternates' if product == 'gecko' or data.testing else ''}
-                                    ${'font-variant-east-asian' if product == 'gecko' or data.testing else ''}
-                                    ${'font-variant-ligatures' if product == 'gecko' or data.testing else ''}
-                                    ${'font-variant-numeric' if product == 'gecko' or data.testing else ''}
-                                    ${'font-variant-position' if product == 'gecko' or data.testing else ''}"
+                                    ${'font-variant-alternates' if product == 'gecko' else ''}
+                                    ${'font-variant-east-asian' if product == 'gecko' else ''}
+                                    ${'font-variant-ligatures' if product == 'gecko' else ''}
+                                    ${'font-variant-numeric' if product == 'gecko' else ''}
+                                    ${'font-variant-position' if product == 'gecko' else ''}"
                     spec="https://drafts.csswg.org/css-fonts-3/#propdef-font-variant">
     <% gecko_sub_properties = "alternates east_asian ligatures numeric position".split() %>
     <%
         sub_properties = ["caps"]
-        if product == "gecko" or data.testing:
+        if product == "gecko":
             sub_properties += gecko_sub_properties
     %>
 
@@ -254,7 +254,7 @@
         } else if input.try(|input| input.expect_ident_matching("none")).is_ok() {
             // The 'none' value sets 'font-variant-ligatures' to 'none' and resets all other sub properties
             // to their initial value.
-        % if product == "gecko" or data.testing:
+        % if product == "gecko":
             ligatures = Some(font_variant_ligatures::get_none_specified_value());
         % endif
         } else {
@@ -294,7 +294,7 @@
         fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
 
             let has_none_ligatures =
-            % if product == "gecko" or data.testing:
+            % if product == "gecko":
                 self.font_variant_ligatures == &font_variant_ligatures::get_none_specified_value();
             % else:
                 false;

components/style/properties/shorthand/position.mako.rs

@@ -239,7 +239,6 @@
 <%helpers:shorthand name="grid-template"
                     sub_properties="grid-template-rows grid-template-columns grid-template-areas"
                     spec="https://drafts.csswg.org/css-grid/#propdef-grid-template"
-                    disable_when_testing="True"
                     products="gecko">
     use parser::Parse;
     use properties::longhands::grid_template_areas::TemplateAreas;
@@ -452,7 +451,6 @@
                                     grid-auto-rows grid-auto-columns grid-row-gap grid-column-gap
                                     grid-auto-flow"
                     spec="https://drafts.csswg.org/css-grid/#propdef-grid"
-                    disable_when_testing="True"
                     products="gecko">
     use parser::Parse;
     use properties::longhands::{grid_auto_columns, grid_auto_rows, grid_auto_flow};
@@ -612,7 +610,7 @@
 
 <%helpers:shorthand name="place-content" sub_properties="align-content justify-content"
                     spec="https://drafts.csswg.org/css-align/#propdef-place-content"
-                    products="gecko" disable_when_testing="True">
+                    products="gecko">
     use properties::longhands::align_content;
     use properties::longhands::justify_content;
 
@@ -648,7 +646,7 @@
 
 <%helpers:shorthand name="place-self" sub_properties="align-self justify-self"
                     spec="https://drafts.csswg.org/css-align/#place-self-property"
-                    products="gecko" disable_when_testing="True">
+                    products="gecko">
     use values::specified::align::AlignJustifySelf;
     use parser::Parse;
 
@@ -684,7 +682,7 @@
 
 <%helpers:shorthand name="place-items" sub_properties="align-items justify-items"
                     spec="https://drafts.csswg.org/css-align/#place-items-property"
-                    products="gecko" disable_when_testing="True">
+                    products="gecko">
     use values::specified::align::{AlignItems, JustifyItems};
     use parser::Parse;
 

components/style/properties/shorthand/text.mako.rs

@@ -6,10 +6,10 @@
 
 <%helpers:shorthand name="text-decoration"
                     sub_properties="text-decoration-line
-                    ${' text-decoration-style text-decoration-color' if product == 'gecko' or data.testing else ''}"
+                    ${' text-decoration-style text-decoration-color' if product == 'gecko' else ''}"
                     spec="https://drafts.csswg.org/css-text-decor/#propdef-text-decoration">
 
-    % if product == "gecko" or data.testing:
+    % if product == "gecko":
         use values::specified;
         use properties::longhands::{text_decoration_line, text_decoration_style, text_decoration_color};
     % else:
@@ -18,7 +18,7 @@
 
     pub fn parse_value<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>)
                                -> Result<Longhands, ParseError<'i>> {
-        % if product == "gecko" or data.testing:
+        % if product == "gecko":
             let (mut line, mut style, mut color, mut any) = (None, None, None, false);
         % else:
             let (mut line, mut any) = (None, false);
@@ -39,7 +39,7 @@
 
             parse_component!(line, text_decoration_line);
 
-            % if product == "gecko" or data.testing:
+            % if product == "gecko":
                 parse_component!(style, text_decoration_style);
                 parse_component!(color, text_decoration_color);
             % endif
@@ -54,7 +54,7 @@
         Ok(expanded! {
             text_decoration_line: unwrap_or_initial!(text_decoration_line, line),
 
-            % if product == "gecko" or data.testing:
+            % if product == "gecko":
                 text_decoration_style: unwrap_or_initial!(text_decoration_style, style),
                 text_decoration_color: unwrap_or_initial!(text_decoration_color, color),
             % endif
@@ -65,7 +65,7 @@
         fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
             self.text_decoration_line.to_css(dest)?;
 
-            % if product == "gecko" or data.testing:
+            % if product == "gecko":
                 if self.text_decoration_style != &text_decoration_style::SpecifiedValue::solid {
                     dest.write_str(" ")?;
                     self.text_decoration_style.to_css(dest)?;

components/style/rule_tree/mod.rs

@@ -953,11 +953,10 @@ impl StrongRuleNode {
         // That's... suspicious, but it's fine if it happens for the rule tree
         // case, so just don't crash in the case we're doing the final GC in
         // script.
-        if !cfg!(feature = "testing") {
-            debug_assert!(!thread_state::get().is_worker() &&
-                          (thread_state::get().is_layout() ||
-                           thread_state::get().is_script()));
-        }
+
+        debug_assert!(!thread_state::get().is_worker() &&
+                      (thread_state::get().is_layout() ||
+                       thread_state::get().is_script()));
 
         let current = me.next_free.load(Ordering::Relaxed);
         if current == FREE_LIST_SENTINEL {

components/style/stylesheets/rule_parser.rs

@@ -359,7 +359,7 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> {
                 Ok(AtRuleType::WithBlock(AtRulePrelude::FontFace(location)))
             },
             "font-feature-values" => {
-                if !cfg!(feature = "gecko") && !cfg!(feature = "testing") {
+                if !cfg!(feature = "gecko") {
                     // Support for this rule is not fully implemented in Servo yet.
                     return Err(StyleParseError::UnsupportedAtRule(name.clone()).into())
                 }

ports/geckolib/Cargo.toml

@@ -11,7 +11,6 @@ crate-type = ["staticlib", "rlib"]
 
 [features]
 bindgen = ["style/use_bindgen"]
-testing = ["style/testing"]
 gecko_debug = ["style/gecko_debug"]
 
 [dependencies]

ports/servo/Cargo.toml

@@ -32,7 +32,6 @@ default = ["webdriver", "max_log_level"]
 max_log_level = ["log/release_max_level_info"]
 webdriver = ["libservo/webdriver_server"]
 energy-profiling = ["libservo/energy-profiling"]
-testing = ["libservo/testing"]
 debugmozjs = ["libservo/debugmozjs"]
 
 [dependencies]