diff --git a/.github/actions/acceptance/acceptance.sh b/.github/actions/acceptance/acceptance.sh index c3fd9edd..acf5ad4e 100644 --- a/.github/actions/acceptance/acceptance.sh +++ b/.github/actions/acceptance/acceptance.sh @@ -39,6 +39,7 @@ for config in ${configs[@]+"${configs[@]}"}; do index=$((index + 1)) name="${config#"$config_dir"/}" report="$report_dir/$index-$(sanitise "$(basename "$config" .json)").xml" + rm -f "$report" # so an earlier run's report can't be read if this one writes none echo "::group::pluginval test: $name" diff --git a/.github/actions/validate/validate.sh b/.github/actions/validate/validate.sh index ef34dd52..461d36f5 100644 --- a/.github/actions/validate/validate.sh +++ b/.github/actions/validate/validate.sh @@ -28,6 +28,7 @@ for plugin in ${plugins[@]+"${plugins[@]}"}; do index=$((index + 1)) report="$report_dir/$index-$(sanitise "$(basename "$plugin")").xml" + rm -f "$report" # so an earlier run's report can't be read if this one writes none echo "::group::pluginval validate: $plugin" # INPUT_ARGS is deliberately unquoted so it splits into separate arguments. diff --git a/CLAUDE.md b/CLAUDE.md index dd52b437..f306bccb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -217,11 +217,11 @@ The command line is structured into subcommands, peeled off by a thin verb dispatcher in front of the settings pipeline (it does **not** use CLI11-native subcommands, so the env/config/CLI layering below is untouched): -- `pluginval validate [options] ` — the default; `` is a +- `pluginval validate [options] ` - the default; `` is a positional argument. `pluginval ` and `pluginval [options] ` (no verb) also resolve to validate. -- `pluginval run-tests` — runs the internal unit tests. -- `pluginval strictness-help [level]` — lists tests at a strictness level. +- `pluginval run-tests` - runs the internal unit tests. +- `pluginval strictness-help [level]` - lists tests at a strictness level. `settings_parser::dispatch()` (in `SettingsParser.cpp`) takes the `tokenise()`d command line and returns a `DispatchResult { command, validateTokens, @@ -248,7 +248,7 @@ that CLI11 binds to directly. A single instance is filled by successive layers, **lowest to highest precedence: defaults → environment → `--config` → CLI** (in `SettingsParser::parseTokens`): -1. **preprocess** the raw command line — rewrite the deprecated `strictnessLevel`, +1. **preprocess** the raw command line - rewrite the deprecated `strictnessLevel`, strip the macOS `-NSDocumentRevisionsDebugMode YES` flag, and insert an implicit `--validate` when the last argument is a bare plugin path. 2. **Environment layer.** Env-var names are *derived* from the registered @@ -267,7 +267,7 @@ uses a `CheckedTransformer`, and the hex/int seed is a small callback. `PluginTests::Options` at the boundary. Adding a new option is three edits: a struct member, an entry in the nlohmann -macro list, and one `add_option(...)` line — its environment variable then works +macro list, and one `add_option(...)` line - its environment variable then works automatically. `SettingsSerializer` handles JSON load/save plus the two remaining conversions (hex seed, disabled-tests file). @@ -279,7 +279,7 @@ are handled by CLI11 (auto usage + a footer with the env-var/commands notes). ### Acceptance Testing (`pluginval test`) A **parallel subsystem** to validate, in `source/acceptance/`. It answers "does -this plugin produce the expected output for a known input + state?" — a +this plugin produce the expected output for a known input + state?" - a deterministic *render + golden-file comparison*, not a unit-test pass/fail. Full spec: `tests/acceptance/Acceptance testing design.md`; end-user guide: `docs/Acceptance testing.md`. @@ -289,13 +289,13 @@ spec: `tests/acceptance/Acceptance testing design.md`; end-user guide: `DispatchResult::testConfigPath`, plus `recordMissing` / `timeoutMs` from `--record-missing` / `--timeout-ms` or `RECORD_MISSING` / `TIMEOUT_MS`, and `testPluginOverride` from `--plugin`, which replaces the config's `plugin` - in `runTestFile()` - so `plugin` is only required when `--plugin` is absent), + in `loadAndRunTestFile()` - so `plugin` is only required when `--plugin` is absent), `isCommandLine()` and `getFooterText()`; `CommandLine.cpp`'s `performCommandLine()` has a `Command::test` branch that starts an `acceptance::TestRunner`, which renders on a **background thread** (message thread left free; plugin deleted on it), with a watchdog for the timeout, and quits via `callAsync` when done. -- **Config**: `acceptance::TestConfig` (`TestConfig.cpp/h`) — std-typed struct, +- **Config**: `acceptance::TestConfig` (`TestConfig.cpp/h`) - std-typed struct, **snake_case JSON keys** mapped via explicit `to_json`/`from_json` (members stay camelCase). It is **independent** of `PluginvalSettings` / the `--config` layering: the test config is a positional argument loaded standalone. @@ -314,7 +314,7 @@ spec: `tests/acceptance/Acceptance testing design.md`; end-user guide: tolerance, default one 16-bit LSB = `1/32768`; `0` = bit-exact). Adding `spectrum`/`crosscorr`/etc. is one registry entry, no config/runner changes. - **Dogfood + self-tests**: three minimal deterministic `juce_add_plugin` targets - behind `PLUGINVAL_BUILD_TEST_PLUGINS` — `tests/test_plugins/tone_generator/` + behind `PLUGINVAL_BUILD_TEST_PLUGINS` - `tests/test_plugins/tone_generator/` (closed-form sine/square generator, phase resets on `prepareToPlay`), `tests/test_plugins/gain/` (a gain effect, dogfoods the `input.audio` path) and `tests/test_plugins/playhead_probe/` (writes the host transport to its output, @@ -352,8 +352,10 @@ log are unaffected. completion, so a crash leaves an error report rather than a stale or missing one. On a timeout the thread that ends the process writes a "timed out" error report as its last act (`PluginsUnitTestRunner`'s `onTimeout` for validate, the - acceptance `Watchdog` for test), so the report always agrees with the exit code - even if the tests finish in the moment before the process is terminated. + acceptance `Watchdog` for test). The two writers never overlap: validate stops + the timeout thread before writing the real report, and for test the worker and + the `Watchdog` each claim the report (an atomic flag) and only the first to + claim it writes, so the report always agrees with the exit code. ### Test Framework @@ -399,8 +401,8 @@ The VST3 validator (Steinberg's vstvalidator) is embedded into pluginval when bu 5. When the `VST3validator` test runs, it spawns the extracted validator as a subprocess **Key files:** -- `cmake/GenerateBinaryHeader.cmake` — binary-to-C-header conversion script -- `source/vst3validator/VST3ValidatorRunner.h/cpp` — extracts embedded binary, returns `juce::File` +- `cmake/GenerateBinaryHeader.cmake` - binary-to-C-header conversion script +- `source/vst3validator/VST3ValidatorRunner.h/cpp` - extracts embedded binary, returns `juce::File` **Disabling embedded validator:** ```bash @@ -449,6 +451,8 @@ Located in `TestUtilities.h`: - `countNaNs()`, `countInfs()`, `countSubnormals()` - Audio validation - `ScopedEditorShower` - RAII editor creation/destruction - `callPrepareToPlayOnMessageThreadIfVST3()` - VST3-safe lifecycle +- `callOnMessageThreadAndWait()` - Run code on the message thread and wait; rethrows its exceptions on the caller +- `ChildProcessOutputDecoder` - Decode child-process output read in chunks without splitting UTF-8 characters - `ScopedAllocationDisabler` - Detect allocations in audio thread ### Real-time Safety Checking @@ -573,7 +577,7 @@ Without the actions: ## Dependencies ### External -- **JUCE** (v8.0.x) - Audio application framework (git submodule) +- **JUCE** (v9.0.x) - Audio application framework (git submodule) - **magic_enum** (v0.9.7) - Enum reflection (fetched via CPM) - **CLI11** (v2.6.2) - CLI argument parsing, header-only (fetched via CPM) - **nlohmann/json** (3.12.0) - JSON settings layering/serialisation (fetched via CPM) diff --git a/source/CommandLineTests.cpp b/source/CommandLineTests.cpp index 869fe2c8..ad562017 100644 --- a/source/CommandLineTests.cpp +++ b/source/CommandLineTests.cpp @@ -586,6 +586,32 @@ struct CommandLineTests : public juce::UnitTest } } + beginTest ("Subcommand: test options in --name=value form"); + { + { + const auto d = settings_parser::dispatch (settings_parser::tokenise ("test --timeout-ms=500 --junit-xml=out/r.xml --json-results=out/r.json --plugin=build/P.vst3 config.json")); + expect (d.error.isEmpty()); + expectEquals (d.timeoutMs, (juce::int64) 500); + expectEquals (d.junitXmlPath, juce::String ("out/r.xml")); + expectEquals (d.jsonResultsPath, juce::String ("out/r.json")); + expectEquals (d.testPluginOverride, juce::String ("build/P.vst3")); + expectEquals (d.testConfigPath, juce::String ("config.json")); + } + { + const auto d = settings_parser::dispatch (settings_parser::tokenise ("test config.json --plugin=")); + expect (d.error.contains ("--plugin")); + } + { + const auto d = settings_parser::dispatch (settings_parser::tokenise ("test --output-dir=out config.json")); + expect (d.error.contains ("--output-dir")); + } + { + // A flag doesn't take a value, so the value isn't taken as the config path + const auto d = settings_parser::dispatch (settings_parser::tokenise ("test --record-missing=1 config.json")); + expect (d.error.contains ("--record-missing")); + } + } + beginTest ("Report writer: JUnit XML"); { report_writer::Report report; diff --git a/source/CrashHandler.cpp b/source/CrashHandler.cpp index d56c8937..f0b8c2b8 100644 --- a/source/CrashHandler.cpp +++ b/source/CrashHandler.cpp @@ -17,7 +17,9 @@ #if ! JUCE_WINDOWS #include "native/pluginval_native_posix.h" - static const auto crashLogPath = "/tmp/pluginval_crash.txt"; + + // Set by initialiseCrashHandler(), as the crash handler can't allocate to build it + static char crashLogPath[1024] {}; #endif namespace @@ -29,11 +31,7 @@ namespace static juce::File getCrashTraceFile() { - #if JUCE_WINDOWS return juce::File::getSpecialLocation (juce::File::tempDirectory).getChildFile ("pluginval_crash.txt"); - #else - return juce::File (crashLogPath); - #endif } #if JUCE_WINDOWS @@ -66,7 +64,13 @@ namespace void initialiseCrashHandler() { // Delete the crash file, this will be created if possible - getCrashTraceFile().deleteFile(); + const auto crashTraceFile = getCrashTraceFile(); + crashTraceFile.deleteFile(); + + #if ! JUCE_WINDOWS + crashTraceFile.getFullPathName().copyToUTF8 (crashLogPath, sizeof (crashLogPath)); + #endif + juce::SystemStats::setApplicationCrashHandler (handleCrash); } diff --git a/source/MainComponent.cpp b/source/MainComponent.cpp index c5a2f7c8..6a0f3d83 100644 --- a/source/MainComponent.cpp +++ b/source/MainComponent.cpp @@ -259,7 +259,7 @@ namespace void showOutputDirDialog() { - juce::String message = TRANS("Set a desintation directory to place log files"); + juce::String message = TRANS("Set a destination directory to place log files"); auto dir = getOutputDir(); if (dir.getFullPathName().isNotEmpty()) diff --git a/source/PluginTests.cpp b/source/PluginTests.cpp index e685dad6..7a34d940 100644 --- a/source/PluginTests.cpp +++ b/source/PluginTests.cpp @@ -117,13 +117,10 @@ void PluginTests::runTest() { beginTest ("Scan for plugins located in: " + fileOrID); - juce::WaitableEvent completionEvent; - juce::MessageManager::callAsync ([&, this]() mutable - { - knownPluginList.scanAndAddDragAndDroppedFiles (formatManager, juce::StringArray (fileOrID), typesFound); - completionEvent.signal(); - }); - completionEvent.wait(); + callOnMessageThreadAndWait ([&] + { + knownPluginList.scanAndAddDragAndDroppedFiles (formatManager, juce::StringArray (fileOrID), typesFound); + }); logMessage ("Num plugins found: " + juce::String (typesFound.size())); expect (! typesFound.isEmpty(), @@ -212,13 +209,7 @@ void PluginTests::testType (const juce::PluginDescription& pd) if (t->needsToRunOnMessageThread()) { - juce::WaitableEvent completionEvent; - juce::MessageManager::callAsync ([&, this]() mutable - { - t->runTest (*this, *instance); - completionEvent.signal(); - }); - completionEvent.wait(); + callOnMessageThreadAndWait ([&] { t->runTest (*this, *instance); }); } else { diff --git a/source/ReportWriter.h b/source/ReportWriter.h index 86268d7e..3e02210c 100644 --- a/source/ReportWriter.h +++ b/source/ReportWriter.h @@ -75,4 +75,4 @@ namespace report_writer failed run). */ bool write (const Report&, const juce::File& junitXml, const juce::File& jsonResults); -} +} // namespace report_writer diff --git a/source/SettingsParser.cpp b/source/SettingsParser.cpp index d8105eb0..eb6ab8f2 100644 --- a/source/SettingsParser.cpp +++ b/source/SettingsParser.cpp @@ -393,17 +393,41 @@ Precedence (lowest to highest): defaults, environment variables, --config, comma // The options, plus the positional acceptance-test config (the first // non-option token after the verb). + // Accept "--name=value" as well as "--name value", as validate (CLI11) does + juce::StringArray tokens; + for (int i = 1; i < tokensIn.size(); ++i) { const auto& token = tokensIn.getReference (i); + if (token.startsWith ("--record-missing=")) + { + result.error = "--record-missing doesn't take a value (got \"" + token + "\")"; + return result; + } + + if (token.startsWith ("--") && token.contains ("=")) + { + tokens.add (token.upToFirstOccurrenceOf ("=", false, false)); + tokens.add (token.fromFirstOccurrenceOf ("=", false, false)); + } + else + { + tokens.add (token); + } + } + + for (int i = 0; i < tokens.size(); ++i) + { + const auto& token = tokens.getReference (i); + if (token == "--record-missing") { result.recordMissing = true; } else if (token == "--timeout-ms") { - const auto value = i + 1 < tokensIn.size() ? tokensIn.getReference (++i) : juce::String(); + const auto value = i + 1 < tokens.size() ? tokens.getReference (++i) : juce::String(); const auto digits = value.startsWith ("-") ? value.substring (1) : value; if (digits.isEmpty() || ! digits.containsOnly ("0123456789")) @@ -416,7 +440,7 @@ Precedence (lowest to highest): defaults, environment variables, --config, comma } else if (token == "--junit-xml" || token == "--json-results") { - const auto value = i + 1 < tokensIn.size() ? tokensIn.getReference (++i) : juce::String(); + const auto value = i + 1 < tokens.size() ? tokens.getReference (++i) : juce::String(); if (value.isEmpty() || value.startsWith ("--")) { @@ -428,7 +452,7 @@ Precedence (lowest to highest): defaults, environment variables, --config, comma } else if (token == "--plugin") { - const auto value = i + 1 < tokensIn.size() ? tokensIn.getReference (++i) : juce::String(); + const auto value = i + 1 < tokens.size() ? tokens.getReference (++i) : juce::String(); if (value.isEmpty() || value.startsWith ("--")) { @@ -599,4 +623,4 @@ Precedence (lowest to highest): defaults, environment variables, --config, comma args.addArray ({ "validate", "--config-base64", b64, fileOrID }); return args; } -} +} // namespace settings_parser diff --git a/source/SettingsParser.h b/source/SettingsParser.h index 6896050b..42130333 100644 --- a/source/SettingsParser.h +++ b/source/SettingsParser.h @@ -119,4 +119,4 @@ namespace settings_parser /** The "--version" output string. */ juce::String getVersionString(); -} +} // namespace settings_parser diff --git a/source/SettingsSerializer.cpp b/source/SettingsSerializer.cpp index a390b4de..e056fc51 100644 --- a/source/SettingsSerializer.cpp +++ b/source/SettingsSerializer.cpp @@ -36,7 +36,7 @@ namespace settings_serializer //============================================================================== std::int64_t parseRandomSeed (const juce::String& raw) { - if (! raw.containsOnly ("x-0123456789acbdef")) + if (! raw.containsOnly ("x-0123456789abcdef")) throw std::invalid_argument ("Invalid random seed argument!"); if (raw.startsWith ("0x")) @@ -65,4 +65,4 @@ namespace settings_serializer return out; } -} +} // namespace settings_serializer diff --git a/source/SettingsSerializer.h b/source/SettingsSerializer.h index a5c9336b..18ffa28c 100644 --- a/source/SettingsSerializer.h +++ b/source/SettingsSerializer.h @@ -47,4 +47,4 @@ namespace settings_serializer the value is treated as a comma-separated list. */ std::vector disabledTestsToList (const juce::String& raw); -} +} // namespace settings_serializer diff --git a/source/TestUtilities.cpp b/source/TestUtilities.cpp index 63ceda46..5e6c691b 100644 --- a/source/TestUtilities.cpp +++ b/source/TestUtilities.cpp @@ -28,22 +28,6 @@ inline bool logAllocationViolationIfNotAllowed() return true; } -inline void violationError (const std::string& text) -{ - switch (AllocatorInterceptor::getViolationBehaviour()) - { - case AllocatorInterceptor::ViolationBehaviour::logToCerr: - std::cerr << text; - break; - case AllocatorInterceptor::ViolationBehaviour::throwException: - throw; - break; - case AllocatorInterceptor::ViolationBehaviour::none: - default: - break; - } -} - inline bool throwIfRequiredAndReturnShouldLog() { switch (AllocatorInterceptor::getViolationBehaviour()) @@ -60,6 +44,14 @@ inline bool throwIfRequiredAndReturnShouldLog() return false; } +/** The deallocation version: operator delete is noexcept, so a throw from it + would terminate the process. Violations are still counted, just not thrown. +*/ +inline bool shouldLogDeallocationViolation() +{ + return AllocatorInterceptor::getViolationBehaviour() == AllocatorInterceptor::ViolationBehaviour::logToCerr; +} + //============================================================================== #if JUCE_CLANG #define ATTRIBUTE_USED __attribute__((used)) @@ -73,7 +65,11 @@ ATTRIBUTE_USED void* operator new (std::size_t sz) if (throwIfRequiredAndReturnShouldLog()) std::cerr << "!!! WARNING: Illegal allocation of " << sz << " bytes\n"; - return std::malloc (sz); + // operator new must never return null (malloc (0) may) + if (auto ptr = std::malloc (sz > 0 ? sz : 1)) + return ptr; + + throw std::bad_alloc(); } ATTRIBUTE_USED void* operator new[] (std::size_t sz) @@ -82,13 +78,17 @@ ATTRIBUTE_USED void* operator new[] (std::size_t sz) if (throwIfRequiredAndReturnShouldLog()) std::cerr << "!!! WARNING: Illegal array allocation of " << sz << " bytes\n"; - return std::malloc (sz); + // operator new must never return null (malloc (0) may) + if (auto ptr = std::malloc (sz > 0 ? sz : 1)) + return ptr; + + throw std::bad_alloc(); } ATTRIBUTE_USED void operator delete (void* ptr) noexcept { if (! logAllocationViolationIfNotAllowed()) - if (throwIfRequiredAndReturnShouldLog()) + if (shouldLogDeallocationViolation()) std::cerr << "!!! WARNING: Illegal deletion\n"; std::free (ptr); @@ -97,7 +97,7 @@ ATTRIBUTE_USED void operator delete (void* ptr) noexcept ATTRIBUTE_USED void operator delete[] (void* ptr) noexcept { if (! logAllocationViolationIfNotAllowed()) - if (throwIfRequiredAndReturnShouldLog()) + if (shouldLogDeallocationViolation()) std::cerr << "!!! WARNING: Illegal array deletion\n"; std::free (ptr); @@ -107,7 +107,7 @@ ATTRIBUTE_USED void operator delete[] (void* ptr) noexcept void operator delete (void* ptr, size_t) noexcept { if (! logAllocationViolationIfNotAllowed()) - if (throwIfRequiredAndReturnShouldLog()) + if (shouldLogDeallocationViolation()) std::cerr << "!!! WARNING: Illegal deletion\n"; std::free (ptr); @@ -116,7 +116,7 @@ void operator delete (void* ptr, size_t) noexcept void operator delete[] (void* ptr, size_t) noexcept { if (! logAllocationViolationIfNotAllowed()) - if (throwIfRequiredAndReturnShouldLog()) + if (shouldLogDeallocationViolation()) std::cerr << "!!! WARNING: Illegal array deletion\n"; std::free (ptr); @@ -253,3 +253,45 @@ struct AllocatorInterceptorTests : public juce::UnitTest, }; static AllocatorInterceptorTests allocatorInterceptorTests; + + +//============================================================================== +struct ChildProcessOutputDecoderTests : public juce::UnitTest +{ + ChildProcessOutputDecoderTests() + : juce::UnitTest ("ChildProcessOutputDecoder", "pluginval") + { + } + + void runTest() override + { + beginTest ("Complete text passes straight through"); + { + ChildProcessOutputDecoder decoder; + expectEquals (juce::String (decoder.decode ("abc", 3)), juce::String ("abc")); + } + + beginTest ("A character split across reads is held back until complete"); + { + const std::string text = "a\xe2\x82\xac" "b"; // "a€b", the euro sign is 3 bytes + + for (size_t split = 1; split < text.size(); ++split) + { + ChildProcessOutputDecoder decoder; + const auto first = decoder.decode (text.data(), split); + const auto second = decoder.decode (text.data() + split, text.size() - split); + + expect (first == text.substr (0, split < 4 ? 1 : split)); + expect (first + second == text); + } + } + + beginTest ("Invalid bytes aren't held back"); + { + ChildProcessOutputDecoder decoder; + expect (decoder.decode ("a\xff", 2) == "a\xff"); + } + } +}; + +static ChildProcessOutputDecoderTests childProcessOutputDecoderTests; diff --git a/source/TestUtilities.h b/source/TestUtilities.h index 148760c8..4e6a0fdb 100644 --- a/source/TestUtilities.h +++ b/source/TestUtilities.h @@ -16,6 +16,8 @@ #include "juce_audio_processors/juce_audio_processors.h" +#include + //============================================================================== struct StopwatchTimer { @@ -151,15 +153,105 @@ static inline float getParametersSum (juce::AudioPluginInstance& instance) //============================================================================== +/** Turns a child process's output, read in arbitrary chunks, into text. A UTF-8 + character split across two reads is held back until the rest arrives, rather + than producing an invalid string. +*/ +struct ChildProcessOutputDecoder +{ + /** Adds the bytes read and returns the complete text so far. */ + std::string decode (const char* data, size_t numBytes) + { + pending.append (data, numBytes); + + const auto numComplete = getNumCompleteBytes (pending); + auto text = pending.substr (0, numComplete); + pending.erase (0, numComplete); + return text; + } + + /** The number of leading bytes that don't end in a partial UTF-8 character. */ + static size_t getNumCompleteBytes (const std::string& bytes) + { + // A character is at most 4 bytes, so only its lead byte in the last 4 matters + for (size_t i = bytes.size(), numChecked = 0; i > 0 && numChecked < 4; --i, ++numChecked) + { + const auto byte = (unsigned char) bytes[i - 1]; + + if ((byte & 0xc0) == 0x80) + continue; // a continuation byte + + const size_t length = byte < 0x80 ? 1 + : (byte & 0xe0) == 0xc0 ? 2 + : (byte & 0xf0) == 0xe0 ? 3 + : (byte & 0xf8) == 0xf0 ? 4 + : 1; // invalid, so pass it on + + return i - 1 + length <= bytes.size() ? bytes.size() : i - 1; + } + + return bytes.size(); + } + +private: + std::string pending; +}; + +//============================================================================== +/** Runs fn on the message thread (directly if already on it) and waits for it to + finish. An exception thrown by fn is rethrown here, so the caller is always + released rather than left waiting forever. +*/ +template +void callOnMessageThreadAndWait (Fn&& fn) +{ + if (juce::MessageManager::getInstance()->isThisTheMessageThread()) + { + fn(); + return; + } + + juce::WaitableEvent waiter; + std::exception_ptr error; + + juce::MessageManager::callAsync ([&] + { + try + { + fn(); + } + catch (...) + { + error = std::current_exception(); + } + + waiter.signal(); + }); + waiter.wait(); + + if (error) + std::rethrow_exception (error); +} + +/** Calls fn on the message thread if the plugin is a VST3, otherwise on this thread. */ +template +void callOnMessageThreadIfVST3 (juce::AudioPluginInstance& instance, Fn&& fn) +{ + if (instance.getPluginDescription().pluginFormatName != "VST3") + fn(); + else + callOnMessageThreadAndWait (fn); +} + //============================================================================== static std::unique_ptr createAndShowEditorOnMessageThread (juce::AudioPluginInstance& instance) { std::unique_ptr editor; - if (juce::MessageManager::getInstance()->isThisTheMessageThread()) + callOnMessageThreadAndWait ([&] { if (! instance.hasEditor()) - return {}; + return; jassert (instance.getActiveEditor() == nullptr); editor.reset (instance.createEditorAndMakeActive()); @@ -175,39 +267,20 @@ static std::unique_ptr createAndShowEditorOnMessageT juce::MessageManager::getInstance()->runDispatchLoopUntil (200); #endif } - } - else - { - juce::WaitableEvent waiter; - juce::MessageManager::callAsync ([&] - { - editor = createAndShowEditorOnMessageThread (instance); - waiter.signal(); - }); - waiter.wait(); - } + }); return editor; } static void deleteEditorOnMessageThread (std::unique_ptr editor) { - if (juce::MessageManager::getInstance()->isThisTheMessageThread()) + callOnMessageThreadAndWait ([&] { - editor->processor.editorBeingDeleted (editor.get()); - editor.reset(); - return; - } + if (editor != nullptr) + editor->processor.editorBeingDeleted (editor.get()); - juce::WaitableEvent waiter; - juce::MessageManager::callAsync ([&] - { - if (editor != nullptr) - editor->processor.editorBeingDeleted (editor.get()); - editor.reset(); - waiter.signal(); - }); - waiter.wait(); + editor.reset(); + }); } //============================================================================== @@ -215,80 +288,24 @@ static void deleteEditorOnMessageThread (std::unique_ptrisThisTheMessageThread()) - { - instance.prepareToPlay (sampleRate, blockSize); - return; - } - - juce::WaitableEvent waiter; - juce::MessageManager::callAsync ([&] - { - instance.prepareToPlay (sampleRate, blockSize); - waiter.signal(); - }); - waiter.wait(); + callOnMessageThreadIfVST3 (instance, [&] { instance.prepareToPlay (sampleRate, blockSize); }); } inline void callReleaseResourcesOnMessageThreadIfVST3 (juce::AudioPluginInstance& instance) { - if (instance.getPluginDescription().pluginFormatName != "VST3" - || juce::MessageManager::getInstance()->isThisTheMessageThread()) - { - instance.releaseResources(); - return; - } - - juce::WaitableEvent waiter; - juce::MessageManager::callAsync ([&] - { - instance.releaseResources(); - waiter.signal(); - }); - waiter.wait(); + callOnMessageThreadIfVST3 (instance, [&] { instance.releaseResources(); }); } inline juce::MemoryBlock callGetStateInformationOnMessageThreadIfVST3 (juce::AudioPluginInstance& instance) { juce::MemoryBlock state; - - if (instance.getPluginDescription().pluginFormatName != "VST3" - || juce::MessageManager::getInstance()->isThisTheMessageThread()) - { - instance.getStateInformation (state); - } - else - { - juce::WaitableEvent waiter; - juce::MessageManager::callAsync ([&] - { - instance.getStateInformation (state); - waiter.signal(); - }); - waiter.wait(); - } - + callOnMessageThreadIfVST3 (instance, [&] { instance.getStateInformation (state); }); return state; } inline void callSetStateInformationOnMessageThreadIfVST3 (juce::AudioPluginInstance& instance, const juce::MemoryBlock& state) { - if (instance.getPluginDescription().pluginFormatName != "VST3" - || juce::MessageManager::getInstance()->isThisTheMessageThread()) - { - instance.setStateInformation (state.getData(), (int) state.getSize()); - } - else - { - juce::WaitableEvent waiter; - juce::MessageManager::callAsync ([&] - { - instance.setStateInformation (state.getData(), (int) state.getSize()); - waiter.signal(); - }); - waiter.wait(); - } + callOnMessageThreadIfVST3 (instance, [&] { instance.setStateInformation (state.getData(), (int) state.getSize()); }); } //============================================================================== diff --git a/source/Validator.cpp b/source/Validator.cpp index 6e4b2620..7019d037 100644 --- a/source/Validator.cpp +++ b/source/Validator.cpp @@ -17,9 +17,11 @@ #include "CrashHandler.h" #include "CommandLine.h" #include "ReportWriter.h" +#include "TestUtilities.h" #include #include #include +#include //============================================================================== struct PluginsUnitTestRunner : public juce::UnitTestRunner, @@ -53,6 +55,15 @@ struct PluginsUnitTestRunner : public juce::UnitTestRunner, return outputStream.get(); } + /** Stops the timeout thread. Call once the tests have finished so a timeout + can't fire (and write its report) while the results are being written. + If the timeout has already fired, this blocks until the process ends. + */ + void stopTimeoutThread() + { + stopThread (5000); + } + void resultsUpdated() override { resetTimeout(); @@ -67,8 +78,13 @@ struct PluginsUnitTestRunner : public juce::UnitTestRunner, if (message.isNotEmpty()) { - if (outputStream) - *outputStream << message << "\n"; + { + // The timeout thread can reset the stream at any point + const std::scoped_lock sl (outputStreamLock); + + if (outputStream) + *outputStream << message << "\n"; + } callback (message + "\n"); } @@ -78,6 +94,7 @@ struct PluginsUnitTestRunner : public juce::UnitTestRunner, std::function callback; std::function onTimeout; // Called on the timeout thread just before the process is terminated std::unique_ptr outputStream; + std::mutex outputStreamLock; const juce::int64 timeoutMs = -1; std::atomic timoutTime { -1 }; std::atomic canSendLogMessage { true }; @@ -95,7 +112,11 @@ struct PluginsUnitTestRunner : public juce::UnitTestRunner, { logMessage ("*** FAILED: Timeout after " + juce::RelativeTime::milliseconds (timeoutMs).getDescription()); canSendLogMessage = false; - outputStream.reset(); + + { + const std::scoped_lock sl (outputStreamLock); + outputStream.reset(); + } // Give the log a second to flush the message before terminating juce::Thread::sleep (1000); @@ -143,7 +164,8 @@ static juce::File getDestinationFile (PluginTests& test) { if (juce::String() == filename) return {}; - return juce::File(filename); + + return juce::File::getCurrentWorkingDirectory().getChildFile (filename); } if (dir.existsAsFile() || ! dir.createDirectory()) @@ -182,7 +204,7 @@ static void updateFileNameIfPossible (PluginTests& test, PluginsUnitTestRunner& { const auto sourceFile = os->getFile(); const auto filename = test.getOptions().outputFilename; - const auto destName = (filename == juce::String())?getFileNameFromDescription (test) : filename; + const auto destName = (filename == juce::String()) ? getFileNameFromDescription (test) : filename; if (destName.isEmpty() || sourceFile.getFileName() == destName) return; @@ -249,8 +271,8 @@ inline juce::Array runTests (PluginTests& test options.junitXmlFile, options.jsonResultsFile); }; - // Written last on a timeout, so the report always agrees with the exit code - // (the tests can finish in the moment between the timeout and the process ending) + // Written by the timeout thread just before it ends the process. The thread + // is stopped before the final report is written, so the two never overlap. PluginsUnitTestRunner testRunner (std::move (callback), createDestinationFileStream (test), options.timeoutMs, [options, writeIncompleteReport] { @@ -264,6 +286,7 @@ inline juce::Array runTests (PluginTests& test juce::Array testsToRun; testsToRun.add (&test); testRunner.runTests (testsToRun, options.randomSeed); + testRunner.stopTimeoutThread(); for (int i = 0; i < testRunner.getNumResults(); ++i) results.add (*testRunner.getResult (i)); @@ -285,9 +308,9 @@ inline juce::Array runTests (PluginTests& test const int failures = getNumFailures (results); if (! failures) - testRunner.logMessage("SUCCESS"); + testRunner.logMessage ("SUCCESS"); else - testRunner.logMessage("FAILURE"); + testRunner.logMessage ("FAILURE"); return results; } @@ -365,6 +388,8 @@ class ChildProcessValidator // Flush the output from the process + ChildProcessOutputDecoder outputDecoder; + for (;;) { for (;;) @@ -374,10 +399,10 @@ class ChildProcessValidator if (const auto numBytesRead = childProcess.readProcessOutput (buffer, sizeof (buffer)); numBytesRead > 0) { - std::string msg (buffer, (size_t) numBytesRead); + const auto msg = outputDecoder.decode (buffer, (size_t) numBytesRead); - if (outputGenerated) - outputGenerated (msg); + if (outputGenerated && ! msg.empty()) + outputGenerated (juce::String::fromUTF8 (msg.data(), (int) msg.size())); } else { diff --git a/source/acceptance/AcceptanceTest.cpp b/source/acceptance/AcceptanceTest.cpp index 9b8e2b2e..a135ff02 100644 --- a/source/acceptance/AcceptanceTest.cpp +++ b/source/acceptance/AcceptanceTest.cpp @@ -20,6 +20,7 @@ #define XXH_INLINE_ALL #include +#include #include #include #include @@ -110,19 +111,7 @@ namespace /** Deletes the plugin on the message thread, as hosts are expected to. */ void deleteOnMessageThread (std::unique_ptr instance) { - if (juce::MessageManager::getInstance()->isThisTheMessageThread()) - { - instance.reset(); - return; - } - - juce::WaitableEvent deleted; - juce::MessageManager::callAsync ([&] - { - instance.reset(); - deleted.signal(); - }); - deleted.wait(); + callOnMessageThreadAndWait ([&] { instance.reset(); }); } /** Owns the plugin for the duration of the render. On the way out (including @@ -135,12 +124,20 @@ namespace if (instance == nullptr) return; - instance->setPlayHead (nullptr); + // Can be called while an exception unwinds the render, so nothing may escape + try + { + instance->setPlayHead (nullptr); - if (prepared) - callReleaseResourcesOnMessageThreadIfVST3 (*instance); + if (prepared) + callReleaseResourcesOnMessageThreadIfVST3 (*instance); - deleteOnMessageThread (std::move (instance)); + deleteOnMessageThread (std::move (instance)); + } + catch (...) + { + std::cout << "!!! WARNING: The plugin threw an exception while being released" << std::endl; + } } std::unique_ptr instance; @@ -591,25 +588,15 @@ TestResult runTest (const TestConfig& config, const RunOptions& options) } //============================================================================== -int runTestFile (const juce::File& configFile, const RunOptions& options) +static void writeIncompleteReport (const juce::File& configFile, const RunOptions& options) { - const auto finish = [&options] (const TestResult& result) - { - // A report that was asked for but couldn't be written is a failure - if (! reporter::report (result, options.junitXmlFile, options.jsonResultsFile)) - { - std::cout << "*** FAILED: Unable to write the report file(s)" << std::endl; - return 1; - } - - return reporter::exitCode (result); - }; - - // Written up front so a crash can't leave a stale (or missing) report behind report_writer::write (report_writer::makeIncomplete ("test", configFile.getFileNameWithoutExtension(), "Test did not complete (crashed)"), options.junitXmlFile, options.jsonResultsFile); +} +static TestResult loadAndRunTestFile (const juce::File& configFile, const RunOptions& options) +{ std::vector configs; try @@ -618,11 +605,11 @@ int runTestFile (const juce::File& configFile, const RunOptions& options) } catch (const std::exception& e) { - return finish (TestResult::makeError (configFile.getFileNameWithoutExtension(), e.what())); + return TestResult::makeError (configFile.getFileNameWithoutExtension(), e.what()); } if (configs.empty()) - return finish (TestResult::makeError (configFile.getFileNameWithoutExtension(), "no test configs in file")); + return TestResult::makeError (configFile.getFileNameWithoutExtension(), "no test configs in file"); // v1 runs the first entry only; the parser already accepts arrays for the // phase-2 multiplexing extension. @@ -635,12 +622,31 @@ int runTestFile (const juce::File& configFile, const RunOptions& options) config.plugin = options.pluginOverride; if (config.plugin.empty()) - return finish (TestResult::makeError (config.getName(), "plugin is required (in the config, or via --plugin)")); + return TestResult::makeError (config.getName(), "plugin is required (in the config, or via --plugin)"); const auto startTime = juce::Time::getMillisecondCounterHiRes(); auto result = runTest (config, options); result.durationSeconds = (juce::Time::getMillisecondCounterHiRes() - startTime) / 1000.0; - return finish (result); + return result; +} + +static int reportResult (const TestResult& result, const RunOptions& options) +{ + // A report that was asked for but couldn't be written is a failure + if (! reporter::report (result, options.junitXmlFile, options.jsonResultsFile)) + { + std::cout << "*** FAILED: Unable to write the report file(s)" << std::endl; + return 1; + } + + return reporter::exitCode (result); +} + +int runTestFile (const juce::File& configFile, const RunOptions& options) +{ + // Written up front so a crash can't leave a stale (or missing) report behind + writeIncompleteReport (configFile, options); + return reportResult (loadAndRunTestFile (configFile, options), options); } //============================================================================== @@ -654,25 +660,36 @@ struct TestRunner::Watchdog : private juce::Thread ~Watchdog() override { - markFinished(); + finished.signal(); stopThread (5000); } - void markFinished() + /** Called by the test thread before it writes the final report. Returns + false if the timeout got there first, in which case the watchdog is + writing the timed-out report and is about to end the process. + */ + bool claimReport() { finished.signal(); + return ! reportClaimed.exchange (true); } private: const juce::File configFile; const RunOptions options; juce::WaitableEvent finished; + std::atomic reportClaimed { false }; void run() override { if (finished.wait ((double) options.timeoutMs)) return; + // The test finished in the moment before the timeout and is writing its + // own report, so let it complete normally + if (reportClaimed.exchange (true)) + return; + // The worker is stuck in a plugin call that can't be interrupted, so // report the failure and end the process. auto result = TestResult::makeError (configFile.getFileNameWithoutExtension(), @@ -693,6 +710,10 @@ TestRunner::TestRunner (const juce::File& file, const RunOptions& opts, std::fun { jassert (onComplete); + // Written up front so a crash can't leave a stale (or missing) report + // behind. Done before the watchdog starts so it can't overwrite a timeout. + writeIncompleteReport (configFile, options); + if (options.timeoutMs > 0) watchdog = std::make_unique (configFile, options); @@ -707,10 +728,11 @@ TestRunner::~TestRunner() void TestRunner::run() { - const auto exitCode = runTestFile (configFile, options); + const auto result = loadAndRunTestFile (configFile, options); - if (watchdog != nullptr) - watchdog->markFinished(); + // If the watchdog has already claimed the report it writes it and ends the + // process, so don't write over it + const auto exitCode = (watchdog == nullptr || watchdog->claimReport()) ? reportResult (result, options) : 1; juce::MessageManager::callAsync ([callback = onComplete, exitCode] { callback (exitCode); }); } diff --git a/source/acceptance/TestConfig.cpp b/source/acceptance/TestConfig.cpp index 3f13495b..42c024fc 100644 --- a/source/acceptance/TestConfig.cpp +++ b/source/acceptance/TestConfig.cpp @@ -13,6 +13,7 @@ ==============================================================================*/ #include "TestConfig.h" +#include "../SettingsParser.h" #include #include @@ -183,15 +184,8 @@ nlohmann::ordered_json TestConfig::getComparison() const juce::String TestConfig::getPluginPathOrID() const { - const juce::String raw (plugin); - - // Resolve relative/home paths against the working directory; leave absolute - // paths and bare component IDs (no '.' or '~') untouched. Mirrors - // settings_parser::resolvePluginPath. - if (raw.contains ("~") || raw.contains (".")) - return juce::File::getCurrentWorkingDirectory().getChildFile (raw).getFullPathName(); - - return raw; + // Resolved against the working directory, as for validate + return settings_parser::resolvePluginPath (juce::String (plugin)); } juce::File TestConfig::resolveAgainstConfigDir (const std::string& path) const diff --git a/source/acceptance/TestReporter.h b/source/acceptance/TestReporter.h index 4939fc32..dc7525a7 100644 --- a/source/acceptance/TestReporter.h +++ b/source/acceptance/TestReporter.h @@ -76,6 +76,6 @@ namespace reporter /** 0 for referenceCreated / passed, 1 for failed / error. */ int exitCode (const TestResult&); -} +} // namespace reporter } // namespace acceptance diff --git a/source/native/pluginval_native_posix.h b/source/native/pluginval_native_posix.h index 1e5cbd40..57d165c9 100644 --- a/source/native/pluginval_native_posix.h +++ b/source/native/pluginval_native_posix.h @@ -16,11 +16,13 @@ #if __has_include() +#include #include #include #include #include #include +#include #include #ifdef __printflike @@ -52,7 +54,7 @@ inline void writeStackTrace (const char* filePath) { // On Linux & Mac this is a signal handler, and therefore only "async-signal-safe" functions should be used. // This means nothing that uses malloc (juce::File, juce::String, std::string, std::vector etc.) or buffered I/O. - int fd = open (filePath, O_RDWR | O_CREAT | O_TRUNC, 00644); + int fd = open (filePath, O_RDWR | O_CREAT | O_TRUNC | O_NOFOLLOW, 00644); // Write stack traces and images { @@ -81,7 +83,12 @@ inline void writeStackTrace (const char* filePath) const char* imageName = info.dli_fname ? strrchr (info.dli_fname, '/') : nullptr; - if (imageName) + if (imageName == nullptr) + { + // Not a path (or no file name at all), so use what there is + imageName = info.dli_fname ? info.dli_fname : "?"; + } + else { ++imageName; @@ -98,8 +105,8 @@ inline void writeStackTrace (const char* filePath) if (info.dli_saddr) { ptrdiff_t offset = static_cast (stacktrace[i]) - static_cast (info.dli_saddr); - writeAsyncSignalSafe (fd, "%-3d %-35s %p %s + %ld\n", i, imageName, stacktrace[i], info.dli_sname, - offset); + writeAsyncSignalSafe (fd, "%-3d %-35s %p %s + %ld\n", i, imageName, stacktrace[i], + info.dli_sname ? info.dli_sname : "?", (long) offset); } else { diff --git a/source/tests/BasicTests.cpp b/source/tests/BasicTests.cpp index 83416a8c..c332b3b8 100644 --- a/source/tests/BasicTests.cpp +++ b/source/tests/BasicTests.cpp @@ -382,20 +382,20 @@ struct PluginStateTestRestoration : public PluginTest // restored, so restore once here to start from the plugin's own values callSetStateInformationOnMessageThreadIfVST3 (instance, originalState); - const auto tolaratedDiff = 0.1f; + const auto toleratedDiff = 0.1f; // Set random parameter values - for (auto parameter : getNonBypassAutomatableParameters(instance)) + for (auto parameter : getNonBypassAutomatableParameters (instance)) { - const auto originalValue = parameter->getValue(); + const auto originalValue = parameter->getValue(); parameter->setValue (getRandomParameterValue (r, *parameter, ut.getOptions().strictnessLevel)); // Restore original state - callSetStateInformationOnMessageThreadIfVST3(instance, originalState); + callSetStateInformationOnMessageThreadIfVST3 (instance, originalState); // Check parameter values return to original - ut.expectWithinAbsoluteError(parameter->getValue(), originalValue, tolaratedDiff, - parameter->getName(1024) + juce::String(" not restored on setStateInformation")); + ut.expectWithinAbsoluteError (parameter->getValue(), originalValue, toleratedDiff, + parameter->getName (1024) + juce::String (" not restored on setStateInformation")); } if (ut.getOptions().strictnessLevel >= 8) @@ -897,6 +897,7 @@ struct AUvalTest : public PluginTest return; juce::MemoryOutputStream outputBuffer; + ChildProcessOutputDecoder outputDecoder; for (;;) { @@ -907,9 +908,9 @@ struct AUvalTest : public PluginTest if (const auto numBytesRead = cp.readProcessOutput (buffer, sizeof (buffer)); numBytesRead > 0) { - std::string msg (buffer, (size_t) numBytesRead); + const auto msg = juce::String::fromUTF8 (outputDecoder.decode (buffer, (size_t) numBytesRead).c_str()); ut.logVerboseMessage (msg); - outputBuffer << juce::String (msg); + outputBuffer << msg; } else { @@ -993,6 +994,7 @@ struct VST3validator : public PluginTest return; juce::MemoryOutputStream outputBuffer; + ChildProcessOutputDecoder outputDecoder; for (;;) { @@ -1003,9 +1005,9 @@ struct VST3validator : public PluginTest if (const auto numBytesRead = cp.readProcessOutput (buffer, sizeof (buffer)); numBytesRead > 0) { - std::string msg (buffer, (size_t) numBytesRead); + const auto msg = juce::String::fromUTF8 (outputDecoder.decode (buffer, (size_t) numBytesRead).c_str()); ut.logVerboseMessage (msg); - outputBuffer << juce::String (msg); + outputBuffer << msg; } else { diff --git a/source/tests/BusTests.cpp b/source/tests/BusTests.cpp index fc40bfe5..420ed69e 100644 --- a/source/tests/BusTests.cpp +++ b/source/tests/BusTests.cpp @@ -125,7 +125,7 @@ static BasicBusTest basicBusTest; sidechain) channel layouts and calls prepareToPlay / processBlock for each. This is intended to catch crashes that only occur for non-default channel - counts — typically a plugin that hardcodes assumptions about its sidechain + counts - typically a plugin that hardcodes assumptions about its sidechain being stereo and then accesses out-of-range channels when the host reconfigures it to mono. auval exercises this with its "1 Channel Test" but pluginval's other tests only process at the plugin's default layout. diff --git a/source/vst3validator/VST3ValidatorRunner.cpp b/source/vst3validator/VST3ValidatorRunner.cpp index c5c0711c..8475bb6c 100644 --- a/source/vst3validator/VST3ValidatorRunner.cpp +++ b/source/vst3validator/VST3ValidatorRunner.cpp @@ -30,12 +30,16 @@ std::unique_ptr getValidatorExecutable() #endif auto exe = tempFile->getFile(); - exe.replaceWithData (vstvalidator_binary, vstvalidator_binary_len); + bool ok = exe.replaceWithData (vstvalidator_binary, vstvalidator_binary_len); #if ! JUCE_WINDOWS - chmod (exe.getFullPathName().toRawUTF8(), 0755); + ok = ok && chmod (exe.getFullPathName().toRawUTF8(), 0755) == 0; #endif + // Callers check the file exists, so remove anything that isn't usable + if (! ok) + exe.deleteFile(); + return tempFile; } diff --git a/tests/acceptance/Acceptance testing design.md b/tests/acceptance/Acceptance testing design.md index 27ee2daf..a40b12f1 100644 --- a/tests/acceptance/Acceptance testing design.md +++ b/tests/acceptance/Acceptance testing design.md @@ -88,11 +88,11 @@ Required edits (small and localised): | Location | Edit | |---|---| -| `SettingsParser.h` — `enum class Command` | add `test` | -| `SettingsParser.cpp` — `dispatch()` | recognise the verb `test`, peel it, capture the positional config path(s) | -| `SettingsParser.cpp` — `isCommandLine()` | recognise `test` so CLI mode is triggered | -| `SettingsParser.cpp` — `getFooterText()` | add `test` to the `Commands:` help block | -| `CommandLine.cpp` — `performCommandLine()` | add a `Command::test` branch that runs the acceptance runner and async-quits | +| `SettingsParser.h` - `enum class Command` | add `test` | +| `SettingsParser.cpp` - `dispatch()` | recognise the verb `test`, peel it, capture the positional config path(s) | +| `SettingsParser.cpp` - `isCommandLine()` | recognise `test` so CLI mode is triggered | +| `SettingsParser.cpp` - `getFooterText()` | add `test` to the `Commands:` help block | +| `CommandLine.cpp` - `performCommandLine()` | add a `Command::test` branch that runs the acceptance runner and async-quits | Because `test` is brand new there are **no deprecated-alias** concerns. @@ -193,7 +193,7 @@ struct Comparator ``` - A small **registry** maps method name -> factory. -- `config` is whatever sits under the comparator's key — a bare number +- `config` is whatever sits under the comparator's key - a bare number (`1e-6`) for `sample`, or an object for richer methods (e.g. `"spectrum": { "tolerance": 0.05, "fftSize": 2048 }`). - When multiple methods are listed, **all must pass** (logical AND); each is @@ -224,9 +224,9 @@ The `spectrum` / `crosscorr` / `fingerprint` methods are what make the A reference is **two files**: -1. **`.wav`** — the rendered output as a **32-bit float WAV** (preserves +1. **`.wav`** - the rendered output as a **32-bit float WAV** (preserves full precision; it is the source of truth for comparison). -2. **`.wav.json`** — a sidecar manifest: +2. **`.wav.json`** - a sidecar manifest: ```jsonc { @@ -240,7 +240,7 @@ A reference is **two files**: } ``` -- `created_on` is **informational only** — it is never used for matching, because +- `created_on` is **informational only** - it is never used for matching, because references are meant to be portable and shared. - `config_hash` lets the runner detect a **stale** reference (one produced from a different config than the one now being run) and warn. It is an XXH3-64 hash @@ -287,7 +287,7 @@ following the existing test style. ## 8. Dogfood test plugin (tone generator) To develop and self-test the acceptance feature we need a **device under test -that is fully deterministic** — something whose output is known exactly, doesn't +that is fully deterministic** - something whose output is known exactly, doesn't depend on a third-party binary, and is stable across runs and platforms. We build a tiny in-repo **tone-generator plugin** for this purpose ("dogfooding" the feature with our own plugin). @@ -298,9 +298,9 @@ A minimal JUCE plugin (built with `juce_add_plugin`) that synthesises a simple, deterministic tone: - **Parameters** (so we can exercise state / parameter application): - - `waveform` — choice: `sine`, `square` (extendable to `saw`, `triangle`). - - `frequency` — Hz (e.g. 20–20000, default 440). - - `gain` — linear or dB (default −6 dB). + - `waveform` - choice: `sine`, `square` (extendable to `saw`, `triangle`). + - `frequency` - Hz (e.g. 20-20000, default 440). + - `gain` - linear or dB (default −6 dB). - **Determinism**: the oscillator **phase resets to 0 on `prepareToPlay`** and the waveform is computed in closed form from the sample index, so the same config always renders the identical buffer (ideal for the `sample` @@ -347,24 +347,24 @@ paths and the input/reference directories are substituted at CMake configure time (the tests can't hardcode a build-tree plugin path). Each `pluginval test ` CTest case asserts exit code `0` against the checked-in reference. Because the dogfood plugins are closed-form deterministic, these references are -stable enough to commit — a first smoke test of cross-platform portability for +stable enough to commit - a first smoke test of cross-platform portability for the `sample` comparator and the baseline for future comparators (`spectrum`, `crosscorr`, …). The five cases cover the distinct render paths: -- **`sine-440` / `square-220`** — the `state.parameters` (name/index → normalised +- **`sine-440` / `square-220`** - the `state.parameters` (name/index → normalised value) path. `square-220` compares bit-exact (`"sample": 0`). -- **`square-state`** — the binary `state.file` (`setStateInformation`) path, +- **`square-state`** - the binary `state.file` (`setStateInformation`) path, plus a `state.parameters` `gain` override on top, exercising the state-then-parameters precedence of §4. The blob is captured once and checked in alongside its reference WAV. -- **`gain-half`** — the **`input.audio`** effect path: a second dogfood plugin +- **`gain-half`** - the **`input.audio`** effect path: a second dogfood plugin (`tests/test_plugins/gain/`, a deterministic gain effect) gains a checked-in full-height (±1.0) sine by 0.5 and is compared bit-exact (0.5 is exact in float). It also omits `render_duration`, so the render length is derived from the input file. -- **`playhead-120`** — the **`playhead`** path: a third dogfood plugin +- **`playhead-120`** - the **`playhead`** path: a third dogfood plugin (`tests/test_plugins/playhead_probe/`) writes the host transport into its output (channel 0 = `ppqPosition`, channel 1 = tempo), so the recorded reference is a direct check that the fixed transport reached the plugin. If the