From 4213350301b73d8f30c523d3a38eda05aadb54b1 Mon Sep 17 00:00:00 2001 From: Nathan Froyd <froydnj@gmail.com> Date: Sat, 5 Aug 2017 09:58:26 -0500 Subject: [PATCH] servo: Merge #17974 - remove testing feature from stylo_tests (from froydnj:remove-testing-feature); r=SimonSapin `stylo_tests` currently requires a separate version of the `style` crate, compiled with the `testing` feature, so a function testing the size of specified values can be accessed. With a few tweaks, we can make the information needed for the test available to the `stylo_tests` crate directly, eliminating the need for a separately-compiled `style` crate. This doesn't matter much for Servo itself (it might make CI times slightly faster?), but Gecko automation/development would like to run `stylo_tests`, and not having to compile two versions of the `style` crate (or have a dead, test-only function hanging around in the `style` crate) would be a win. --- - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] There are tests for these changes Source-Repo: https://github.com/servo/servo Source-Revision: a6369149dc5344b2b80a12fca1c43cf99c94fdc9 --HG-- extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear extra : subtree_revision : f23cc39c69a996f3d1023f4c00f8d564d4a3e97f --- servo/components/style/lib.rs | 2 +- .../style/properties/properties.mako.rs | 40 +-------------- servo/tests/unit/style/Cargo.toml | 3 -- servo/tests/unit/style/lib.rs | 4 +- servo/tests/unit/style/size_of.rs | 5 -- servo/tests/unit/stylo/Cargo.toml | 3 -- servo/tests/unit/stylo/lib.rs | 1 + servo/tests/unit/stylo/size_of.rs | 5 -- servo/tests/unit/stylo/specified_values.rs | 49 +++++++++++++++++++ 9 files changed, 56 insertions(+), 56 deletions(-) create mode 100644 servo/tests/unit/stylo/specified_values.rs diff --git a/servo/components/style/lib.rs b/servo/components/style/lib.rs index b82d2fd3cc0a2..e1453706977d6 100644 --- a/servo/components/style/lib.rs +++ b/servo/components/style/lib.rs @@ -171,7 +171,7 @@ pub mod gecko_properties { } macro_rules! reexport_computed_values { - ( $( $name: ident )+ ) => { + ( $( { $name: ident, $boxed: expr } )+ ) => { /// Types for [computed values][computed]. /// /// [computed]: https://drafts.csswg.org/css-cascade/#computed diff --git a/servo/components/style/properties/properties.mako.rs b/servo/components/style/properties/properties.mako.rs index a80fcfb8c1e14..ab8fce24621c0 100644 --- a/servo/components/style/properties/properties.mako.rs +++ b/servo/components/style/properties/properties.mako.rs @@ -3442,49 +3442,13 @@ macro_rules! css_properties_accessors { } } - +#[macro_export] macro_rules! longhand_properties_idents { ($macro_name: ident) => { $macro_name! { % for property in data.longhands: - ${property.ident} + { ${property.ident}, ${"true" if property.boxed else "false"} } % endfor } } } - -/// Testing function to check the size of all SpecifiedValues. -#[cfg(feature = "testing")] -pub fn test_size_of_specified_values() { - use std::mem::size_of; - let threshold = 24; - - let mut longhands = vec![]; - % for property in data.longhands: - longhands.push(("${property.name}", - size_of::<longhands::${property.ident}::SpecifiedValue>(), - ${"true" if property.boxed else "false"})); - % endfor - - let mut failing_messages = vec![]; - - for specified_value in longhands { - if specified_value.1 > threshold && !specified_value.2 { - failing_messages.push( - format!("Your changes have increased the size of {} SpecifiedValue to {}. The threshold is \ - currently {}. SpecifiedValues affect size of PropertyDeclaration enum and \ - increasing the size may negative affect style system performance. Please consider \ - using `boxed=\"True\"` in this longhand.", - specified_value.0, specified_value.1, threshold)); - } else if specified_value.1 <= threshold && specified_value.2 { - failing_messages.push( - format!("Your changes have decreased the size of {} SpecifiedValue to {}. Good work! \ - The threshold is currently {}. Please consider removing `boxed=\"True\"` from this longhand.", - specified_value.0, specified_value.1, threshold)); - } - } - - if !failing_messages.is_empty() { - panic!("{}", failing_messages.join("\n\n")); - } -} diff --git a/servo/tests/unit/style/Cargo.toml b/servo/tests/unit/style/Cargo.toml index 51a0278c989b3..53585feb6677e 100644 --- a/servo/tests/unit/style/Cargo.toml +++ b/servo/tests/unit/style/Cargo.toml @@ -9,9 +9,6 @@ name = "style_tests" path = "lib.rs" doctest = false -[features] -testing = ["style/testing"] - [dependencies] byteorder = "1.0" app_units = "0.5" diff --git a/servo/tests/unit/style/lib.rs b/servo/tests/unit/style/lib.rs index 0647a59251fd1..572a608d212d1 100644 --- a/servo/tests/unit/style/lib.rs +++ b/servo/tests/unit/style/lib.rs @@ -19,7 +19,7 @@ extern crate servo_atoms; extern crate servo_config; extern crate servo_url; #[macro_use] extern crate size_of_test; -extern crate style; +#[macro_use] extern crate style; extern crate style_traits; extern crate test; @@ -32,6 +32,8 @@ mod parsing; mod properties; mod rule_tree; mod size_of; +#[path = "../stylo/specified_values.rs"] +mod specified_values; mod str; mod stylesheets; mod stylist; diff --git a/servo/tests/unit/style/size_of.rs b/servo/tests/unit/style/size_of.rs index 486fecf4e25fd..3f15a8d24e44b 100644 --- a/servo/tests/unit/style/size_of.rs +++ b/servo/tests/unit/style/size_of.rs @@ -12,8 +12,3 @@ size_of_test!(test_size_of_property_declaration, properties::PropertyDeclaration // This is huge, but we allocate it on the stack and then never move it, // we only pass `&mut SourcePropertyDeclaration` references around. size_of_test!(test_size_of_parsed_declaration, properties::SourcePropertyDeclaration, 576); - -#[test] -fn size_of_specified_values() { - ::style::properties::test_size_of_specified_values(); -} diff --git a/servo/tests/unit/stylo/Cargo.toml b/servo/tests/unit/stylo/Cargo.toml index 21eab2958a18f..b3381276ff12d 100644 --- a/servo/tests/unit/stylo/Cargo.toml +++ b/servo/tests/unit/stylo/Cargo.toml @@ -11,9 +11,6 @@ name = "stylo_tests" path = "lib.rs" doctest = false -[features] -testing = ["style/testing"] - [dependencies] atomic_refcell = "0.1" cssparser = "0.18" diff --git a/servo/tests/unit/stylo/lib.rs b/servo/tests/unit/stylo/lib.rs index 05a32869c4eb5..45b9d63a9fdad 100644 --- a/servo/tests/unit/stylo/lib.rs +++ b/servo/tests/unit/stylo/lib.rs @@ -14,6 +14,7 @@ extern crate style_traits; mod sanity_checks; mod size_of; +mod specified_values; mod servo_function_signatures; diff --git a/servo/tests/unit/stylo/size_of.rs b/servo/tests/unit/stylo/size_of.rs index 4f4521df9e283..3a830e1c40a28 100644 --- a/servo/tests/unit/stylo/size_of.rs +++ b/servo/tests/unit/stylo/size_of.rs @@ -47,8 +47,3 @@ size_of_test!(test_size_of_rule_node, RuleNode, 80); // This is huge, but we allocate it on the stack and then never move it, // we only pass `&mut SourcePropertyDeclaration` references around. size_of_test!(test_size_of_parsed_declaration, style::properties::SourcePropertyDeclaration, 704); - -#[test] -fn size_of_specified_values() { - ::style::properties::test_size_of_specified_values(); -} diff --git a/servo/tests/unit/stylo/specified_values.rs b/servo/tests/unit/stylo/specified_values.rs new file mode 100644 index 0000000000000..484f9534502bc --- /dev/null +++ b/servo/tests/unit/stylo/specified_values.rs @@ -0,0 +1,49 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +use style; + +#[test] +fn size_of_specified_values() { + use std::mem::size_of; + let threshold = 24; + + let mut bad_properties = vec![]; + + macro_rules! check_property { + ( $( { $name: ident, $boxed: expr } )+ ) => { + $( + let size = size_of::<style::properties::longhands::$name::SpecifiedValue>(); + let is_boxed = $boxed; + if (!is_boxed && size > threshold) || (is_boxed && size <= threshold) { + bad_properties.push(("$name", size, is_boxed)); + } + )+ + } + } + + longhand_properties_idents!(check_property); + + let mut failing_messages = vec![]; + + for bad_prop in bad_properties { + if !bad_prop.2 { + failing_messages.push( + format!("Your changes have increased the size of {} SpecifiedValue to {}. The threshold is \ + currently {}. SpecifiedValues affect size of PropertyDeclaration enum and \ + increasing the size may negative affect style system performance. Please consider \ + using `boxed=\"True\"` in this longhand.", + bad_prop.0, bad_prop.1, threshold)); + } else if bad_prop.2 { + failing_messages.push( + format!("Your changes have decreased the size of {} SpecifiedValue to {}. Good work! \ + The threshold is currently {}. Please consider removing `boxed=\"True\"` from this longhand.", + bad_prop.0, bad_prop.1, threshold)); + } + } + + if !failing_messages.is_empty() { + panic!("{}", failing_messages.join("\n\n")); + } +} -- GitLab