Skip to content

Commit

Permalink
Replace test boilerplate with strum macros
Browse files Browse the repository at this point in the history 
This removes a lot of potential for error as the number of test cases
keeps growing.
  • Loading branch information
@robbert-vdh
robbert-vdh committed Nov 4, 2022
1 parent 32b14ae commit 42bb1bb
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 137 deletions.
27 changes: 27 additions & 0 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ regex = "1.6"
serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0"
simplelog = "0.12"
strum = "0.24.1"
strum_macros = "0.24.1"
tempfile = "3.3"
textwrap = { version = "0.15.0", features = ["terminal_size"] }
walkdir = "2.3"
Expand Down
21 changes: 7 additions & 14 deletions src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,12 @@
use anyhow::{Context, Result};
use serde::{Deserialize, Serialize};
use std::ffi::OsStr;
use std::fmt::Display;
use std::fs;
use std::path::PathBuf;
use std::process::{Command, Stdio};
use std::str::FromStr;
use strum::IntoEnumIterator;

use crate::util;

Expand Down Expand Up @@ -56,21 +59,11 @@ pub enum TestStatus {

/// An abstraction for a test case. This mostly exists because we need two separate kinds of tests
/// (per library and per plugin), and it's good to keep the interface uniform.
pub trait TestCase<'a>: Sized + 'static {
pub trait TestCase<'a>: Display + FromStr + IntoEnumIterator + Sized + 'static {
/// The type of the arguments the test cases are parameterized over. This can be an instance of
/// the plugin library and a plugin ID, or just the file path to the plugin library.
type TestArgs;

/// All available test cases.
const ALL: &'static [Self];

/// Try to parse a test case's string representation as produced by
/// [`as_str()`][Self::as_str()]. Returns `None` if the test case name was not recognized.
fn from_str(string: &str) -> Option<Self>;

/// Get the string representation of this test case.
fn as_str(&self) -> &'static str;

/// Get the textual description for a test case. This description won't contain any line breaks,
/// but it may consist of multiple sentences.
fn description(&self) -> String;
Expand Down Expand Up @@ -133,7 +126,7 @@ pub trait TestCase<'a>: Sized + 'static {
.context("Error while waiting on clap-validator to finish running the test")?;
if !exit_status.success() {
return Ok(TestResult {
name: self.as_str().to_string(),
name: self.to_string(),
description: self.description(),
status: TestStatus::Crashed {
details: exit_status.to_string(),
Expand Down Expand Up @@ -161,7 +154,7 @@ pub trait TestCase<'a>: Sized + 'static {
fn temporary_file(&self, plugin_id: &str, name: &str) -> Result<(PathBuf, fs::File)> {
let path = util::validator_temp_dir()
.join(plugin_id)
.join(self.as_str())
.join(self.to_string())
.join(name);
if path.exists() {
panic!(
Expand All @@ -182,7 +175,7 @@ pub trait TestCase<'a>: Sized + 'static {
/// Create a [`TestResult`] for this test case.
fn create_result(&self, status: TestStatus) -> TestResult {
TestResult {
name: self.as_str().to_string(),
name: self.to_string(),
description: self.description(),
status,
}
Expand Down
100 changes: 21 additions & 79 deletions src/tests/plugin.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,31 +11,31 @@ mod params;
mod processing;
mod state;

const TEST_CATEGORY_FEATURES: &str = "features-categories";
const TEST_DUPLICATE_FEATURES: &str = "features-duplicates";
const TEST_BASIC_OUT_OF_PLACE_AUDIO_PROCESSING: &str = "process-audio-out-of-place-basic";
const TEST_BASIC_OUT_OF_PLACE_NOTE_PROCESSING: &str = "process-note-out-of-place-basic";
const TEST_INCONSISTENT_NOTE_PROCESSING: &str = "process-note-inconsistent";
const TEST_CONVERT_PARAMS: &str = "param-conversions";
const TEST_WRONG_NAMESPACE_SET_PARAMS: &str = "param-set-wrong-namespace";
const TEST_BASIC_STATE_REPRODUCIBILITY: &str = "state-reproducibility-basic";
const TEST_NULL_COOKIES_STATE_REPRODUCIBILITY: &str = "state-reproducibility-null-cookies";
const TEST_FLUSH_STATE_REPRODUCIBILITY: &str = "state-reproducibility-flush";
const TEST_BUFFERED_STATE_STREAMS: &str = "state-buffered-streams";

/// The tests for individual CLAP plugins. See the module's heading for more information, and the
/// `description` function below for a description of each test case.
#[derive(strum_macros::Display, strum_macros::EnumString, strum_macros::EnumIter)]
pub enum PluginTestCase {
#[strum(serialize = "features-categories")]
CategoryFeatures,
#[strum(serialize = "features-duplicates")]
DuplicateFeatures,
#[strum(serialize = "process-audio-out-of-place-basic")]
BasicOutOfPlaceAudioProcessing,
#[strum(serialize = "process-note-out-of-place-basic")]
BasicOutOfPlaceNoteProcessing,
#[strum(serialize = "process-note-inconsistent")]
InconsistentNoteProcessing,
#[strum(serialize = "param-conversions")]
ConvertParams,
#[strum(serialize = "param-set-wrong-namespace")]
WrongNamespaceSetParams,
#[strum(serialize = "state-reproducibility-basic")]
BasicStateReproducibility,
#[strum(serialize = "state-reproducibility-null-cookies")]
NullCookiesStateReproducibility,
#[strum(serialize = "state-reproducibility-flush")]
FlushStateReproducibility,
#[strum(serialize = "state-buffered-streams")]
BufferedStateStreams,
}

Expand All @@ -44,65 +44,6 @@ impl<'a> TestCase<'a> for PluginTestCase {
/// should be tested.
type TestArgs = (&'a PluginLibrary, &'a str);

const ALL: &'static [Self] = &[
PluginTestCase::CategoryFeatures,
PluginTestCase::DuplicateFeatures,
PluginTestCase::BasicOutOfPlaceAudioProcessing,
PluginTestCase::BasicOutOfPlaceNoteProcessing,
PluginTestCase::InconsistentNoteProcessing,
PluginTestCase::ConvertParams,
PluginTestCase::WrongNamespaceSetParams,
PluginTestCase::BasicStateReproducibility,
PluginTestCase::NullCookiesStateReproducibility,
PluginTestCase::FlushStateReproducibility,
PluginTestCase::BufferedStateStreams,
];

fn from_str(string: &str) -> Option<Self> {
match string {
TEST_CATEGORY_FEATURES => Some(PluginTestCase::CategoryFeatures),
TEST_DUPLICATE_FEATURES => Some(PluginTestCase::DuplicateFeatures),
TEST_BASIC_OUT_OF_PLACE_AUDIO_PROCESSING => {
Some(PluginTestCase::BasicOutOfPlaceAudioProcessing)
}
TEST_BASIC_OUT_OF_PLACE_NOTE_PROCESSING => {
Some(PluginTestCase::BasicOutOfPlaceNoteProcessing)
}
TEST_INCONSISTENT_NOTE_PROCESSING => Some(PluginTestCase::InconsistentNoteProcessing),
TEST_CONVERT_PARAMS => Some(PluginTestCase::ConvertParams),
TEST_WRONG_NAMESPACE_SET_PARAMS => Some(PluginTestCase::WrongNamespaceSetParams),
TEST_BASIC_STATE_REPRODUCIBILITY => Some(PluginTestCase::BasicStateReproducibility),
TEST_NULL_COOKIES_STATE_REPRODUCIBILITY => {
Some(PluginTestCase::NullCookiesStateReproducibility)
}
TEST_FLUSH_STATE_REPRODUCIBILITY => Some(PluginTestCase::FlushStateReproducibility),
TEST_BUFFERED_STATE_STREAMS => Some(PluginTestCase::BufferedStateStreams),
_ => None,
}
}

fn as_str(&self) -> &'static str {
match self {
PluginTestCase::CategoryFeatures => TEST_CATEGORY_FEATURES,
PluginTestCase::DuplicateFeatures => TEST_DUPLICATE_FEATURES,
PluginTestCase::BasicOutOfPlaceAudioProcessing => {
TEST_BASIC_OUT_OF_PLACE_AUDIO_PROCESSING
}
PluginTestCase::BasicOutOfPlaceNoteProcessing => {
TEST_BASIC_OUT_OF_PLACE_NOTE_PROCESSING
}
PluginTestCase::InconsistentNoteProcessing => TEST_INCONSISTENT_NOTE_PROCESSING,
PluginTestCase::ConvertParams => TEST_CONVERT_PARAMS,
PluginTestCase::WrongNamespaceSetParams => TEST_WRONG_NAMESPACE_SET_PARAMS,
PluginTestCase::BasicStateReproducibility => TEST_BASIC_STATE_REPRODUCIBILITY,
PluginTestCase::NullCookiesStateReproducibility => {
TEST_NULL_COOKIES_STATE_REPRODUCIBILITY
}
PluginTestCase::FlushStateReproducibility => TEST_FLUSH_STATE_REPRODUCIBILITY,
PluginTestCase::BufferedStateStreams => TEST_BUFFERED_STATE_STREAMS,
}
}

fn description(&self) -> String {
match self {
PluginTestCase::CategoryFeatures => String::from(
Expand Down Expand Up @@ -143,9 +84,10 @@ impl<'a> TestCase<'a> for PluginTestCase {
as before. The parameter values are updated using the process function.",
),
PluginTestCase::NullCookiesStateReproducibility => format!(
"The exact same test as {TEST_BASIC_STATE_REPRODUCIBILITY}, but with all cookies \
in the parameter events set to null pointers. The plugin should handle this in \
the same way as the other test case.",
"The exact same test as {}, but with all cookies in the parameter events set to \
null pointers. The plugin should handle this in the same way as the other test \
case.",
PluginTestCase::BasicStateReproducibility
),
PluginTestCase::FlushStateReproducibility => String::from(
"Randomizes a plugin's parameters, saves its state, recreates the plugin \
Expand All @@ -155,16 +97,16 @@ impl<'a> TestCase<'a> for PluginTestCase {
function to create the second state.",
),
PluginTestCase::BufferedStateStreams => format!(
"Performs the same state and parameter reproducibility check as in \
'{TEST_BASIC_STATE_REPRODUCIBILITY}', but this time the plugin is only allowed \
to read a small prime number of bytes at a time when reloading and resaving the \
state."
"Performs the same state and parameter reproducibility check as in '{}', but this \
time the plugin is only allowed to read a small prime number of bytes at a time \
when reloading and resaving the state.",
PluginTestCase::BasicStateReproducibility
),
}
}

fn set_out_of_process_args(&self, command: &mut Command, (library, plugin_id): Self::TestArgs) {
let test_name = self.as_str();
let test_name = self.to_string();

command
.arg(
Expand Down
37 changes: 5 additions & 32 deletions src/tests/plugin_library.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,52 +13,25 @@ use crate::plugin::library::PluginLibrary;

use super::{TestCase, TestResult, TestStatus};

const TEST_SCAN_TIME: &str = "scan-time";
const TEST_QUERY_NONEXISTENT_FACTORY: &str = "query-factory-nonexistent";
const TEST_CREATE_ID_WITH_TRAILING_GARBAGE: &str = "create-id-with-trailing-garbage";

const SCAN_TIME_LIMIT: Duration = Duration::from_millis(100);

/// Tests for entire CLAP libraries. These are mostly to ensure good plugin scanning practices. See
/// the module's heading for more information, and the `description` function below for a
/// description of each test case.
#[derive(strum_macros::Display, strum_macros::EnumString, strum_macros::EnumIter)]
pub enum PluginLibraryTestCase {
#[strum(serialize = "scan-time")]
ScanTime,
#[strum(serialize = "query-factory-nonexistent")]
QueryNonexistentFactory,
#[strum(serialize = "create-id-with-trailing-garbage")]
CreateIdWithTrailingGarbage,
}

impl<'a> TestCase<'a> for PluginLibraryTestCase {
/// The path to a CLAP plugin library.
type TestArgs = &'a Path;

const ALL: &'static [Self] = &[
PluginLibraryTestCase::ScanTime,
PluginLibraryTestCase::QueryNonexistentFactory,
PluginLibraryTestCase::CreateIdWithTrailingGarbage,
];

fn from_str(string: &str) -> Option<Self> {
match string {
TEST_SCAN_TIME => Some(PluginLibraryTestCase::ScanTime),
TEST_QUERY_NONEXISTENT_FACTORY => Some(PluginLibraryTestCase::QueryNonexistentFactory),
TEST_CREATE_ID_WITH_TRAILING_GARBAGE => {
Some(PluginLibraryTestCase::CreateIdWithTrailingGarbage)
}
_ => None,
}
}

fn as_str(&self) -> &'static str {
match self {
PluginLibraryTestCase::ScanTime => TEST_SCAN_TIME,
PluginLibraryTestCase::QueryNonexistentFactory => TEST_QUERY_NONEXISTENT_FACTORY,
PluginLibraryTestCase::CreateIdWithTrailingGarbage => {
TEST_CREATE_ID_WITH_TRAILING_GARBAGE
}
}
}

fn description(&self) -> String {
match self {
PluginLibraryTestCase::ScanTime => format!(
Expand All @@ -77,7 +50,7 @@ impl<'a> TestCase<'a> for PluginLibraryTestCase {
}

fn set_out_of_process_args(&self, command: &mut Command, library_path: Self::TestArgs) {
let test_name = self.as_str();
let test_name = self.to_string();

command
.arg(
Expand Down
27 changes: 15 additions & 12 deletions src/validator.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use serde::Serialize;
use std::collections::BTreeMap;
use std::fs;
use std::path::PathBuf;
use strum::IntoEnumIterator;

use crate::plugin::library::PluginLibrary;
use crate::tests::{PluginLibraryTestCase, PluginTestCase, TestCase, TestResult, TestStatus};
Expand Down Expand Up @@ -147,12 +148,11 @@ pub fn validate(settings: &ValidatorSettings) -> Result<ValidationResult> {
// mode makes a bit more sense. Otherwise we would be measuring plugin scanning time on
// libraries that may still be loaded in the process.
let mut plugin_library_results = Vec::new();
for test in PluginLibraryTestCase::ALL {
for test in PluginLibraryTestCase::iter() {
let test_name = test.to_string();
match (&test_filter_re, settings.invert_filter) {
(Some(test_filter_re), false) if !test_filter_re.is_match(test.as_str()) => {
continue
}
(Some(test_filter_re), true) if test_filter_re.is_match(test.as_str()) => continue,
(Some(test_filter_re), false) if !test_filter_re.is_match(&test_name) => continue,
(Some(test_filter_re), true) if test_filter_re.is_match(&test_name) => continue,
_ => (),
}

Expand Down Expand Up @@ -207,14 +207,13 @@ pub fn validate(settings: &ValidatorSettings) -> Result<ValidationResult> {
}

let mut plugin_test_results = Vec::new();
for test in PluginTestCase::ALL {
for test in PluginTestCase::iter() {
let test_name = test.to_string();
match (&test_filter_re, settings.invert_filter) {
(Some(test_filter_re), false) if !test_filter_re.is_match(test.as_str()) => {
continue
}
(Some(test_filter_re), true) if test_filter_re.is_match(test.as_str()) => {
(Some(test_filter_re), false) if !test_filter_re.is_match(&test_name) => {
continue
}
(Some(test_filter_re), true) if test_filter_re.is_match(&test_name) => continue,
_ => (),
}

Expand Down Expand Up @@ -248,15 +247,19 @@ pub fn validate(settings: &ValidatorSettings) -> Result<ValidationResult> {
pub fn run_single_test(settings: &SingleTestSettings) -> Result<()> {
let result = match settings.test_type {
SingleTestType::PluginLibrary => {
let test_case = PluginLibraryTestCase::from_str(&settings.name)
let test_case = settings
.name
.parse::<PluginLibraryTestCase>()
.with_context(|| format!("Unknown test name: {}", &settings.name))?;

test_case.run_in_process(&settings.path)
}
SingleTestType::Plugin => {
let plugin_library = PluginLibrary::load(&settings.path)
.with_context(|| format!("Could not load '{}'", settings.path.display()))?;
let test_case = PluginTestCase::from_str(&settings.name)
let test_case = settings
.name
.parse::<PluginTestCase>()
.with_context(|| format!("Unknown test name: {}", &settings.name))?;

test_case.run_in_process((&plugin_library, &settings.plugin_id))
Expand Down

0 comments on commit 42bb1bb

Please sign in to comment.