From 33ec3a2b99cbac2e1100d67375fc077f84106721 Mon Sep 17 00:00:00 2001 From: Colin Rofls Date: Thu, 13 Feb 2025 12:28:47 -0500 Subject: [PATCH] [fea-rs] Add ability to filter ttx tests with env var This functionality existed but only worked for the bundled fonttools tests (for historical reasons) this expands on that so it can be used for all the ttx-based tests, and lets it be controlled via the FEA_FILTER_TESTS environment variable. Also removes some redundant code, but otherwise no functional change. --- fea-rs/src/bin/ttx_test.rs | 2 +- fea-rs/src/tests/compile.rs | 4 +-- fea-rs/src/tests/parse.rs | 6 ++-- fea-rs/src/util.rs | 4 +++ fea-rs/src/util/ttx.rs | 60 ++++++++++++++++++++----------------- 5 files changed, 42 insertions(+), 34 deletions(-) diff --git a/fea-rs/src/bin/ttx_test.rs b/fea-rs/src/bin/ttx_test.rs index 0af22b37f..1971f171b 100644 --- a/fea-rs/src/bin/ttx_test.rs +++ b/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 diff --git a/fea-rs/src/tests/compile.rs b/fea-rs/src/tests/compile.rs index 53fdc99d9..3257a3a3a 100644 --- a/fea-rs/src/tests/compile.rs +++ b/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::>(); + let tests = test_utils::iter_fea_files(tests_dir, Filter::from_env()).collect::>(); (glyph_map, var_info, tests) }) } diff --git a/fea-rs/src/tests/parse.rs b/fea-rs/src/tests/parse.rs index 0b5fb64dd..62ccce674 100644 --- a/fea-rs/src/tests/parse.rs +++ b/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::>(); @@ -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(), ) diff --git a/fea-rs/src/util.rs b/fea-rs/src/util.rs index 68c89a6fa..6004acb70 100644 --- a/fea-rs/src/util.rs +++ b/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"; diff --git a/fea-rs/src/util/ttx.rs b/fea-rs/src/util/ttx.rs index 3ba71baae..e06bfbc00 100644 --- a/fea-rs/src/util/ttx.rs +++ b/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); + +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) -> Self { Self( input - .map(|s| s.split(',').map(|s| s.trim()).collect::>()) + .map(|s| { + s.split(',') + .map(|s| s.trim().to_owned()) + .collect::>() + }) .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) -> 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::>(); @@ -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 + '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) -> impl Iterator + 'static { - let mut dir = path.as_ref().read_dir().ok(); +pub fn iter_fea_files( + path: impl AsRef, + filter: Filter, +) -> impl Iterator + '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); } })