Merge pull request #1270 from googlefonts/fea-filter-tests

[fea-rs] Add ability to filter ttx tests with env var

fea-rs/src/bin/ttx_test.rs

@@ -12,7 +12,7 @@ fn main() {
     env_logger::init();
     let args = Args::parse();
 
-    let results = ttx::run_fonttools_tests(args.test_filter.as_ref());
+    let results = ttx::run_fonttools_tests(args.test_filter);
 
     if let Some(to_compare) = args
         .compare

fea-rs/src/tests/compile.rs

@@ -4,7 +4,7 @@ use std::path::{Path, PathBuf};
 
 use crate::{
     compile::{error::CompilerError, Compiler, MockVariationInfo, NopFeatureProvider, Opts},
-    util::ttx::{self as test_utils, Report, TestCase, TestResult},
+    util::ttx::{self as test_utils, Filter, Report, TestCase, TestResult},
     GlyphMap,
 };
 use fontdrasil::types::GlyphName;
@@ -74,7 +74,7 @@ fn iter_test_groups(
         let glyph_map = glyph_order.lines().map(GlyphName::new).collect();
         let var_info = test_utils::make_var_info();
         let tests_dir = dir.join(test_dir);
-        let tests = test_utils::iter_fea_files(tests_dir).collect::<Vec<_>>();
+        let tests = test_utils::iter_fea_files(tests_dir, Filter::from_env()).collect::<Vec<_>>();
         (glyph_map, var_info, tests)
     })
 }

fea-rs/src/tests/parse.rs

@@ -9,7 +9,7 @@
 use std::{env, path::PathBuf};
 
 use crate::{
-    util::ttx::{self as test_utils, Report, TestCase, TestResult},
+    util::ttx::{self as test_utils, Filter, Report, TestCase, TestResult},
     GlyphIdent, GlyphMap,
 };
 
@@ -30,7 +30,7 @@ fn parse_good() -> Result<(), Report> {
 
     let glyph_map = parse_test_glyph_order();
 
-    let results = test_utils::iter_fea_files(PARSE_GOOD)
+    let results = test_utils::iter_fea_files(PARSE_GOOD, Filter::from_env())
         .chain(OTHER_TESTS.iter().map(PathBuf::from))
         .map(|path| run_good_test(path, &glyph_map))
         .collect::<Vec<_>>();
@@ -40,7 +40,7 @@ fn parse_good() -> Result<(), Report> {
 #[test]
 fn parse_bad() -> Result<(), Report> {
     test_utils::finalize_results(
-        test_utils::iter_fea_files(PARSE_BAD)
+        test_utils::iter_fea_files(PARSE_BAD, Filter::from_env())
             .map(run_bad_test)
             .collect(),
     )

fea-rs/src/util.rs

@@ -17,3 +17,7 @@ pub static SPACES: &str = "
 pub(crate) static WRITE_RESULTS_VAR: &str = "FEA_WRITE_TEST_OUTPUT";
 #[cfg(any(test, feature = "test"))]
 pub(crate) static VERBOSE: &str = "FEA_VERBOSE";
+
+// pass a comma separated list of words, tests which contain those words are run
+#[cfg(any(test, feature = "test"))]
+pub(crate) static FEA_FILTER_TESTS: &str = "FEA_FILTER_TESTS";

fea-rs/src/util/ttx.rs

@@ -26,6 +26,8 @@ use write_fonts::{
     types::{GlyphId16, Tag},
 };
 
+use super::FEA_FILTER_TESTS;
+
 static IGNORED_TESTS: &[&str] = &[
     // ## tests with unofficial syntax extensiosn we haven't implemented yet ## //
     "AlternateChained.fea",
@@ -128,21 +130,32 @@ pub fn assert_has_ttx_executable() {
 }
 
 /// Selectively filter which files to run.
-pub struct Filter<'a>(Vec<&'a str>);
+#[derive(Clone, Debug, Default)]
+pub struct Filter(Vec<String>);
+
+impl Filter {
+    /// A new filter, reading the `FEA_FILTER_TESTS` env var
+    pub fn from_env() -> Self {
+        Self::new(std::env::var(FEA_FILTER_TESTS).ok())
+    }
 
-impl<'a> Filter<'a> {
     /// Create a new filter from a comma-separated list of inputs
-    pub fn new(input: Option<&'a String>) -> Self {
+    pub fn new(input: Option<String>) -> Self {
         Self(
             input
-                .map(|s| s.split(',').map(|s| s.trim()).collect::<Vec<_>>())
+                .map(|s| {
+                    s.split(',')
+                        .map(|s| s.trim().to_owned())
+                        .collect::<Vec<_>>()
+                })
                 .unwrap_or_default(),
         )
     }
 
     /// true if this matches the filter, false if not
-    pub fn filter(&self, item: &str) -> bool {
-        self.0.is_empty() || self.0.iter().any(|needle| item.contains(needle))
+    pub fn filter(&self, item: &Path) -> bool {
+        let str_item = item.to_str().unwrap_or_default();
+        self.0.is_empty() || self.0.iter().any(|needle| str_item.contains(needle))
     }
 }
 
@@ -153,13 +166,17 @@ impl<'a> Filter<'a> {
 ///
 /// `filter` is an optional comma-separated list of strings. If present, only
 /// tests which contain one of the strings in the list will be run.
-pub fn run_fonttools_tests(filter: Option<&String>) -> Report {
+pub fn run_fonttools_tests(filter: Option<String>) -> Report {
     let fonttools_data_dir = test_data_dir().join("fonttools-tests");
     let glyph_map = fonttools_test_glyph_order();
     let filter = Filter::new(filter);
     let var_info = make_var_info();
 
-    let result = iter_compile_tests(fonttools_data_dir.as_ref(), filter)
+    let result = iter_fea_files(&fonttools_data_dir, filter)
+        .filter(|test| {
+            test.with_extension("ttx").exists()
+                && !IGNORED_TESTS.contains(&test.file_name().unwrap().to_str().unwrap())
+        })
         .par_bridge()
         .map(|path| run_test(path, &glyph_map, &var_info))
         .collect::<Vec<_>>();
@@ -195,30 +212,17 @@ fn is_fea(path: &Path) -> bool {
     pstr.ends_with(".fea")
 }
 
-fn iter_compile_tests<'a>(
-    path: &'a Path,
-    filter: Filter<'a>,
-) -> impl Iterator<Item = PathBuf> + 'a {
-    iter_fea_files(path).filter(move |p| {
-        if is_fea(p) && p.with_extension("ttx").exists() {
-            let path_str = p.file_name().unwrap().to_str().unwrap();
-            return should_run_test(path_str) && filter.filter(path_str);
-        }
-        false
-    })
-}
-
-fn should_run_test(path: &str) -> bool {
-    !IGNORED_TESTS.contains(&path)
-}
-
 /// Iterate over all the files in a directory with the 'fea' suffix. Only used for test purposes.
-pub fn iter_fea_files(path: impl AsRef<Path>) -> impl Iterator<Item = PathBuf> + 'static {
-    let mut dir = path.as_ref().read_dir().ok();
+pub fn iter_fea_files(
+    path: impl AsRef<Path>,
+    filter: Filter,
+) -> impl Iterator<Item = PathBuf> + 'static {
+    let path = path.as_ref();
+    let mut dir = path.read_dir().ok();
     std::iter::from_fn(move || loop {
         let entry = dir.as_mut()?.next()?.unwrap();
         let path = entry.path();
-        if is_fea(&path) {
+        if is_fea(&path) && filter.filter(&path) {
             return Some(path);
         }
     })