Code Review

Overview

This PR adds a CTest-based smoke test suite for the CLI target and fixes a real bug: outputSampleRate was never set when --sr was parsed without an --input file, so WAV headers generated by --sine --output always had the default 44100 Hz regardless of the requested rate. The fix is correct and minimal.

Bug Fix (IPlug/CLI/IPlugCLI_main.cpp)

Correct. The one-liner outputSampleRate = static_cast<int>(sampleRate) in the --sr branch mirrors the identical assignment that already exists in the --input/--process-file branch (line 446). The test test_generated_audio_outputs directly validates this fix by asserting sample_rate == 1000 in the WAV header.

CMake (CMakeLists.txt + Tests/CMakeLists.txt + Tests/CLI/CMakeLists.txt)

Issues:

• enable_testing() is redundant in Tests/CLI/CMakeLists.txt. include(CTest) at the root already calls enable_testing() for the whole tree. Calling it again in a subdirectory is harmless but unnecessary. When the file is used as a standalone build it is needed, but in that case the root include(CTest) is absent — so the two modes have inconsistent behavior. Consider guarding with if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR).

• cmake_minimum_required + project in Tests/CLI/CMakeLists.txt conflict with subdirectory use. CMake silently ignores cmake_minimum_required/project inside add_subdirectory, but having them there signals intent to also build standalone. The standalone path (cmake -S Tests/CLI) requires iPlug2 to be findable independently, which won't work in a fresh checkout without extra -DIPLUG2_DIR flags. The PR description says this was tested standalone, but there is no documentation or wrapper script for it. Either document the standalone invocation explicitly or remove the standalone design if it is not intended.

• Fixture ordering may cause a race on parallel CTest runs. The FIXTURES_SETUP / FIXTURES_REQUIRED mechanism should handle this, but IPlugCLITest.CLI.Build invokes cmake --build on ${CMAKE_BINARY_DIR} which is the build directory of the test project, not necessarily the directory that owns IPlugCLITest-cli when built as part of the main tree. Verify this resolves correctly when ctest is run from the main build tree.

Python Smoke Test (Tests/CLI/cli_smoke_test.py)

Correctness:

• The bug fix is directly exercised: test_generated_audio_outputs asserts sample_rate == 1000 in the WAV header. Good.
• test_parameters normalizes param 0 to 0.5 and expects value == 50.0, implicitly assuming a linear [0, 100] Gain range. If the example plugin ever adds a curve or changes the range, this breaks silently. A comment naming the assumption would help.

Style / minor issues:

# Deprecated in Python 3.9+; use the built-in tuple[] syntax instead
from typing import Tuple
def read_wav_params(path: Path) -> Tuple[int, int, int, int]:

Replace with -> tuple[int, int, int, int] (requires from __future__ import annotations already present, so this is safe on Python 3.7+).

def assert_close(actual: float, expected: float, tolerance: float = 1.0e-9) -> None:
    if not math.isclose(actual, expected, rel_tol=tolerance, abs_tol=tolerance):

Passing the same value as both rel_tol and abs_tol is unusual — for values near zero rel_tol is irrelevant, and for large values abs_tol may be too tight. Separate them or pick one appropriate strategy. For parameter round-trip at 12.5, abs_tol=1e-6 alone is more readable.

Test coverage gaps:

• No negative/error-path tests (e.g., missing executable, invalid param index, unreadable input file). These would catch regressions in CLI error handling.
• No validation of the generated audio content — only the WAV header is checked. Even a simple sanity check (non-zero RMS, or that a 100 Hz sine at 1000 Hz SR has the right period) would catch DSP-level regressions.
• test_midi_render only checks that 10 samples are written; it does not verify any audio was actually rendered (samples could all be zero).

Hardcoded plugin metadata:

assert info["name"] == "IPlugCLITest"
assert info["version"] == "1.0.0"
assert info["manufacturer"] == "AcmeInc"
assert [param["name"] for param in params] == ["Gain", "Frequency", "Attack", "Decay"]

These couple the test tightly to IPlugCLITest's current config.h. That is intentional (it is a regression test), but a comment saying so avoids confusion when someone updates the example.

Summary

The core fix and test infrastructure are solid. The items above are mostly polish — the most actionable ones are the enable_testing() guard and adding at least one negative-path test.