From 0d872acce5e4a0fc75ae7428bc863c3f1e4bc9f9 Mon Sep 17 00:00:00 2001 From: Bailey Pumfleet Date: Mon, 2 Nov 2026 11:18:09 -0600 Subject: [PATCH 01/10] Watch the vendored engines' upstreams weekly - scripts/upstream_behind.sh lists the upstream commits a product's vendored engine is missing (on upstream main since the pin in products//UPSTREAM), using upstream_common.sh's fetch. It only fetches; nothing else changes. - Upstream Watch (Mondays 06:17 UTC, or by hand) runs it for every products/*/UPSTREAM and keeps one tracking issue, labelled upstream-watch: created when an engine falls behind, its table rewritten each week, a comment only when the counts change, and closed once every engine is current. Commit subjects sit in code spans so upstream's @mentions and #numbers don't ping or link anything here. Permissions are contents: read and issues: write; it never changes code or opens pull requests. - Today: compressor 1 commit behind, eq current, limiter 24. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/upstream_watch.yml | 105 +++++++++++++++++++++++++++ scripts/upstream_behind.sh | 20 +++++ 2 files changed, 125 insertions(+) create mode 100644 .github/workflows/upstream_watch.yml create mode 100755 scripts/upstream_behind.sh diff --git a/.github/workflows/upstream_watch.yml b/.github/workflows/upstream_watch.yml new file mode 100644 index 00000000..00b0c988 --- /dev/null +++ b/.github/workflows/upstream_watch.yml @@ -0,0 +1,105 @@ +name: Upstream Watch + +# How far each vendored engine (products/*/UPSTREAM) is behind its upstream repository's main branch. +# Keeps a single tracking issue (labelled upstream-watch) up to date and closes it once every engine is current. +# It only reports: it never changes code or opens pull requests (moving a pin is scripts/sync_upstream.sh's job). + +on: + schedule: + - cron: "17 6 * * 1" # Mondays, 06:17 UTC + workflow_dispatch: + +permissions: + contents: read + issues: write + +concurrency: + group: ${{ github.workflow }} + cancel-in-progress: false + +env: + ISSUE_LABEL: upstream-watch + ISSUE_TITLE: Vendored engines are behind upstream + +jobs: + report: + name: Compare pins with upstream + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v7 + + - name: List upstream commits since each pin + shell: bash + run: | + set -euo pipefail + run_url="${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" + marker="" behind_total=0 table="" details="" + for pin in products/*/UPSTREAM; do + product=$(basename "$(dirname "$pin")") + repo=$(bash -c 'source "$1"; echo "${REPO%.git}"' _ "$pin") + commit=$(bash -c 'source "$1"; echo "$COMMIT"' _ "$pin") + scripts/upstream_behind.sh "$product" > "behind-$product.tsv" + count=$(wc -l < "behind-$product.tsv" | tr -d ' ') + behind_total=$((behind_total + count)) + marker+=" $product=$count" + if [ "$count" -eq 0 ]; then + behind="current" + else + noun=$([ "$count" -eq 1 ] && echo commit || echo commits) + behind="[$count $noun]($repo/compare/$commit...main)" + details+=$'\n'"
$product: $count $noun"$'\n\n' + # subjects go in code spans so upstream's @mentions and #numbers don't ping or link anything here + while IFS=$'\t' read -r sha date subject; do + subject=$(printf '%s' "$subject" | tr '`' "'") + details+="- [\`${sha:0:7}\`]($repo/commit/$sha) $date \`$subject\`"$'\n' + done < "behind-$product.tsv" + details+=$'\n'"
"$'\n' + fi + table+="| $product | [${repo#https://github.com/}]($repo) | [\`${commit:0:7}\`]($repo/commit/$commit) | $behind |"$'\n' + done + { + echo "" + echo "Commits on each vendored engine's upstream \`main\` since its pin in \`products//UPSTREAM\`," + echo "as of $(date -u +%Y-%m-%d) ([run]($run_url))." + echo + echo "| Product | Upstream | Pinned at | Behind |" + echo "|---|---|---|---|" + printf '%s' "$table" + printf '%s' "$details" + echo + echo "To take them: \`scripts/sync_upstream.sh \`, port the changes it lists to the files the product" + echo "adapted, then build and test. This issue is updated weekly by the Upstream Watch workflow and closed once" + echo "every engine is current." + } > report.md + cat report.md >> "$GITHUB_STEP_SUMMARY" + echo "BEHIND_TOTAL=$behind_total" >> "$GITHUB_ENV" + + - name: Update the tracking issue + shell: bash + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_REPO: ${{ github.repository }} + run: | + set -euo pipefail + number=$(gh issue list --label "$ISSUE_LABEL" --state open --limit 1 --json number --jq '.[0].number // empty') + if [ "$BEHIND_TOTAL" -eq 0 ]; then + if [ -n "$number" ]; then + gh issue close "$number" --comment "All vendored engines are current with upstream as of $(date -u +%Y-%m-%d)." + fi + echo "All vendored engines are current with upstream." + exit 0 + fi + if [ -z "$number" ]; then + gh label create "$ISSUE_LABEL" --color FBCA04 --description "Vendored engines behind upstream" --force + gh issue create --title "$ISSUE_TITLE" --label "$ISSUE_LABEL" --body-file report.md + exit 0 + fi + previous=$(gh issue view "$number" --json body --jq '.body' | head -n 1 | tr -d '\r') + current=$(head -n 1 report.md) + gh issue edit "$number" --title "$ISSUE_TITLE" --body-file report.md + # a comment (which notifies) only when the counts changed, not every week + if [ "$previous" != "$current" ]; then + counts=$(echo "$current" | sed -E 's//\1/') + gh issue comment "$number" --body "Upstream moved: now behind by $counts. The table above is updated." + fi diff --git a/scripts/upstream_behind.sh b/scripts/upstream_behind.sh new file mode 100755 index 00000000..44e44a37 --- /dev/null +++ b/scripts/upstream_behind.sh @@ -0,0 +1,20 @@ +#!/usr/bin/env bash +# Lists the upstream commits a product's vendored engine is missing: those on the upstream ref since the pinned commit. +# scripts/upstream_behind.sh [ref] (ref: upstream branch or tag; default main) +# Prints one tab-separated line per commit, newest first: ; nothing when the pin is current. +# Changes nothing (it only fetches); CI's weekly upstream watch runs it for every products/*/UPSTREAM. +set -euo pipefail +source "$(dirname "$0")/upstream_common.sh" +ref="${2:-main}" + +if [ -z "$COMMIT" ]; then + echo "$product's engine has not been imported yet: run scripts/sync_upstream.sh $product " >&2 + exit 1 +fi +pin="$(fetch_upstream "$COMMIT")" +head="$(fetch_upstream "$ref")" + +if ! git merge-base --is-ancestor "$pin" "$head"; then + echo "warning: $product's pin ${pin:0:12} is not on upstream $ref (was upstream's history rewritten?)" >&2 +fi +git --no-pager log --format='%H%x09%as%x09%s' "$pin..$head" From 4a447c548ca697eec6253b08f008f067ff974922 Mon Sep 17 00:00:00 2001 From: Bailey Pumfleet Date: Mon, 2 Nov 2026 11:18:15 -0600 Subject: [PATCH 02/10] Run the sanitizers and heavy tests weekly on every product - Sanitizer runs Saturdays and Heavy Test Sundays, both 02:23 UTC; both keep workflow_dispatch with its product input. On a schedule there is no input, which used to mean -DALTVANE_PRODUCTS= (nothing built) and a missing .env. - A small products job now picks what to test: the product asked for, or on the schedule every products/*/CMakeLists.txt. build_and_test runs over product x runner, skipping runners a product's PLATFORMS file rules out (Dialogue on Linux), as the top-level CMakeLists.txt does. The existing include entries are unchanged; the runner names are listed once more in the products job, with the platform each builds on. - Each workflow's concurrency group now includes the workflow name: with ${{ github.ref }} alone, a scheduled run on main and a push to main (or the other scheduled workflow) cancelled each other. - What they build and test is unchanged. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/cmake_heavy_test.yml | 64 +++++++++++++++++++++++--- .github/workflows/sanitizer.yml | 64 +++++++++++++++++++++++--- 2 files changed, 114 insertions(+), 14 deletions(-) diff --git a/.github/workflows/cmake_heavy_test.yml b/.github/workflows/cmake_heavy_test.yml index 42ad08ab..85b8f4e1 100644 --- a/.github/workflows/cmake_heavy_test.yml +++ b/.github/workflows/cmake_heavy_test.yml @@ -1,6 +1,8 @@ name: macOS/Windows/Linux Heavy Test on: + schedule: + - cron: "23 2 * * 0" # Sundays, 02:23 UTC, every product (see the products job) workflow_dispatch: inputs: product: @@ -8,9 +10,10 @@ on: required: true default: eq -# When pushing new commits, cancel any running builds on that branch +# When pushing new commits, cancel any running builds on that branch (of this workflow: the scheduled run on main must +# not cancel, or be cancelled by, another workflow's run there) concurrency: - group: ${{ github.ref }} + group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true permissions: @@ -29,12 +32,59 @@ env: CMAKE_CXX_COMPILER_LAUNCHER: sccache jobs: + products: + # what to test: the product asked for or, on the schedule, every product; minus the runners a product can't build + # on (products//PLATFORMS, read as the top-level CMakeLists.txt does) + name: Products + runs-on: ubuntu-latest + outputs: + products: ${{ steps.list.outputs.products }} + names: ${{ steps.list.outputs.names }} + exclude: ${{ steps.list.outputs.exclude }} + steps: + - name: Checkout code + uses: actions/checkout@v7 + + - name: List products and runners + id: list + shell: bash + env: + PRODUCT: ${{ inputs.product }} + # each build_and_test entry's name and the platform it builds on, as - + RUNNERS: '{"Linux": "Linux-x86_64", "macOS": "Darwin-arm64", "Windows": "Windows-AMD64"}' + run: | + python3 - <<'EOF' + import json, os, pathlib + runners = json.loads(os.environ["RUNNERS"]) + if os.environ["PRODUCT"]: + products = [os.environ["PRODUCT"]] + else: + products = sorted(p.parent.name for p in pathlib.Path("products").glob("*/CMakeLists.txt")) + exclude = [] + for product in products: + platforms_file = pathlib.Path("products", product, "PLATFORMS") + if platforms_file.exists(): + lines = [line.strip() for line in platforms_file.read_text().splitlines()] + platforms = {line for line in lines if line and not line.startswith("#")} + exclude += [{"product": product, "name": name} for name, platform in runners.items() + if platform not in platforms] + with open(os.environ["GITHUB_OUTPUT"], "a") as out: + out.write(f"products={json.dumps(products)}\n") + out.write(f"names={json.dumps(list(runners))}\n") + out.write(f"exclude={json.dumps(exclude)}\n") + print("products:", products, "\nskipped:", exclude) + EOF + build_and_test: - name: ${{ matrix.name }} + name: ${{ matrix.name }} (${{ matrix.product }}) + needs: products runs-on: ${{ matrix.os }} strategy: fail-fast: false matrix: + product: ${{ fromJSON(needs.products.outputs.products) }} + name: ${{ fromJSON(needs.products.outputs.names) }} + exclude: ${{ fromJSON(needs.products.outputs.exclude) }} include: - name: Linux os: ubuntu-latest @@ -80,7 +130,7 @@ jobs: if: runner.os != 'Windows' shell: bash run: | - cmake -B ${{ env.BUILD_DIR }} -G Ninja -DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE}} -DALTVANE_PRODUCTS=${{ inputs.product }} ${{ matrix.cmake_extra_flags }} . + cmake -B ${{ env.BUILD_DIR }} -G Ninja -DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE}} -DALTVANE_PRODUCTS=${{ matrix.product }} ${{ matrix.cmake_extra_flags }} . cmake --build ${{ env.BUILD_DIR }} --config ${{ env.BUILD_TYPE }} - name: Configure and Build (Windows) @@ -89,7 +139,7 @@ jobs: run: | for /f "usebackq tokens=*" %%i in (`"%ProgramFiles(x86)%\Microsoft Visual Studio\Installer\vswhere.exe" -latest -property installationPath`) do set VS_PATH=%%i call "%VS_PATH%\VC\Auxiliary\Build\${{ matrix.vcvars_script }}" - cmake -B ${{ env.BUILD_DIR }} -G Ninja -DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE}} -DALTVANE_PRODUCTS=${{ inputs.product }} ${{ matrix.cmake_extra_flags }} . + cmake -B ${{ env.BUILD_DIR }} -G Ninja -DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE}} -DALTVANE_PRODUCTS=${{ matrix.product }} ${{ matrix.cmake_extra_flags }} . cmake --build ${{ env.BUILD_DIR }} --config ${{ env.BUILD_TYPE }} - name: Compiler cache stats @@ -99,8 +149,8 @@ jobs: - name: Read in the product's .env from CMake # written by altvane_add_plugin shell: bash run: | - cat "${{ env.BUILD_DIR }}/${{ inputs.product }}.env" # show us the config - cat "${{ env.BUILD_DIR }}/${{ inputs.product }}.env" >> $GITHUB_ENV # pull in our PRODUCT_NAME, etc + cat "${{ env.BUILD_DIR }}/${{ matrix.product }}.env" # show us the config + cat "${{ env.BUILD_DIR }}/${{ matrix.product }}.env" >> $GITHUB_ENV # pull in our PRODUCT_NAME, etc - name: Pluginval setup shell: bash diff --git a/.github/workflows/sanitizer.yml b/.github/workflows/sanitizer.yml index cd7502e8..b97d76c1 100644 --- a/.github/workflows/sanitizer.yml +++ b/.github/workflows/sanitizer.yml @@ -1,6 +1,8 @@ name: Sanitizer on: + schedule: + - cron: "23 2 * * 6" # Saturdays, 02:23 UTC, every product (see the products job) workflow_dispatch: inputs: product: @@ -8,9 +10,10 @@ on: required: true default: eq -# When pushing new commits, cancel any running builds on that branch +# When pushing new commits, cancel any running builds on that branch (of this workflow: the scheduled run on main must +# not cancel, or be cancelled by, another workflow's run there) concurrency: - group: ${{ github.ref }} + group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true permissions: @@ -29,12 +32,59 @@ env: CMAKE_CXX_COMPILER_LAUNCHER: sccache jobs: + products: + # what to test: the product asked for or, on the schedule, every product; minus the runners a product can't build + # on (products//PLATFORMS, read as the top-level CMakeLists.txt does) + name: Products + runs-on: ubuntu-latest + outputs: + products: ${{ steps.list.outputs.products }} + names: ${{ steps.list.outputs.names }} + exclude: ${{ steps.list.outputs.exclude }} + steps: + - name: Checkout code + uses: actions/checkout@v7 + + - name: List products and runners + id: list + shell: bash + env: + PRODUCT: ${{ inputs.product }} + # each build_and_test entry's name and the platform it builds on, as - + RUNNERS: '{"macOS Address Sanitizer": "Darwin-arm64", "macOS Thread Sanitizer": "Darwin-arm64", "Linux Address Sanitizer": "Linux-x86_64", "Linux Thread Sanitizer": "Linux-x86_64"}' + run: | + python3 - <<'EOF' + import json, os, pathlib + runners = json.loads(os.environ["RUNNERS"]) + if os.environ["PRODUCT"]: + products = [os.environ["PRODUCT"]] + else: + products = sorted(p.parent.name for p in pathlib.Path("products").glob("*/CMakeLists.txt")) + exclude = [] + for product in products: + platforms_file = pathlib.Path("products", product, "PLATFORMS") + if platforms_file.exists(): + lines = [line.strip() for line in platforms_file.read_text().splitlines()] + platforms = {line for line in lines if line and not line.startswith("#")} + exclude += [{"product": product, "name": name} for name, platform in runners.items() + if platform not in platforms] + with open(os.environ["GITHUB_OUTPUT"], "a") as out: + out.write(f"products={json.dumps(products)}\n") + out.write(f"names={json.dumps(list(runners))}\n") + out.write(f"exclude={json.dumps(exclude)}\n") + print("products:", products, "\nskipped:", exclude) + EOF + build_and_test: - name: ${{ matrix.name }} + name: ${{ matrix.name }} (${{ matrix.product }}) + needs: products runs-on: ${{ matrix.os }} strategy: fail-fast: false matrix: + product: ${{ fromJSON(needs.products.outputs.products) }} + name: ${{ fromJSON(needs.products.outputs.names) }} + exclude: ${{ fromJSON(needs.products.outputs.exclude) }} include: - name: macOS Address Sanitizer os: macos-latest @@ -86,7 +136,7 @@ jobs: if: runner.os != 'Windows' shell: bash run: | - cmake -B ${{ env.BUILD_DIR }} -G Ninja -DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE}} -DALTVANE_PRODUCTS=${{ inputs.product }} -DALTVANE_LTO=OFF ${{ matrix.cmake_extra_flags }} . + cmake -B ${{ env.BUILD_DIR }} -G Ninja -DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE}} -DALTVANE_PRODUCTS=${{ matrix.product }} -DALTVANE_LTO=OFF ${{ matrix.cmake_extra_flags }} . cmake --build ${{ env.BUILD_DIR }} --config ${{ env.BUILD_TYPE }} - name: Configure and Build (Windows) @@ -94,7 +144,7 @@ jobs: shell: cmd run: | call ${{ matrix.vcvars64_path }} - cmake -B ${{ env.BUILD_DIR }} -G Ninja -DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE}} -DALTVANE_PRODUCTS=${{ inputs.product }} -DALTVANE_LTO=OFF ${{ matrix.cmake_extra_flags }} . + cmake -B ${{ env.BUILD_DIR }} -G Ninja -DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE}} -DALTVANE_PRODUCTS=${{ matrix.product }} -DALTVANE_LTO=OFF ${{ matrix.cmake_extra_flags }} . cmake --build ${{ env.BUILD_DIR }} --config ${{ env.BUILD_TYPE }} - name: Compiler cache stats @@ -104,8 +154,8 @@ jobs: - name: Read in the product's .env from CMake # written by altvane_add_plugin shell: bash run: | - cat "${{ env.BUILD_DIR }}/${{ inputs.product }}.env" # show us the config - cat "${{ env.BUILD_DIR }}/${{ inputs.product }}.env" >> $GITHUB_ENV # pull in our PRODUCT_NAME, etc + cat "${{ env.BUILD_DIR }}/${{ matrix.product }}.env" # show us the config + cat "${{ env.BUILD_DIR }}/${{ matrix.product }}.env" >> $GITHUB_ENV # pull in our PRODUCT_NAME, etc - name: Checkout pluginval code uses: actions/checkout@v7 From f3276f047c5459958ebcf2530c1b13edae543ec4 Mon Sep 17 00:00:00 2001 From: Bailey Pumfleet Date: Mon, 2 Nov 2026 11:28:31 -0600 Subject: [PATCH 03/10] Add tests for libs/ui altvane_ui is shared by all six products but had no tests of its own. AltvaneUITests (libs/ui/tests, built with ALTVANE_BUILD_TESTS like the products' tests) is a Catch2 console app registered with CTest, so CI's existing ctest step runs it on Linux, macOS and Windows. altvane_ui compiles into whatever links it, so the app builds it and its JUCE modules itself, under a stand-in plugin name ("Altvane UI Tests"). A fake processor (five parameters: float, skewed float, choice, bool, and a "meter" left out of presets), its Session and a ParamEditor stand in for a product. Covered: - value_format: Hz/kHz, gain signs and the typographic minus, -0 and -inf, q, ms, %, note names with cents; parsing units, either minus, a decimal comma, note names (sharps, flats, negative octaves), junk, infinities; formatted values parse back; a decimal-comma C locale changes nothing (skipped where no such locale is installed). - ParamEditor: setNow, continuous edits, cancel, clamping and snapping, toggle, undo/redo order and the redo branch, undo mid-edit, a new edit ending the open one, an editor destroyed mid-edit; one host gesture per parameter throughout. - bindValue: typing into a Knob's text entry (through the real TextEditor and message loop), out-of-range and junk text, escape, accessibility setValueAsString, and a wheel's late on_end not ending another control's edit. - Session: A/B switching both ways, the first switch copying A, history cleared, copy to the other slot, an out-of-range slot; UISettings round trip, clamping and apply(). - PresetManager in a temporary folder: factory presets (groups, once per version, never over user edits, without a settings file), save/load, modified tracking (ignoring excluded parameters), load ending an open edit and clearing history, six kinds of corrupt file and a missing one, saving over a name, rename (favourite and current follow, a name collision, empty and illegal names, case only), groups, natural order, favourites, step with wrap-around, initialise, the JSON codec. - SpscQueue (including across threads), TriBuffer, FreqScale, GainScale. PresetManager::setDirectoryOverride lets the tests keep presets in a temporary folder; nothing else calls it. Co-Authored-By: Claude Opus 5.5 --- libs/ui/CMakeLists.txt | 4 + .../altvane_ui/session/preset_manager.cpp | 14 + .../altvane_ui/session/preset_manager.hpp | 5 + libs/ui/tests/CMakeLists.txt | 37 ++ libs/ui/tests/fake_processor.hpp | 136 ++++++ libs/ui/tests/juce_init.cpp | 33 ++ libs/ui/tests/param_editor_tests.cpp | 370 +++++++++++++++ libs/ui/tests/preset_manager_tests.cpp | 445 ++++++++++++++++++ libs/ui/tests/session_tests.cpp | 228 +++++++++ libs/ui/tests/util_tests.cpp | 134 ++++++ libs/ui/tests/value_format_tests.cpp | 203 ++++++++ 11 files changed, 1609 insertions(+) create mode 100644 libs/ui/tests/CMakeLists.txt create mode 100644 libs/ui/tests/fake_processor.hpp create mode 100644 libs/ui/tests/juce_init.cpp create mode 100644 libs/ui/tests/param_editor_tests.cpp create mode 100644 libs/ui/tests/preset_manager_tests.cpp create mode 100644 libs/ui/tests/session_tests.cpp create mode 100644 libs/ui/tests/util_tests.cpp create mode 100644 libs/ui/tests/value_format_tests.cpp diff --git a/libs/ui/CMakeLists.txt b/libs/ui/CMakeLists.txt index 51138930..ea5cd6bb 100644 --- a/libs/ui/CMakeLists.txt +++ b/libs/ui/CMakeLists.txt @@ -19,3 +19,7 @@ source_group(TREE "${CMAKE_CURRENT_SOURCE_DIR}/source" PREFIX "" FILES ${ui_file target_include_directories(altvane_ui INTERFACE "${CMAKE_CURRENT_SOURCE_DIR}/source") target_compile_features(altvane_ui INTERFACE cxx_std_20) target_link_libraries(altvane_ui INTERFACE altvane_ui_assets juce_gui_basics juce_audio_processors) + +if (ALTVANE_BUILD_TESTS) + add_subdirectory(tests) +endif () diff --git a/libs/ui/source/altvane_ui/session/preset_manager.cpp b/libs/ui/source/altvane_ui/session/preset_manager.cpp index 291fdb98..305997c7 100644 --- a/libs/ui/source/altvane_ui/session/preset_manager.cpp +++ b/libs/ui/source/altvane_ui/session/preset_manager.cpp @@ -84,11 +84,25 @@ namespace altvane::ui { captureReference(); } + namespace { + juce::File& directoryOverride() { + static juce::File dir; + return dir; + } + } + juce::File PresetManager::getDirectory() { + if (directoryOverride() != juce::File()) { + return directoryOverride(); + } return juce::File::getSpecialLocation(juce::File::userApplicationDataDirectory) .getChildFile(JucePlugin_Manufacturer).getChildFile(JucePlugin_Name).getChildFile("Presets"); } + void PresetManager::setDirectoryOverride(const juce::File& dir) { + directoryOverride() = dir; + } + void PresetManager::installFactoryPresets(const std::vector>& presets, const int version) { if (settings_ != nullptr && settings_->getIntValue("factory_presets", 0) >= version) { diff --git a/libs/ui/source/altvane_ui/session/preset_manager.hpp b/libs/ui/source/altvane_ui/session/preset_manager.hpp index 31ff7bc1..716a5654 100644 --- a/libs/ui/source/altvane_ui/session/preset_manager.hpp +++ b/libs/ui/source/altvane_ui/session/preset_manager.hpp @@ -59,6 +59,11 @@ namespace altvane::ui { static juce::File getDirectory(); + /** + * keep presets in another folder (an empty File restores the default), so tests never touch the user's + */ + static void setDirectoryOverride(const juce::File& dir); + /** * copy the product's factory presets into the preset folder once per version, never over user edits * @param presets relative path (e.g. "Factory/Interview") -> preset file contents diff --git a/libs/ui/tests/CMakeLists.txt b/libs/ui/tests/CMakeLists.txt new file mode 100644 index 00000000..d5acb923 --- /dev/null +++ b/libs/ui/tests/CMakeLists.txt @@ -0,0 +1,37 @@ +# AltvaneUITests: Catch2 tests for altvane_ui's logic (value formatting, presets, A/B, undo, the parameter editor), +# registered with CTest next to every product's tests. +# altvane_ui compiles into whatever links it, so this console app builds it, and the JUCE modules it needs, on its own +# under a stand-in plugin name: nothing it does can reach a real product's presets or settings. + +juce_add_console_app(AltvaneUITests PRODUCT_NAME "Altvane UI Tests" VERSION 1.0.0) + +file(GLOB_RECURSE test_files CONFIGURE_DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/*.cpp" "${CMAKE_CURRENT_SOURCE_DIR}/*.hpp") +target_sources(AltvaneUITests PRIVATE ${test_files}) +source_group(TREE "${CMAKE_CURRENT_SOURCE_DIR}" PREFIX "" FILES ${test_files}) +target_compile_features(AltvaneUITests PRIVATE cxx_std_20) + +target_compile_definitions(AltvaneUITests + PRIVATE + JUCE_WEB_BROWSER=0 + JUCE_USE_CURL=0 + # the typing tests pump the message loop, which JUCE only allows with modal loops (as in the product tests) + JUCE_MODAL_LOOPS_PERMITTED=1 + # what altvane_ui reads from a plugin's settings (juce_add_plugin, altvane_add_plugin) + JucePlugin_Manufacturer="${ALTVANE_COMPANY_NAME}" + JucePlugin_Name="Altvane UI Tests" + PRODUCT_NAME_WITHOUT_VERSION="Altvane UI Tests" + VERSION="1.0.0" +) + +target_link_libraries(AltvaneUITests + PRIVATE + altvane_ui + juce_audio_processors + juce_gui_basics + juce::juce_recommended_config_flags + juce::juce_recommended_warning_flags + Catch2::Catch2WithMain +) +set_target_properties(AltvaneUITests PROPERTIES FOLDER "Targets" XCODE_GENERATE_SCHEME ON) + +catch_discover_tests(AltvaneUITests) diff --git a/libs/ui/tests/fake_processor.hpp b/libs/ui/tests/fake_processor.hpp new file mode 100644 index 00000000..4b906e4a --- /dev/null +++ b/libs/ui/tests/fake_processor.hpp @@ -0,0 +1,136 @@ +// Copyright (C) 2026 - Altvane +// This file is part of Altvane +// +// Altvane is free software: you can redistribute it and/or modify it under the terms of the GNU Affero General Public License Version 3 as published by the Free Software Foundation. +// +// Altvane is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License along with Altvane. If not, see . +#pragma once + +#include + +#include +#include + +namespace altvane::ui::test { + namespace ids { + inline constexpr auto kGain = "gain"; + inline constexpr auto kFreq = "freq"; + inline constexpr auto kMode = "mode"; + inline constexpr auto kBypass = "bypass"; + // a fader the engine moves: products leave such parameters out of presets + inline constexpr auto kMeter = "meter"; + } + + /** + * a processor with a handful of parameters of each kind, its state being the parameter tree, + * plus the editor-side objects a product's processor keeps (Session) and its editor makes (ParamEditor) + */ + class FakeProcessor final : public juce::AudioProcessor { + public: + juce::AudioProcessorValueTreeState parameters_; + Session session_; + ParamEditor editor_{session_.undo_manager}; + + FakeProcessor() : + AudioProcessor(BusesProperties() + .withInput("Input", juce::AudioChannelSet::stereo(), true) + .withOutput("Output", juce::AudioChannelSet::stereo(), true)), + parameters_(*this, nullptr, "FakeState", createLayout()) { + } + + juce::RangedAudioParameter* para(const char* id) const { return parameters_.getParameter(id); } + + float plain(const char* id) const { return parameters_.getRawParameterValue(id)->load(); } + + /** + * set a parameter by its plain value, as automation would (outside the editor and its history) + */ + void setPlain(const char* id, const float v) const { + auto* p = para(id); + p->setValueNotifyingHost(p->convertTo0to1(v)); + } + + const juce::String getName() const override { return "Fake"; } + void prepareToPlay(double, int) override {} + void releaseResources() override {} + void processBlock(juce::AudioBuffer&, juce::MidiBuffer&) override {} + double getTailLengthSeconds() const override { return 0.0; } + bool acceptsMidi() const override { return false; } + bool producesMidi() const override { return false; } + juce::AudioProcessorEditor* createEditor() override { return nullptr; } + bool hasEditor() const override { return false; } + int getNumPrograms() override { return 1; } + int getCurrentProgram() override { return 0; } + void setCurrentProgram(int) override {} + const juce::String getProgramName(int) override { return {}; } + void changeProgramName(int, const juce::String&) override {} + + void getStateInformation(juce::MemoryBlock& dest) override { + if (const auto xml = parameters_.copyState().createXml()) { + copyXmlToBinary(*xml, dest); + } + } + + void setStateInformation(const void* data, const int size) override { + if (const auto xml = getXmlFromBinary(data, size); xml != nullptr + && xml->hasTagName(parameters_.state.getType())) { + parameters_.replaceState(juce::ValueTree::fromXml(*xml)); + } + } + + private: + static juce::AudioProcessorValueTreeState::ParameterLayout createLayout() { + juce::AudioProcessorValueTreeState::ParameterLayout layout; + layout.add(std::make_unique( + juce::ParameterID{ids::kGain, 1}, "Gain", juce::NormalisableRange(-24.f, 24.f, .01f), 0.f)); + layout.add(std::make_unique( + juce::ParameterID{ids::kFreq, 1}, "Freq", + juce::NormalisableRange(20.f, 20000.f, 0.f, .25f), 1000.f)); + layout.add(std::make_unique( + juce::ParameterID{ids::kMode, 1}, "Mode", juce::StringArray{"Clean", "Warm", "Hot"}, 0)); + layout.add(std::make_unique(juce::ParameterID{ids::kBypass, 1}, "Bypass", false)); + layout.add(std::make_unique( + juce::ParameterID{ids::kMeter, 1}, "Meter", juce::NormalisableRange(0.f, 1.f), .5f)); + return layout; + } + }; + + /** + * counts a parameter's host gestures: every begin must be matched by an end + */ + class GestureCounter final : private juce::AudioProcessorParameter::Listener { + public: + int begins{0}, ends{0}, changes{0}; + + explicit GestureCounter(juce::AudioProcessorParameter* p) : p_(p) { p_->addListener(this); } + + ~GestureCounter() override { p_->removeListener(this); } + + private: + juce::AudioProcessorParameter* p_; + + void parameterValueChanged(int, float) override { ++changes; } + + void parameterGestureChanged(int, const bool starting) override { ++(starting ? begins : ends); } + }; + + /** + * a fresh, empty folder (by default under the system's temporary folder), deleted when the test ends + */ + class TempDir final { + public: + explicit TempDir(const juce::File& parent = juce::File::getSpecialLocation(juce::File::tempDirectory)) : + dir_(parent.getNonexistentChildFile("altvane_ui_tests", "", false)) { + dir_.createDirectory(); + } + + ~TempDir() { dir_.deleteRecursively(); } + + const juce::File& get() const { return dir_; } + + private: + juce::File dir_; + }; +} diff --git a/libs/ui/tests/juce_init.cpp b/libs/ui/tests/juce_init.cpp new file mode 100644 index 00000000..afc7c18b --- /dev/null +++ b/libs/ui/tests/juce_init.cpp @@ -0,0 +1,33 @@ +// Copyright (C) 2026 - Altvane +// This file is part of Altvane +// +// Altvane is free software: you can redistribute it and/or modify it under the terms of the GNU Affero General Public License Version 3 as published by the Free Software Foundation. +// +// Altvane is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License along with Altvane. If not, see . +#include +#include +#include + +/** + * parameters, widgets and the message loop need JUCE (message manager, desktop) for the whole run; + * scoping it to the run gives a clean shutdown before JUCE's leak detector checks + */ +class JuceRunListener final : public Catch::EventListenerBase { +public: + using EventListenerBase::EventListenerBase; + + void testRunStarting(const Catch::TestRunInfo&) override { + juce_ = std::make_unique(); + } + + void testRunEnded(const Catch::TestRunStats&) override { + juce_.reset(); + } + +private: + std::unique_ptr juce_; +}; + +CATCH_REGISTER_LISTENER(JuceRunListener) diff --git a/libs/ui/tests/param_editor_tests.cpp b/libs/ui/tests/param_editor_tests.cpp new file mode 100644 index 00000000..3b2f6a90 --- /dev/null +++ b/libs/ui/tests/param_editor_tests.cpp @@ -0,0 +1,370 @@ +// Copyright (C) 2026 - Altvane +// This file is part of Altvane +// +// Altvane is free software: you can redistribute it and/or modify it under the terms of the GNU Affero General Public License Version 3 as published by the Free Software Foundation. +// +// Altvane is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License along with Altvane. If not, see . +#include +#include + +#include +#include + +#include "fake_processor.hpp" + +using namespace altvane::ui; +using namespace altvane::ui::test; +using Catch::Matchers::WithinAbs; + +namespace { + juce::UndoManager& history(FakeProcessor& p) { return p.session_.undo_manager; } +} + +TEST_CASE("setNow changes parameters as one undoable step with one gesture each", "[param_editor]") { + FakeProcessor p; + GestureCounter gain(p.para(ids::kGain)), freq(p.para(ids::kFreq)); + + p.editor_.setNow({{p.para(ids::kGain), 6.f}, {p.para(ids::kFreq), 250.f}}, "Set both"); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(6.0, 1e-4)); + CHECK_THAT(p.plain(ids::kFreq), WithinAbs(250.0, 1e-2)); + CHECK(gain.begins == 1); + CHECK(gain.ends == 1); + CHECK(freq.begins == 1); + CHECK(freq.ends == 1); + CHECK_FALSE(p.editor_.isEditing()); + + REQUIRE(history(p).canUndo()); + CHECK(history(p).getUndoDescription() == "Set both"); + + p.editor_.undo(); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(0.0, 1e-4)); + CHECK_THAT(p.plain(ids::kFreq), WithinAbs(1000.0, 1e-2)); + CHECK_FALSE(history(p).canUndo()); + // the host hears undo as gestures too + CHECK(gain.begins == gain.ends); + + p.editor_.redo(); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(6.0, 1e-4)); + CHECK_THAT(p.plain(ids::kFreq), WithinAbs(250.0, 1e-2)); + CHECK_FALSE(history(p).canRedo()); +} + +TEST_CASE("an edit that changes nothing leaves no undo step", "[param_editor]") { + FakeProcessor p; + p.editor_.setNow({{p.para(ids::kGain), 0.f}}, "Nothing"); + CHECK_FALSE(history(p).canUndo()); + + juce::RangedAudioParameter* paras[] = {p.para(ids::kGain)}; + p.editor_.beginEdit(paras, "Drag"); + p.editor_.setPlain(p.para(ids::kGain), 3.f); + p.editor_.setPlain(p.para(ids::kGain), 0.f); + p.editor_.endEdit(); + CHECK_FALSE(history(p).canUndo()); +} + +TEST_CASE("a continuous edit is one gesture and one undo step back to where it began", "[param_editor]") { + FakeProcessor p; + p.setPlain(ids::kGain, -3.f); + GestureCounter gain(p.para(ids::kGain)); + + juce::RangedAudioParameter* paras[] = {p.para(ids::kGain)}; + p.editor_.beginEdit(paras, "Change Gain"); + CHECK(p.editor_.isEditing()); + for (int i = 1; i <= 10; ++i) { + p.editor_.setPlain(p.para(ids::kGain), -3.f + static_cast(i)); + } + CHECK(gain.begins == 1); + CHECK(gain.ends == 0); + p.editor_.endEdit(); + CHECK(gain.ends == 1); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(7.0, 1e-4)); + + REQUIRE(history(p).canUndo()); + CHECK(history(p).getUndoDescription() == "Change Gain"); + p.editor_.undo(); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(-3.0, 1e-4)); + CHECK_FALSE(history(p).canUndo()); +} + +TEST_CASE("cancelling an edit restores the parameters and leaves no undo step", "[param_editor]") { + FakeProcessor p; + GestureCounter gain(p.para(ids::kGain)); + juce::RangedAudioParameter* paras[] = {p.para(ids::kGain), p.para(ids::kFreq)}; + p.editor_.beginEdit(paras, "Drag"); + p.editor_.setPlain(p.para(ids::kGain), 12.f); + p.editor_.setPlain(p.para(ids::kFreq), 5000.f); + p.editor_.cancelEdit(); + + CHECK_FALSE(p.editor_.isEditing()); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(0.0, 1e-4)); + CHECK_THAT(p.plain(ids::kFreq), WithinAbs(1000.0, 1e-2)); + CHECK_FALSE(history(p).canUndo()); + CHECK(gain.begins == 1); + CHECK(gain.ends == 1); +} + +TEST_CASE("plain values are clamped to the range and snapped to legal values", "[param_editor]") { + FakeProcessor p; + p.editor_.setNow({{p.para(ids::kGain), 99.f}}, "Too loud"); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(24.0, 1e-4)); + p.editor_.setNow({{p.para(ids::kGain), -99.f}}, "Too quiet"); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(-24.0, 1e-4)); + p.editor_.setNow({{p.para(ids::kGain), 1.234567f}}, "Fine"); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(1.23, 1e-4)); + + p.editor_.setNow({{p.para(ids::kMode), 1.4f}}, "Mode"); + CHECK(p.plain(ids::kMode) == 1.f); + p.editor_.setNow({{p.para(ids::kMode), 7.f}}, "Mode"); + CHECK(p.plain(ids::kMode) == 2.f); + + // a null parameter is ignored rather than dereferenced + p.editor_.setPlain(nullptr, 1.f); + p.editor_.toggle(nullptr, "Nothing"); +} + +TEST_CASE("toggle flips a switch as an undoable step", "[param_editor]") { + FakeProcessor p; + p.editor_.toggle(p.para(ids::kBypass), "Bypass"); + CHECK(p.plain(ids::kBypass) == 1.f); + p.editor_.toggle(p.para(ids::kBypass), "Bypass"); + CHECK(p.plain(ids::kBypass) == 0.f); + p.editor_.undo(); + CHECK(p.plain(ids::kBypass) == 1.f); + p.editor_.undo(); + CHECK(p.plain(ids::kBypass) == 0.f); + CHECK_FALSE(history(p).canUndo()); +} + +TEST_CASE("each edit is its own undo step, undone newest first", "[param_editor]") { + FakeProcessor p; + p.editor_.setNow({{p.para(ids::kGain), 1.f}}, "One"); + p.editor_.setNow({{p.para(ids::kGain), 2.f}}, "Two"); + p.editor_.setNow({{p.para(ids::kFreq), 100.f}}, "Three"); + + p.editor_.undo(); + CHECK_THAT(p.plain(ids::kFreq), WithinAbs(1000.0, 1e-2)); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(2.0, 1e-4)); + p.editor_.undo(); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(1.0, 1e-4)); + p.editor_.redo(); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(2.0, 1e-4)); + + // a new edit after undo drops the redo branch + p.editor_.undo(); + p.editor_.setNow({{p.para(ids::kGain), 5.f}}, "Five"); + CHECK_FALSE(history(p).canRedo()); + p.editor_.undo(); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(1.0, 1e-4)); +} + +TEST_CASE("undo during an edit ends the edit and undoes it", "[param_editor]") { + FakeProcessor p; + GestureCounter gain(p.para(ids::kGain)); + juce::RangedAudioParameter* paras[] = {p.para(ids::kGain)}; + p.editor_.beginEdit(paras, "Drag"); + p.editor_.setPlain(p.para(ids::kGain), 9.f); + p.editor_.undo(); + + CHECK_FALSE(p.editor_.isEditing()); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(0.0, 1e-4)); + CHECK(gain.begins == gain.ends); + p.editor_.redo(); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(9.0, 1e-4)); +} + +TEST_CASE("a new edit ends the open one first", "[param_editor]") { + FakeProcessor p; + GestureCounter gain(p.para(ids::kGain)); + juce::RangedAudioParameter* first[] = {p.para(ids::kGain)}; + p.editor_.beginEdit(first, "First"); + const auto first_id = p.editor_.getEditId(); + p.editor_.setPlain(p.para(ids::kGain), 4.f); + + juce::RangedAudioParameter* second[] = {p.para(ids::kFreq)}; + p.editor_.beginEdit(second, "Second"); + CHECK(gain.ends == 1); + CHECK_FALSE(p.editor_.isEditing(first_id)); + CHECK(p.editor_.isEditing(p.editor_.getEditId())); + p.editor_.setPlain(p.para(ids::kFreq), 300.f); + p.editor_.endEdit(); + + p.editor_.undo(); + CHECK_THAT(p.plain(ids::kFreq), WithinAbs(1000.0, 1e-2)); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(4.0, 1e-4)); + p.editor_.undo(); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(0.0, 1e-4)); +} + +TEST_CASE("an editor closed mid-edit ends the gesture and keeps the step", "[param_editor]") { + FakeProcessor p; + GestureCounter gain(p.para(ids::kGain)); + { + ParamEditor editor(p.session_.undo_manager); + juce::RangedAudioParameter* paras[] = {p.para(ids::kGain)}; + editor.beginEdit(paras, "Drag"); + editor.setPlain(p.para(ids::kGain), -6.f); + } + CHECK(gain.begins == 1); + CHECK(gain.ends == 1); + REQUIRE(history(p).canUndo()); + history(p).undo(); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(0.0, 1e-4)); +} + +TEST_CASE("setNotifyingHost changes a parameter outside the history", "[param_editor]") { + FakeProcessor p; + GestureCounter meter(p.para(ids::kMeter)); + ParamEditor::setNotifyingHost(p.para(ids::kMeter), .25f); + CHECK_THAT(p.plain(ids::kMeter), WithinAbs(.25, 1e-6)); + CHECK(meter.begins == 1); + CHECK(meter.ends == 1); + CHECK_FALSE(history(p).canUndo()); +} + +// ---------------------------------------------------------------- bindValue: a control wired to a parameter + +namespace { + struct BoundKnob { + FakeProcessor p; + Knob knob; + GestureCounter gestures{p.para(ids::kGain)}; + + BoundKnob() { + bindValue(knob, p.editor_, p.para(ids::kGain), "Gain"); + knob.setSize(80, 100); + } + + juce::TextEditor* textEditor() const { + for (auto* child : knob.getChildren()) { + if (auto* editor = dynamic_cast(child)) { + return editor; + } + } + return nullptr; + } + + /** + * open the control's text entry, type and press a key, as a user would + */ + void type(const juce::String& text, const int key = juce::KeyPress::returnKey) { + knob.beginTextEdit(); + auto* editor = textEditor(); + REQUIRE(editor != nullptr); + editor->setText(text, false); + editor->keyPressed(juce::KeyPress(key)); + // the text editor tells its listeners about return and escape asynchronously + juce::MessageManager::getInstance()->runDispatchLoopUntil(20); + REQUIRE(textEditor() == nullptr); + } + }; +} + +TEST_CASE("bindValue gives the control the parameter's range, default and ID", "[param_editor][typing]") { + BoundKnob b; + CHECK(b.knob.getComponentID() == ids::kGain); + CHECK(b.knob.getRange().start == -24.f); + CHECK(b.knob.getRange().end == 24.f); + CHECK(b.knob.getTitle() == "Gain"); + REQUIRE(b.knob.parse); +} + +TEST_CASE("typing a value sets the parameter as one gesture and one undo step", "[param_editor][typing]") { + BoundKnob b; + b.type(format::kMinus + "6.5 dB"); + CHECK_THAT(b.p.plain(ids::kGain), WithinAbs(-6.5, 1e-4)); + CHECK_THAT(b.knob.getValue(), WithinAbs(-6.5, 1e-4)); + CHECK(b.gestures.begins == 1); + CHECK(b.gestures.ends == 1); + REQUIRE(b.p.session_.undo_manager.canUndo()); + CHECK(b.p.session_.undo_manager.getUndoDescription() == "Change Gain"); + + b.p.editor_.undo(); + CHECK_THAT(b.p.plain(ids::kGain), WithinAbs(0.0, 1e-4)); +} + +TEST_CASE("typed values accept hyphens, signs and no unit", "[param_editor][typing]") { + BoundKnob b; + b.type("-3"); + CHECK_THAT(b.p.plain(ids::kGain), WithinAbs(-3.0, 1e-4)); + b.type("+4.25"); + CHECK_THAT(b.p.plain(ids::kGain), WithinAbs(4.25, 1e-4)); + b.type("2,5 dB"); + CHECK_THAT(b.p.plain(ids::kGain), WithinAbs(2.5, 1e-4)); +} + +TEST_CASE("a typed value outside the range is clamped", "[param_editor][typing]") { + BoundKnob b; + b.type("+99"); + CHECK_THAT(b.p.plain(ids::kGain), WithinAbs(24.0, 1e-4)); + b.type("-1000 dB"); + CHECK_THAT(b.p.plain(ids::kGain), WithinAbs(-24.0, 1e-4)); +} + +TEST_CASE("typing junk changes nothing and leaves no undo step", "[param_editor][typing]") { + BoundKnob b; + b.p.setPlain(ids::kGain, 3.f); + for (const auto* junk : {"", "loud", "dB", "-inf"}) { + INFO("text: '" << junk << "'"); + b.type(junk); + CHECK_THAT(b.p.plain(ids::kGain), WithinAbs(3.0, 1e-4)); + } + CHECK(b.gestures.begins == 0); + CHECK_FALSE(b.p.session_.undo_manager.canUndo()); +} + +TEST_CASE("escape closes the text entry without a change", "[param_editor][typing]") { + BoundKnob b; + b.type("12", juce::KeyPress::escapeKey); + CHECK_THAT(b.p.plain(ids::kGain), WithinAbs(0.0, 1e-4)); + CHECK(b.gestures.begins == 0); +} + +TEST_CASE("accessibility clients type values through the same parser", "[param_editor][typing]") { + BoundKnob b; + const auto handler = b.knob.createAccessibilityHandler(); + auto* value = handler->getValueInterface(); + REQUIRE(value != nullptr); + value->setValueAsString("-12 dB"); + CHECK_THAT(b.p.plain(ids::kGain), WithinAbs(-12.0, 1e-4)); + value->setValueAsString("nonsense"); + CHECK_THAT(b.p.plain(ids::kGain), WithinAbs(-12.0, 1e-4)); + CHECK(b.gestures.begins == 1); + CHECK(b.gestures.ends == 1); +} + +TEST_CASE("a control ends only its own edit", "[param_editor][typing]") { + // a wheel's edit ends on a timer, after another control may have begun its own + FakeProcessor p; + Knob a, b; + bindValue(a, p.editor_, p.para(ids::kGain), "Gain"); + bindValue(b, p.editor_, p.para(ids::kFreq), "Freq"); + GestureCounter gain(p.para(ids::kGain)), freq(p.para(ids::kFreq)); + + a.on_begin(); + a.on_change(5.f); + b.on_begin(); + b.on_change(200.f); + // a's wheel timer fires: b's edit must stay open + a.on_end(); + CHECK(p.editor_.isEditing()); + CHECK(freq.ends == 0); + b.on_end(); + CHECK_FALSE(p.editor_.isEditing()); + + // a's next wheel notch after b took over opens a fresh edit rather than change gain outside a gesture + a.on_change(6.f); + CHECK(p.editor_.isEditing()); + a.on_end(); + CHECK(gain.begins == gain.ends); + CHECK(freq.begins == freq.ends); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(6.0, 1e-4)); + + p.editor_.undo(); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(5.0, 1e-4)); + p.editor_.undo(); + CHECK_THAT(p.plain(ids::kFreq), WithinAbs(1000.0, 1e-2)); + p.editor_.undo(); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(0.0, 1e-4)); +} diff --git a/libs/ui/tests/preset_manager_tests.cpp b/libs/ui/tests/preset_manager_tests.cpp new file mode 100644 index 00000000..3dbafe19 --- /dev/null +++ b/libs/ui/tests/preset_manager_tests.cpp @@ -0,0 +1,445 @@ +// Copyright (C) 2026 - Altvane +// This file is part of Altvane +// +// Altvane is free software: you can redistribute it and/or modify it under the terms of the GNU Affero General Public License Version 3 as published by the Free Software Foundation. +// +// Altvane is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License along with Altvane. If not, see . +#include +#include + +#include +#include + +#include + +#include "fake_processor.hpp" + +#if JUCE_LINUX || JUCE_MAC +#include +#endif + +using namespace altvane::ui; +using namespace altvane::ui::test; +using Catch::Matchers::WithinAbs; + +namespace { + /** + * where the tests' preset folders go: the temporary folder, except on Linux, where gio only trashes files on the + * home folder's file system (often not /tmp's), so it's ~/.cache there + */ + juce::File scratchRoot() { +#if JUCE_LINUX + const auto cache = juce::File::getSpecialLocation(juce::File::userHomeDirectory).getChildFile(".cache"); + cache.createDirectory(); + return cache; +#else + return juce::File::getSpecialLocation(juce::File::tempDirectory); +#endif + } + +#if JUCE_LINUX + /** + * send removed presets to a trash of the run's own (gio reads XDG_DATA_HOME once per process), + * so tests never fill the user's + */ + void useTestTrash() { + struct TestTrash { + TempDir dir{scratchRoot()}; + + TestTrash() { setenv("XDG_DATA_HOME", dir.get().getFullPathName().toRawUTF8(), 1); } + }; + static TestTrash trash; + } +#endif + + PresetCodec makeCodec() { + auto codec = PresetCodec::xmlAsJson(); + codec.includes = [](const juce::AudioProcessorParameter& p) { + const auto* with_id = dynamic_cast(&p); + return with_id == nullptr || with_id->paramID != ids::kMeter; + }; + return codec; + } + + /** + * a preset manager on a fake processor, its preset folder and settings file in a temporary folder + */ + struct Presets { + TempDir temp{scratchRoot()}; + FakeProcessor p; + std::unique_ptr settings; + PresetManager presets{p, p.editor_, p.session_, makeCodec()}; + + explicit Presets(const bool with_settings = true) { + PresetManager::setDirectoryOverride(temp.get().getChildFile("Presets")); + if (with_settings) { + juce::PropertiesFile::Options options; + options.storageFormat = juce::PropertiesFile::storeAsXML; + options.millisecondsBeforeSaving = -1; + settings = std::make_unique(temp.get().getChildFile("settings.xml"), options); + presets.setSettings(settings.get()); + } +#if JUCE_LINUX + useTestTrash(); +#endif + } + + ~Presets() { + PresetManager::setDirectoryOverride({}); + } + + static juce::File dir() { return PresetManager::getDirectory(); } + + juce::File file(const juce::String& relative) const { return dir().getChildFile(relative + PresetManager::kExtension); } + + /** + * the preset file text for the fake processor with gain set, as a product's factory presets are written + */ + static juce::String presetText(const float gain, const float mode = 0.f) { + FakeProcessor other; + other.setPlain(ids::kGain, gain); + other.setPlain(ids::kMode, mode); + return PresetCodec::toJsonText(*other.parameters_.copyState().createXml()); + } + + juce::StringArray names() const { + juce::StringArray result; + for (const auto& e : presets.scan()) { + result.add(e.name); + } + return result; + } + + juce::UndoManager& history() { return p.session_.undo_manager; } + }; +} + +TEST_CASE("the preset folder can be moved for tests", "[presets]") { + const auto normal = PresetManager::getDirectory(); + CHECK(normal.getFullPathName().contains("Altvane UI Tests")); + { + const Presets t; + CHECK(Presets::dir() == t.temp.get().getChildFile("Presets")); + } + CHECK(PresetManager::getDirectory() == normal); +} + +TEST_CASE("factory presets are installed into their groups", "[presets][factory]") { + Presets t; + t.presets.installFactoryPresets({{"Factory/Warm", Presets::presetText(3.f, 1.f)}, + {"Factory/Hot", Presets::presetText(9.f, 2.f)}, + {"Init", Presets::presetText(0.f)}}, 1); + CHECK(t.file("Factory/Warm").existsAsFile()); + CHECK(t.file("Factory/Hot").existsAsFile()); + CHECK(t.file("Init").existsAsFile()); + CHECK(t.settings->getIntValue("factory_presets") == 1); + + const auto entries = t.presets.scan(); + REQUIRE(entries.size() == 3); + CHECK(entries[0].name == "Hot"); + CHECK(entries[0].group == "Factory"); + CHECK(entries[1].name == "Init"); + CHECK(entries[1].group.isEmpty()); + CHECK(entries[2].name == "Warm"); + CHECK(t.presets.getGroups() == juce::StringArray{"Factory"}); + + REQUIRE(t.presets.load(t.file("Factory/Hot"))); + CHECK_THAT(t.p.plain(ids::kGain), WithinAbs(9.0, 1e-4)); + CHECK(t.p.plain(ids::kMode) == 2.f); + CHECK(t.presets.getCurrentName() == "Hot"); +} + +TEST_CASE("factory presets install once per version and never over the user's edits", "[presets][factory]") { + Presets t; + const std::vector> factory{ + {"Factory/Warm", Presets::presetText(3.f)}, {"Factory/Hot", Presets::presetText(9.f)}}; + t.presets.installFactoryPresets(factory, 1); + + // the user deletes one and overwrites the other + REQUIRE(t.file("Factory/Warm").deleteFile()); + REQUIRE(t.file("Factory/Hot").replaceWithText(Presets::presetText(-1.f))); + + t.presets.installFactoryPresets(factory, 1); + CHECK_FALSE(t.file("Factory/Warm").exists()); + + // a newer version brings back what is missing but keeps the user's file + t.presets.installFactoryPresets(factory, 2); + CHECK(t.file("Factory/Warm").existsAsFile()); + CHECK(t.file("Factory/Hot").loadFileAsString() == Presets::presetText(-1.f)); + CHECK(t.settings->getIntValue("factory_presets") == 2); +} + +TEST_CASE("without a settings file, factory presets fill in what is missing every time", "[presets][factory]") { + Presets t(false); + const std::vector> factory{{"Factory/Warm", Presets::presetText(3.f)}}; + t.presets.installFactoryPresets(factory, 1); + REQUIRE(t.file("Factory/Warm").deleteFile()); + t.presets.installFactoryPresets(factory, 1); + CHECK(t.file("Factory/Warm").existsAsFile()); + // and favourites are simply off + t.presets.setFavourite(t.file("Factory/Warm"), true); + CHECK_FALSE(t.presets.isFavourite(t.file("Factory/Warm"))); +} + +TEST_CASE("presets save and load the processor state", "[presets]") { + Presets t; + t.p.setPlain(ids::kGain, -7.5f); + t.p.setPlain(ids::kFreq, 3200.f); + t.p.setPlain(ids::kMode, 1.f); + t.p.setPlain(ids::kBypass, 1.f); + REQUIRE(t.presets.save(t.file("Mine/Vocal"))); + CHECK(t.file("Mine/Vocal").existsAsFile()); + CHECK(t.presets.getCurrentFile() == t.file("Mine/Vocal")); + CHECK(t.presets.getCurrentName() == "Vocal"); + CHECK_FALSE(t.presets.isModified()); + + t.presets.initialise(); + CHECK_THAT(t.p.plain(ids::kGain), WithinAbs(0.0, 1e-4)); + + REQUIRE(t.presets.load(t.file("Mine/Vocal"))); + CHECK_THAT(t.p.plain(ids::kGain), WithinAbs(-7.5, 1e-4)); + CHECK_THAT(t.p.plain(ids::kFreq), WithinAbs(3200.0, 1e-1)); + CHECK(t.p.plain(ids::kMode) == 1.f); + CHECK(t.p.plain(ids::kBypass) == 1.f); + CHECK(t.presets.getCurrentFile() == t.file("Mine/Vocal")); + CHECK_FALSE(t.presets.isModified()); +} + +TEST_CASE("a preset is modified once a preset parameter differs from it", "[presets]") { + Presets t; + REQUIRE(t.presets.save(t.file("Base"))); + CHECK_FALSE(t.presets.isModified()); + + // a parameter left out of presets doesn't count + t.p.setPlain(ids::kMeter, .9f); + CHECK_FALSE(t.presets.isModified()); + + t.p.setPlain(ids::kGain, 2.f); + CHECK(t.presets.isModified()); + t.p.setPlain(ids::kGain, 0.f); + CHECK_FALSE(t.presets.isModified()); + + t.p.setPlain(ids::kGain, 2.f); + REQUIRE(t.presets.save(t.file("Base"))); + CHECK_FALSE(t.presets.isModified()); +} + +TEST_CASE("loading a preset ends an open edit and clears the history", "[presets]") { + Presets t; + REQUIRE(t.presets.save(t.file("Base"))); + t.p.editor_.setNow({{t.p.para(ids::kGain), 5.f}}, "Edit"); + GestureCounter gain(t.p.para(ids::kGain)); + juce::RangedAudioParameter* paras[] = {t.p.para(ids::kGain)}; + t.p.editor_.beginEdit(paras, "Drag"); + t.p.editor_.setPlain(t.p.para(ids::kGain), 6.f); + + REQUIRE(t.presets.load(t.file("Base"))); + CHECK_FALSE(t.p.editor_.isEditing()); + CHECK(gain.begins == gain.ends); + CHECK_FALSE(t.history().canUndo()); + CHECK_THAT(t.p.plain(ids::kGain), WithinAbs(0.0, 1e-4)); +} + +TEST_CASE("a corrupt or missing preset file loads nothing", "[presets][corrupt]") { + Presets t; + REQUIRE(t.presets.save(t.file("Good"))); + t.p.editor_.setNow({{t.p.para(ids::kGain), 4.f}}, "Edit"); + + const std::vector> bad{ + {"Empty", ""}, + {"Truncated", "{\"tag\": \"FakeState\", \"children\": [{\"tag\": \"PARAM\", \"id\": \"ga"}, + {"Not JSON", ""}, + {"No tag", "{\"id\": \"gain\", \"value\": \"1\"}"}, + {"Array", "[1, 2, 3]"}, + {"Binary", "\x01\x02\xff\xfe garbage"}, + }; + for (const auto& [name, text] : bad) { + INFO(name); + const auto file = t.file(name); + REQUIRE(file.replaceWithData(text, std::strlen(text))); + CHECK_FALSE(t.presets.load(file)); + CHECK_THAT(t.p.plain(ids::kGain), WithinAbs(4.0, 1e-4)); + CHECK(t.presets.getCurrentFile() == t.file("Good")); + CHECK(t.history().canUndo()); + } + CHECK_FALSE(t.presets.load(t.file("Does not exist"))); + CHECK(t.presets.getCurrentFile() == t.file("Good")); + + // corrupt files still list, so they can be deleted + CHECK(t.names().contains("Truncated")); +} + +TEST_CASE("saving over an existing preset replaces it", "[presets][collision]") { + Presets t; + t.p.setPlain(ids::kGain, 1.f); + REQUIRE(t.presets.save(t.file("Same"))); + t.p.setPlain(ids::kGain, 2.f); + REQUIRE(t.presets.save(t.file("Same"))); + CHECK(t.names() == juce::StringArray{"Same"}); + t.p.setPlain(ids::kGain, 0.f); + REQUIRE(t.presets.load(t.file("Same"))); + CHECK_THAT(t.p.plain(ids::kGain), WithinAbs(2.0, 1e-4)); +} + +TEST_CASE("renaming a preset moves its file, the current preset and its favourite", "[presets][rename]") { + Presets t; + REQUIRE(t.presets.save(t.file("Old"))); + t.presets.setFavourite(t.file("Old"), true); + + REQUIRE(t.presets.rename(t.file("Old"), " New ")); + CHECK_FALSE(t.file("Old").exists()); + CHECK(t.file("New").existsAsFile()); + CHECK(t.presets.getCurrentFile() == t.file("New")); + CHECK(t.presets.isFavourite(t.file("New"))); + CHECK_FALSE(t.presets.isFavourite(t.file("Old"))); + CHECK(t.names() == juce::StringArray{"New"}); +} + +TEST_CASE("renaming onto another preset's name fails and keeps both", "[presets][rename][collision]") { + Presets t; + t.p.setPlain(ids::kGain, 1.f); + REQUIRE(t.presets.save(t.file("One"))); + t.p.setPlain(ids::kGain, 2.f); + REQUIRE(t.presets.save(t.file("Two"))); + const auto one = t.file("One").loadFileAsString(); + const auto two = t.file("Two").loadFileAsString(); + + CHECK_FALSE(t.presets.rename(t.file("One"), "Two")); + CHECK(t.file("One").loadFileAsString() == one); + CHECK(t.file("Two").loadFileAsString() == two); + CHECK(t.presets.getCurrentFile() == t.file("Two")); +} + +TEST_CASE("renaming only the case of a preset's name works", "[presets][rename]") { + // case-insensitive file systems (macOS, Windows) see the new name as the existing file + Presets t; + REQUIRE(t.presets.save(t.file("bass"))); + REQUIRE(t.presets.rename(t.file("bass"), "Bass")); + CHECK(t.names() == juce::StringArray{"Bass"}); + CHECK(t.presets.getCurrentName() == "Bass"); +} + +TEST_CASE("renaming to an empty or illegal name", "[presets][rename]") { + Presets t; + REQUIRE(t.presets.save(t.file("Keep"))); + CHECK_FALSE(t.presets.rename(t.file("Keep"), "")); + CHECK_FALSE(t.presets.rename(t.file("Keep"), " ")); + CHECK(t.file("Keep").existsAsFile()); + + // characters a file name can't have are dropped rather than creating folders + REQUIRE(t.presets.rename(t.file("Keep"), "a/b:c")); + REQUIRE(t.presets.scan().size() == 1); + CHECK(t.presets.scan()[0].group.isEmpty()); + CHECK(t.presets.getCurrentFile().existsAsFile()); +} + +TEST_CASE("presets list in natural order with their groups and favourites", "[presets]") { + Presets t; + for (const auto* name : {"Preset 10", "Preset 2", "Drums/Kick", "Preset 1"}) { + REQUIRE(t.presets.save(t.file(name))); + } + t.presets.setFavourite(t.file("Preset 2"), true); + REQUIRE(t.file("Readme").withFileExtension("txt").replaceWithText("not a preset")); + + const auto entries = t.presets.scan(); + REQUIRE(entries.size() == 4); + CHECK(entries[0].name == "Kick"); + CHECK(entries[0].group == "Drums"); + CHECK(entries[1].name == "Preset 1"); + CHECK(entries[2].name == "Preset 2"); + CHECK(entries[2].favourite); + CHECK(entries[3].name == "Preset 10"); + CHECK_FALSE(entries[3].favourite); + + t.presets.setFavourite(t.file("Preset 2"), false); + CHECK_FALSE(t.presets.isFavourite(t.file("Preset 2"))); +} + +TEST_CASE("groups are folders in the preset folder", "[presets]") { + Presets t; + const auto group = t.presets.createGroup(" Vocals "); + CHECK(group.isDirectory()); + CHECK(group == Presets::dir().getChildFile("Vocals")); + t.presets.createGroup("Bass"); + CHECK(t.presets.getGroups() == juce::StringArray{"Bass", "Vocals"}); +} + +TEST_CASE("stepping through presets wraps around", "[presets][step]") { + Presets t; + for (const auto* name : {"A", "B", "C"}) { + REQUIRE(t.presets.save(t.file(name))); + } + t.presets.initialise(); + REQUIRE(t.presets.getCurrentFile() == juce::File()); + + // from no preset, forward goes to the first and back to the last + t.presets.step(1); + CHECK(t.presets.getCurrentName() == "A"); + t.presets.step(1); + CHECK(t.presets.getCurrentName() == "B"); + t.presets.step(-1); + CHECK(t.presets.getCurrentName() == "A"); + t.presets.step(-1); + CHECK(t.presets.getCurrentName() == "C"); + t.presets.step(1); + CHECK(t.presets.getCurrentName() == "A"); + + t.presets.initialise(); + t.presets.step(-1); + CHECK(t.presets.getCurrentName() == "C"); +} + +TEST_CASE("stepping with no presets does nothing", "[presets][step]") { + Presets t; + t.presets.step(1); + CHECK(t.presets.getCurrentFile() == juce::File()); +} + +TEST_CASE("initialise resets preset parameters to their defaults as one undoable step", "[presets]") { + Presets t; + t.p.setPlain(ids::kGain, 5.f); + t.p.setPlain(ids::kMode, 2.f); + t.p.setPlain(ids::kMeter, .1f); + REQUIRE(t.presets.save(t.file("Loud"))); + + t.presets.initialise(); + CHECK_THAT(t.p.plain(ids::kGain), WithinAbs(0.0, 1e-4)); + CHECK(t.p.plain(ids::kMode) == 0.f); + // left out of presets, so left alone + CHECK_THAT(t.p.plain(ids::kMeter), WithinAbs(.1, 1e-6)); + CHECK(t.presets.getCurrentName() == "Default"); + CHECK_FALSE(t.presets.isModified()); + + REQUIRE(t.history().canUndo()); + CHECK(t.history().getUndoDescription() == "Initialise"); + t.p.editor_.undo(); + CHECK_THAT(t.p.plain(ids::kGain), WithinAbs(5.0, 1e-4)); + CHECK(t.p.plain(ids::kMode) == 2.f); +} + +TEST_CASE("the JSON codec round-trips a state", "[presets][codec]") { + const TempDir temp; + const auto codec = PresetCodec::xmlAsJson(); + FakeProcessor a; + a.setPlain(ids::kGain, 3.25f); + a.setPlain(ids::kBypass, 1.f); + juce::MemoryBlock state; + a.getStateInformation(state); + + const auto file = temp.get().getChildFile("state.json"); + REQUIRE(codec.write(file, state).wasOk()); + CHECK(juce::JSON::parse(file).getProperty("tag", {}).toString() == "FakeState"); + + juce::MemoryBlock read; + REQUIRE(codec.read(file, read).wasOk()); + FakeProcessor b; + b.setStateInformation(read.getData(), static_cast(read.getSize())); + CHECK_THAT(b.plain(ids::kGain), WithinAbs(3.25, 1e-4)); + CHECK(b.plain(ids::kBypass) == 1.f); + + // a state that isn't XML can't be written + juce::MemoryBlock junk("junk", 4); + CHECK_FALSE(codec.write(temp.get().getChildFile("junk.json"), junk).wasOk()); +} diff --git a/libs/ui/tests/session_tests.cpp b/libs/ui/tests/session_tests.cpp new file mode 100644 index 00000000..a4dae634 --- /dev/null +++ b/libs/ui/tests/session_tests.cpp @@ -0,0 +1,228 @@ +// Copyright (C) 2026 - Altvane +// This file is part of Altvane +// +// Altvane is free software: you can redistribute it and/or modify it under the terms of the GNU Affero General Public License Version 3 as published by the Free Software Foundation. +// +// Altvane is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License along with Altvane. If not, see . +#include +#include + +#include +#include +#include + +#include "fake_processor.hpp" + +using namespace altvane::ui; +using namespace altvane::ui::test; +using Catch::Matchers::WithinAbs; + +TEST_CASE("a new session is in slot A with nothing to undo", "[session]") { + const Session s; + CHECK(s.getABSlot() == 0); + CHECK_FALSE(s.ab_states[0].isValid()); + CHECK_FALSE(s.ab_states[1].isValid()); + CHECK(s.current_preset == juce::File()); + CHECK_FALSE(s.undo_manager.canUndo()); +} + +TEST_CASE("the first switch to B starts B as a copy of A", "[session][ab]") { + FakeProcessor p; + p.setPlain(ids::kGain, 4.f); + p.setPlain(ids::kMode, 2.f); + p.session_.switchAB(p.parameters_); + + CHECK(p.session_.getABSlot() == 1); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(4.0, 1e-4)); + CHECK(p.plain(ids::kMode) == 2.f); + CHECK(p.session_.ab_states[0].isValid()); + CHECK(p.session_.ab_states[1].isValid()); +} + +TEST_CASE("switching A/B swaps the parameter states both ways", "[session][ab]") { + FakeProcessor p; + p.setPlain(ids::kGain, 4.f); + p.session_.switchAB(p.parameters_); + + // B + p.editor_.setNow({{p.para(ids::kGain), -8.f}, {p.para(ids::kFreq), 120.f}}, "Edit B"); + p.editor_.toggle(p.para(ids::kBypass), "Bypass"); + p.session_.switchAB(p.parameters_); + + // back in A + CHECK(p.session_.getABSlot() == 0); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(4.0, 1e-4)); + CHECK_THAT(p.plain(ids::kFreq), WithinAbs(1000.0, 1e-2)); + CHECK(p.plain(ids::kBypass) == 0.f); + + p.setPlain(ids::kGain, 1.f); + p.session_.switchAB(p.parameters_); + CHECK(p.session_.getABSlot() == 1); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(-8.0, 1e-4)); + CHECK_THAT(p.plain(ids::kFreq), WithinAbs(120.0, 1e-2)); + CHECK(p.plain(ids::kBypass) == 1.f); + + // A kept the change made in it before the switch + p.session_.switchAB(p.parameters_); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(1.0, 1e-4)); +} + +TEST_CASE("switching A/B clears the edit history", "[session][ab]") { + FakeProcessor p; + p.editor_.setNow({{p.para(ids::kGain), 3.f}}, "Edit A"); + REQUIRE(p.session_.undo_manager.canUndo()); + p.session_.switchAB(p.parameters_); + CHECK_FALSE(p.session_.undo_manager.canUndo()); + CHECK_FALSE(p.session_.undo_manager.canRedo()); + + // so undo can't carry A's edit into B + p.editor_.undo(); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(3.0, 1e-4)); +} + +TEST_CASE("copying to the other slot overwrites it with the current state", "[session][ab]") { + FakeProcessor p; + p.session_.switchAB(p.parameters_); + p.setPlain(ids::kGain, -2.f); + p.session_.switchAB(p.parameters_); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(0.0, 1e-4)); + + // in A: copy A over B, then B reads like A + p.setPlain(ids::kGain, 7.f); + p.session_.copyToOtherSlot(p.parameters_); + CHECK(p.session_.getABSlot() == 0); + p.session_.switchAB(p.parameters_); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(7.0, 1e-4)); +} + +TEST_CASE("copying to the other slot before any switch seeds it", "[session][ab]") { + FakeProcessor p; + p.setPlain(ids::kGain, 5.f); + p.session_.copyToOtherSlot(p.parameters_); + p.setPlain(ids::kGain, -5.f); + p.session_.switchAB(p.parameters_); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(5.0, 1e-4)); + p.session_.switchAB(p.parameters_); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(-5.0, 1e-4)); +} + +TEST_CASE("an out-of-range slot (e.g. from an old session) reads as A or B", "[session][ab]") { + FakeProcessor p; + p.session_.ab_slot = 7; + CHECK(p.session_.getABSlot() == 1); + p.session_.ab_slot = -3; + CHECK(p.session_.getABSlot() == 0); + + p.session_.ab_slot = 7; + p.setPlain(ids::kGain, 2.f); + p.session_.switchAB(p.parameters_); + CHECK(p.session_.ab_slot == 0); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(2.0, 1e-4)); +} + +TEST_CASE("the A/B slots hold copies, not the live state", "[session][ab]") { + FakeProcessor p; + p.session_.switchAB(p.parameters_); + // parameter changes in B must not leak into the stored A (or the B snapshot) behind the session's back + p.setPlain(ids::kGain, 9.f); + p.session_.switchAB(p.parameters_); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(0.0, 1e-4)); + p.setPlain(ids::kGain, -9.f); + p.session_.switchAB(p.parameters_); + CHECK_THAT(p.plain(ids::kGain), WithinAbs(9.0, 1e-4)); +} + +// ---------------------------------------------------------------- UISettings + +namespace { + std::unique_ptr openSettings(const juce::File& file) { + juce::PropertiesFile::Options options; + options.storageFormat = juce::PropertiesFile::storeAsXML; + options.millisecondsBeforeSaving = -1; + return std::make_unique(file, options); + } +} + +TEST_CASE("UI settings round-trip through their file", "[session][settings]") { + const TempDir temp; + const auto path = temp.get().getChildFile("settings.xml"); + { + UISettings s; + s.scale = 1.25f; + s.accent = 2; + s.reduce_motion = true; + s.drag_sensitivity = .5f; + s.wheel_sensitivity = 2.f; + s.switches["show_meters"] = false; + auto file = openSettings(path); + s.save(*file); + REQUIRE(file->save()); + } + UISettings s; + s.switches["show_meters"] = true; + s.switches["not_saved"] = true; + s.load(*openSettings(path)); + CHECK(s.scale == 1.25f); + CHECK(s.accent == 2); + CHECK(s.reduce_motion); + CHECK(s.drag_sensitivity == .5f); + CHECK(s.wheel_sensitivity == 2.f); + CHECK_FALSE(s.getSwitch("show_meters")); + // a switch the file doesn't have keeps its declared default; one never declared is off + CHECK(s.getSwitch("not_saved")); + CHECK_FALSE(s.getSwitch("never_declared")); +} + +TEST_CASE("UI settings from a damaged or edited file are clamped", "[session][settings]") { + const TempDir temp; + auto file = openSettings(temp.get().getChildFile("settings.xml")); + file->setValue("interface_scale", 40.0); + file->setValue("accent", 99); + file->setValue("drag_sensitivity", -1.0); + file->setValue("wheel_sensitivity", 1000.0); + + UISettings s; + s.load(*file); + CHECK(s.scale == UISettings::kScales.back()); + CHECK(s.accent == static_cast(tokens::colour::kAccentChoices.size()) - 1); + CHECK(s.drag_sensitivity == .25f); + CHECK(s.wheel_sensitivity == 4.f); + + file->setValue("interface_scale", 0.0); + file->setValue("accent", -4); + s.load(*file); + CHECK(s.scale == UISettings::kScales.front()); + CHECK(s.accent == 0); +} + +TEST_CASE("an empty settings file gives the defaults", "[session][settings]") { + const TempDir temp; + UISettings s; + s.load(*openSettings(temp.get().getChildFile("settings.xml"))); + CHECK(s.scale == 1.f); + CHECK(s.accent == 0); + CHECK_FALSE(s.reduce_motion); + CHECK(s.drag_sensitivity == 1.f); + CHECK(s.wheel_sensitivity == 1.f); +} + +TEST_CASE("applying UI settings sets the shared theme and interaction state", "[session][settings]") { + const auto accent = tokens::colour::kAccent; + UISettings s; + s.accent = 1; + s.reduce_motion = true; + s.drag_sensitivity = 2.f; + s.wheel_sensitivity = .5f; + s.apply(); + CHECK(tokens::colour::kAccent == tokens::colour::kAccentChoices[1].second); + CHECK(InteractionSettings::reduce_motion); + CHECK(InteractionSettings::drag_sensitivity == 2.f); + CHECK(InteractionSettings::wheel_sensitivity == .5f); + + UISettings{}.apply(); + CHECK_FALSE(InteractionSettings::reduce_motion); + CHECK(InteractionSettings::drag_sensitivity == 1.f); + tokens::colour::kAccent = accent; +} diff --git a/libs/ui/tests/util_tests.cpp b/libs/ui/tests/util_tests.cpp new file mode 100644 index 00000000..ed8e0c06 --- /dev/null +++ b/libs/ui/tests/util_tests.cpp @@ -0,0 +1,134 @@ +// Copyright (C) 2026 - Altvane +// This file is part of Altvane +// +// Altvane is free software: you can redistribute it and/or modify it under the terms of the GNU Affero General Public License Version 3 as published by the Free Software Foundation. +// +// Altvane is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License along with Altvane. If not, see . +#include +#include + +#include +#include + +#include +#include +#include + +using namespace altvane::ui; +using Catch::Matchers::WithinAbs; +using Catch::Matchers::WithinRel; + +TEST_CASE("the queue delivers every item in order and drops what doesn't fit", "[util][queue]") { + SpscQueue q; + int v{0}; + CHECK_FALSE(q.pop(v)); + for (int i = 0; i < 4; ++i) { + CHECK(q.push(i)); + } + CHECK_FALSE(q.push(99)); + CHECK(q.size() == 4); + for (int i = 0; i < 4; ++i) { + REQUIRE(q.pop(v)); + CHECK(v == i); + } + CHECK_FALSE(q.pop(v)); + + // wrapping around the ring many times + for (int i = 0; i < 100; ++i) { + REQUIRE(q.push(i)); + REQUIRE(q.push(i + 1000)); + REQUIRE(q.pop(v)); + CHECK(v == i); + REQUIRE(q.pop(v)); + CHECK(v == i + 1000); + } + + q.push(1); + q.push(2); + q.clear(); + CHECK(q.size() == 0); + CHECK_FALSE(q.pop(v)); +} + +TEST_CASE("the queue hands items across threads in order", "[util][queue]") { + SpscQueue q; + constexpr int kCount = 100000; + std::thread producer([&] { + for (int i = 0; i < kCount;) { + if (q.push(i)) { + ++i; + } else { + std::this_thread::yield(); + } + } + }); + int expected{0}; + bool in_order{true}; + while (expected < kCount) { + if (int v; q.pop(v)) { + in_order = in_order && v == expected; + ++expected; + } else { + std::this_thread::yield(); + } + } + producer.join(); + CHECK(in_order); +} + +TEST_CASE("the triple buffer gives the reader the latest publication", "[util][tri_buffer]") { + TriBuffer b; + CHECK_FALSE(b.pull()); + + b.getWriter() = 1; + b.publish(); + b.getWriter() = 2; + b.publish(); + REQUIRE(b.pull()); + CHECK(b.getReader() == 2); + CHECK_FALSE(b.pull()); + CHECK(b.getReader() == 2); + + b.getWriter() = 3; + b.publish(); + REQUIRE(b.pull()); + CHECK(b.getReader() == 3); +} + +TEST_CASE("the frequency axis is logarithmic from 10 Hz to 30 kHz", "[util][scale]") { + FreqScale s; + s.setBounds(100.f, 600.f); + CHECK_THAT(s.freqToX(FreqScale::kMinFreq), WithinAbs(100.0, 1e-3)); + CHECK_THAT(s.freqToX(FreqScale::kMaxFreq), WithinAbs(700.0, 1e-3)); + // equal ratios, equal distances + CHECK_THAT(s.freqToX(200.0) - s.freqToX(100.0), WithinAbs(s.freqToX(2000.0) - s.freqToX(1000.0), 1e-3)); + for (const auto hz : {20.0, 440.0, 1000.0, 15000.0}) { + CHECK_THAT(s.xToFreq(s.freqToX(hz)), WithinRel(hz, 1e-5)); + } + // zero and negative frequencies don't produce NaN + CHECK(std::isfinite(s.freqToX(0.0))); + CHECK(std::isfinite(s.freqToX(-5.0))); + + CHECK_THAT(FreqScale::pointToFreq(0, 11), WithinRel(FreqScale::kMinFreq, 1e-9)); + CHECK_THAT(FreqScale::pointToFreq(10, 11), WithinRel(FreqScale::kMaxFreq, 1e-9)); + CHECK_THAT(FreqScale::pointToFreq(0, 1), WithinRel(FreqScale::kMinFreq, 1e-9)); + + // a zero-width axis is treated as one pixel wide rather than dividing by zero + s.setBounds(0.f, 0.f); + CHECK(s.getWidth() == 1.f); + CHECK(std::isfinite(s.xToFreq(0.5f))); +} + +TEST_CASE("the gain axis is linear and centred on 0 dB", "[util][scale]") { + GainScale s; + s.setBounds(0.f, 220.f, 10.f); + s.setMaxDB(12.f); + CHECK(s.dbToY(0.f) == 110.f); + CHECK_THAT(s.dbToY(12.f), WithinAbs(10.0, 1e-4)); + CHECK_THAT(s.dbToY(-12.f), WithinAbs(210.0, 1e-4)); + CHECK_THAT(s.yToDB(s.dbToY(5.5f)), WithinAbs(5.5, 1e-4)); + s.setMaxDB(0.f); + CHECK(s.getMaxDB() > 0.f); +} diff --git a/libs/ui/tests/value_format_tests.cpp b/libs/ui/tests/value_format_tests.cpp new file mode 100644 index 00000000..53326a71 --- /dev/null +++ b/libs/ui/tests/value_format_tests.cpp @@ -0,0 +1,203 @@ +// Copyright (C) 2026 - Altvane +// This file is part of Altvane +// +// Altvane is free software: you can redistribute it and/or modify it under the terms of the GNU Affero General Public License Version 3 as published by the Free Software Foundation. +// +// Altvane is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License along with Altvane. If not, see . +#include +#include + +#include +#include +#include +#include +#include + +#include + +using namespace altvane::ui; +using Catch::Matchers::WithinAbs; +using Catch::Matchers::WithinRel; + +namespace { + const juce::String kMinus = format::kMinus; + + /** + * switch the C and C++ locales to one with a decimal comma, if the system has one, for the scope + */ + class CommaLocale final { + public: + CommaLocale() { + previous_ = std::setlocale(LC_ALL, nullptr); + for (const auto* name : {"de_DE.UTF-8", "de_DE.utf8", "de_DE", "fr_FR.UTF-8", "German_Germany.1252", "de-DE"}) { + if (std::setlocale(LC_ALL, name) != nullptr) { + active_ = true; + try { + cpp_previous_ = std::locale::global(std::locale(name)); + } catch (const std::runtime_error&) { + } + break; + } + } + } + + ~CommaLocale() { + std::setlocale(LC_ALL, previous_.c_str()); + std::locale::global(cpp_previous_); + } + + bool isActive() const { return active_; } + + private: + std::string previous_; + std::locale cpp_previous_{std::locale()}; + bool active_{false}; + }; +} + +TEST_CASE("frequencies read in Hz below 1 kHz and kHz above", "[format]") { + CHECK(format::freq(20.0) == "20.0 Hz"); + CHECK(format::freq(56.78) == "56.8 Hz"); + CHECK(format::freq(440.0) == "440 Hz"); + CHECK(format::freq(1234.0) == "1.23 kHz"); + CHECK(format::freq(12345.0) == "12.3 kHz"); + CHECK(format::freq(20000.0) == "20.0 kHz"); + CHECK(format::freq(1234.0, false) == "1.23"); + CHECK(format::freq(440.0, false) == "440"); +} + +TEST_CASE("gains have a sign, a typographic minus and fewer decimals from 10 dB", "[format]") { + CHECK(format::gain(3.0) == "+3.00 dB"); + CHECK(format::gain(-3.0) == kMinus + "3.00 dB"); + CHECK(format::gain(-12.34) == kMinus + "12.3 dB"); + CHECK(format::gain(12.0, false) == "+12.0"); + CHECK(format::gain(3.0, true, false) == "3.00 dB"); + CHECK(format::gain(-3.0, true, false) == kMinus + "3.00 dB"); +} + +TEST_CASE("a gain that rounds to zero has no sign", "[format]") { + CHECK(format::gain(0.0) == "0.00 dB"); + CHECK(format::gain(-0.0) == "0.00 dB"); + CHECK(format::gain(-0.004) == "0.00 dB"); + CHECK(format::gain(0.004) == "0.00 dB"); +} + +TEST_CASE("an infinite gain reads as inf", "[format]") { + const auto inf = std::numeric_limits::infinity(); + CHECK(format::gain(-inf) == kMinus + "inf dB"); + CHECK(format::gain(inf) == "+inf dB"); +} + +TEST_CASE("q, times, percentages and notes", "[format]") { + CHECK(format::q(0.707) == "0.71"); + CHECK(format::q(12.0) == "12.0"); + + CHECK(format::ms(12.34) == "12.3 ms"); + CHECK(format::ms(250.0) == "250 ms"); + CHECK(format::ms(1500.0) == "1.50 s"); + + CHECK(format::percent(49.6) == "50%"); + CHECK(format::percent(0.0) == "0%"); + + CHECK(format::note(440.0) == "A4"); + CHECK(format::note(261.6256) == "C4"); + CHECK(format::note(445.0) == "A4 +20"); + CHECK(format::note(435.0) == "A4 " + kMinus + "20"); + CHECK(format::note(27.5) == "A0"); + CHECK(format::note(8.1758) == "C-1"); + CHECK(format::note(5.0).isEmpty()); +} + +TEST_CASE("numbers parse with signs, units and either minus", "[format][parse]") { + CHECK(format::parseNumber("-3") == -3.0); + CHECK(format::parseNumber("+3.5") == 3.5); + CHECK(format::parseNumber("3.5 dB") == 3.5); + CHECK(format::parseNumber("3.5dB") == 3.5); + CHECK(format::parseNumber(" -12 DB ") == -12.0); + CHECK(format::parseNumber(kMinus + "6") == -6.0); + CHECK(format::parseNumber(kMinus + "6.5 dB") == -6.5); + CHECK(format::parseNumber("50%") == 50.0); + CHECK(format::parseNumber(".5") == 0.5); + CHECK(format::parseNumber("-.5") == -0.5); + CHECK(format::parseNumber("0") == 0.0); + // units products parse themselves (e.g. ms and s) are left to them, after the number + CHECK(format::parseNumber("20 ms") == 20.0); +} + +TEST_CASE("numbers parse a decimal comma", "[format][parse]") { + CHECK(format::parseNumber("1,5") == 1.5); + CHECK(format::parseNumber(kMinus + "2,25 dB") == -2.25); +} + +TEST_CASE("text without a leading number does not parse", "[format][parse]") { + for (const auto* junk : {"", " ", "dB", "%", "abc", "-", "+", ".", "inf", "nan"}) { + INFO("text: '" << junk << "'"); + CHECK_FALSE(format::parseNumber(junk).has_value()); + } +} + +TEST_CASE("infinities do not parse as numbers", "[format][parse]") { + // parameters have finite ranges: "-inf" is a product's word for its minimum, not a number + CHECK_FALSE(format::parseNumber("-inf").has_value()); + CHECK_FALSE(format::parseNumber(kMinus + "inf dB").has_value()); + CHECK_FALSE(format::parseNumber("1e999").has_value()); + CHECK_FALSE(format::parseNumber("-1e999").has_value()); +} + +TEST_CASE("frequencies parse with and without units", "[format][parse]") { + CHECK(format::parseFreq("1200") == 1200.0); + CHECK(format::parseFreq("1.2k") == 1200.0); + CHECK(format::parseFreq("1.2 kHz") == 1200.0); + CHECK(format::parseFreq("1.2KHZ") == 1200.0); + CHECK(format::parseFreq("1k2") == 1200.0); + CHECK(format::parseFreq("500 Hz") == 500.0); + CHECK(format::parseFreq("500hz") == 500.0); + CHECK(format::parseFreq("1,5k") == 1500.0); + CHECK(format::parseFreq(" 80 ") == 80.0); +} + +TEST_CASE("frequencies parse from note names", "[format][parse]") { + CHECK_THAT(*format::parseFreq("A4"), WithinAbs(440.0, 1e-9)); + CHECK_THAT(*format::parseFreq("a4"), WithinAbs(440.0, 1e-9)); + CHECK_THAT(*format::parseFreq("C4"), WithinRel(261.6256, 1e-5)); + CHECK_THAT(*format::parseFreq("C#3"), WithinRel(138.5913, 1e-5)); + CHECK_THAT(*format::parseFreq("Eb2"), WithinRel(77.7817, 1e-5)); + CHECK_THAT(*format::parseFreq("Bb1"), WithinRel(58.2705, 1e-5)); + CHECK_THAT(*format::parseFreq("B2"), WithinRel(123.4708, 1e-5)); + CHECK_THAT(*format::parseFreq("C-1"), WithinRel(8.1758, 1e-4)); +} + +TEST_CASE("frequencies that are not positive numbers or notes do not parse", "[format][parse]") { + for (const auto* junk : {"", "hz", "khz", "k", "-100", "0", "0 Hz", "abc", "12x", "x12", "h4", "a", "c#", "bb", + "1e3", "A4 Hz"}) { + INFO("text: '" << junk << "'"); + CHECK_FALSE(format::parseFreq(junk).has_value()); + } +} + +TEST_CASE("formatted values parse back to what they show", "[format][parse]") { + for (const auto hz : {20.0, 56.8, 440.0, 1230.0, 12300.0}) { + INFO(hz << " Hz"); + CHECK_THAT(*format::parseFreq(format::freq(hz)), WithinRel(hz, 1e-9)); + } + for (const auto db : {-30.5, -6.25, 0.0, 3.5, 18.0}) { + INFO(db << " dB"); + CHECK_THAT(*format::parseNumber(format::gain(db)), WithinAbs(db, 1e-9)); + } + CHECK(format::parseNumber(format::percent(42.0)) == 42.0); +} + +TEST_CASE("formatting and parsing ignore the process locale", "[format][parse]") { + const CommaLocale locale; + if (!locale.isActive()) { + SKIP("no locale with a decimal comma is installed"); + } + CHECK(format::freq(1234.0) == "1.23 kHz"); + CHECK(format::gain(-3.5) == kMinus + "3.50 dB"); + CHECK(format::ms(1500.0) == "1.50 s"); + CHECK(format::parseNumber("1.5") == 1.5); + CHECK(format::parseNumber("1,5") == 1.5); + CHECK(format::parseFreq("1.2k") == 1200.0); +} From 1bb3cc9dc2b28798c8549a40cc9f8ba805c1ee67 Mon Sep 17 00:00:00 2001 From: Bailey Pumfleet Date: Mon, 2 Nov 2026 11:35:56 -0600 Subject: [PATCH 04/10] Fix value formatting at range boundaries and junk before a number Found by the new libs/ui tests: - format::ms() printed values from 100 ms to 1 s with every decimal: juce::String(v, 0) means "as many as needed", not none, so 250.4 ms read "250.4 ms" and 333.333 ms "333.333 ms". The Limiter's, EQ's and De-Esser's time controls use it. Whole milliseconds are rounded now. - Values that round up to a range boundary took the lower range's format, so one number had two spellings: 99.97 Hz read "100.0 Hz" (100 Hz reads "100 Hz"), 999.7 Hz "1000 Hz", 9999.7 Hz "10.00 kHz", 9.996 dB "+10.00 dB", q 9.996 "10.00", 99.97 ms "100.0 ms" and 999.7 ms "999.7 ms". freq, gain, q and ms now pick the format by the number as shown. - parseNumber accepted text with junk before the number and read it as 0: typing "x5", "--3" or "gain 3" into any control bound with bindValue set the parameter to 0, as an undoable edit. It now wants the number first (a sign and a leading "." are fine); units after it are still the caller's. - parseFreq read "1.2.3" as 1.2 Hz and "1.5k2" as 1.5 kHz; two decimal points no longer parse. Co-Authored-By: Claude Opus 5.5 --- .../source/altvane_ui/util/value_format.cpp | 31 +++++++++++----- libs/ui/tests/param_editor_tests.cpp | 10 ++++++ libs/ui/tests/value_format_tests.cpp | 36 +++++++++++++++++++ 3 files changed, 68 insertions(+), 9 deletions(-) diff --git a/libs/ui/source/altvane_ui/util/value_format.cpp b/libs/ui/source/altvane_ui/util/value_format.cpp index fa7d612a..9673435c 100644 --- a/libs/ui/source/altvane_ui/util/value_format.cpp +++ b/libs/ui/source/altvane_ui/util/value_format.cpp @@ -19,6 +19,15 @@ namespace altvane::ui::format { return text.trim().replace(kMinus, "-").replace(",", ".").toLowerCase(); } + /** + * whether v, shown with this many decimals, reads below limit: picks a format by the number as shown, + * so a value that rounds up to a boundary (99.97 Hz) reads like the boundary ("100 Hz", not "100.0 Hz") + */ + bool showsBelow(const double v, const double limit, const int decimals) { + const auto scale = std::pow(10.0, decimals); + return std::round(v * scale) / scale < limit; + } + juce::String signedNumber(const double v, const int decimals, const bool force_sign) { const auto rounded = std::round(v * std::pow(10.0, decimals)) / std::pow(10.0, decimals); const auto magnitude = juce::String(std::abs(rounded), decimals); @@ -35,13 +44,13 @@ namespace altvane::ui::format { juce::String freq(const double hz, const bool with_unit) { juce::String s; juce::String unit; - if (hz < 100.0) { + if (showsBelow(hz, 100.0, 1)) { s = juce::String(hz, 1); unit = " Hz"; - } else if (hz < 1000.0) { + } else if (showsBelow(hz, 1000.0, 0)) { s = juce::String(juce::roundToInt(hz)); unit = " Hz"; - } else if (hz < 10000.0) { + } else if (showsBelow(hz / 1000.0, 10.0, 2)) { s = juce::String(hz / 1000.0, 2); unit = " kHz"; } else { @@ -52,19 +61,20 @@ namespace altvane::ui::format { } juce::String gain(const double db, const bool with_unit, const bool force_sign) { - const auto s = signedNumber(db, std::abs(db) < 10.0 ? 2 : 1, force_sign); + const auto s = signedNumber(db, showsBelow(std::abs(db), 10.0, 2) ? 2 : 1, force_sign); return with_unit ? s + " dB" : s; } juce::String q(const double q) { - return juce::String(q, q < 10.0 ? 2 : 1); + return juce::String(q, showsBelow(q, 10.0, 2) ? 2 : 1); } juce::String ms(const double ms) { - if (ms >= 1000.0) { + if (!showsBelow(ms, 1000.0, 0)) { return juce::String(ms / 1000.0, 2) + " s"; } - return juce::String(ms, ms < 100.0 ? 1 : 0) + " ms"; + // juce::String(v, 0) prints every decimal, so whole milliseconds are rounded here + return (showsBelow(ms, 100.0, 1) ? juce::String(ms, 1) : juce::String(juce::roundToInt(ms))) + " ms"; } juce::String percent(const double p) { @@ -90,7 +100,10 @@ namespace altvane::ui::format { std::optional parseNumber(const juce::String& text) { const auto t = normalise(text).removeCharacters(" db%"); - if (t.isEmpty() || !t.containsAnyOf("0123456789")) { + // the number comes first ("-3", "+.5"); text after it, such as a unit, is the caller's + const auto unsigned_part = t.startsWithChar('-') || t.startsWithChar('+') ? t.substring(1) : t; + const auto digits = unsigned_part.startsWithChar('.') ? unsigned_part.substring(1) : unsigned_part; + if (!juce::CharacterFunctions::isDigit(digits[0])) { return std::nullopt; } const auto v = t.getDoubleValue(); @@ -135,7 +148,7 @@ namespace altvane::ui::format { t = t.replaceCharacter('k', '.'); scale = 1000.0; } - if (!t.containsAnyOf("0123456789") || !t.containsOnly("0123456789.")) { + if (!t.containsAnyOf("0123456789") || !t.containsOnly("0123456789.") || t.indexOfChar('.') != t.lastIndexOfChar('.')) { return std::nullopt; } const auto v = t.getDoubleValue() * scale; diff --git a/libs/ui/tests/param_editor_tests.cpp b/libs/ui/tests/param_editor_tests.cpp index 3b2f6a90..0ca4212d 100644 --- a/libs/ui/tests/param_editor_tests.cpp +++ b/libs/ui/tests/param_editor_tests.cpp @@ -314,6 +314,16 @@ TEST_CASE("typing junk changes nothing and leaves no undo step", "[param_editor] CHECK_FALSE(b.p.session_.undo_manager.canUndo()); } +TEST_CASE("typing junk before a number changes nothing", "[param_editor][typing]") { + // it used to set the parameter to 0, as an undoable edit + BoundKnob b; + b.p.setPlain(ids::kGain, 3.f); + b.type("x5"); + CHECK_THAT(b.p.plain(ids::kGain), WithinAbs(3.0, 1e-4)); + CHECK(b.gestures.begins == 0); + CHECK_FALSE(b.p.session_.undo_manager.canUndo()); +} + TEST_CASE("escape closes the text entry without a change", "[param_editor][typing]") { BoundKnob b; b.type("12", juce::KeyPress::escapeKey); diff --git a/libs/ui/tests/value_format_tests.cpp b/libs/ui/tests/value_format_tests.cpp index 53326a71..9aad06db 100644 --- a/libs/ui/tests/value_format_tests.cpp +++ b/libs/ui/tests/value_format_tests.cpp @@ -68,6 +68,13 @@ TEST_CASE("frequencies read in Hz below 1 kHz and kHz above", "[format]") { CHECK(format::freq(440.0, false) == "440"); } +TEST_CASE("frequencies that round up to a range boundary read as the next range", "[format]") { + // as shown, never "100.0 Hz", "1000 Hz" or "10.00 kHz": the same frequency must not have two spellings + CHECK(format::freq(99.97) == "100 Hz"); + CHECK(format::freq(999.7) == "1.00 kHz"); + CHECK(format::freq(9999.7) == "10.0 kHz"); +} + TEST_CASE("gains have a sign, a typographic minus and fewer decimals from 10 dB", "[format]") { CHECK(format::gain(3.0) == "+3.00 dB"); CHECK(format::gain(-3.0) == kMinus + "3.00 dB"); @@ -84,12 +91,26 @@ TEST_CASE("a gain that rounds to zero has no sign", "[format]") { CHECK(format::gain(0.004) == "0.00 dB"); } +TEST_CASE("a gain that rounds up to 10 dB reads with the decimals of 10 dB", "[format]") { + CHECK(format::gain(9.996) == "+10.0 dB"); + CHECK(format::gain(-9.996) == kMinus + "10.0 dB"); + CHECK(format::q(9.996) == "10.0"); +} + TEST_CASE("an infinite gain reads as inf", "[format]") { const auto inf = std::numeric_limits::infinity(); CHECK(format::gain(-inf) == kMinus + "inf dB"); CHECK(format::gain(inf) == "+inf dB"); } +TEST_CASE("times from 100 ms read in whole milliseconds, and from 1 s in seconds", "[format]") { + CHECK(format::ms(250.4) == "250 ms"); + CHECK(format::ms(333.333) == "333 ms"); + // as shown: never "100.0 ms" or "1000 ms" + CHECK(format::ms(99.97) == "100 ms"); + CHECK(format::ms(999.7) == "1.00 s"); +} + TEST_CASE("q, times, percentages and notes", "[format]") { CHECK(format::q(0.707) == "0.71"); CHECK(format::q(12.0) == "12.0"); @@ -138,6 +159,14 @@ TEST_CASE("text without a leading number does not parse", "[format][parse]") { } } +TEST_CASE("a number after junk does not parse", "[format][parse]") { + // rather than reading as 0 + for (const auto* junk : {"x5", "--3", "+-3", "gain 3"}) { + INFO("text: '" << junk << "'"); + CHECK_FALSE(format::parseNumber(junk).has_value()); + } +} + TEST_CASE("infinities do not parse as numbers", "[format][parse]") { // parameters have finite ranges: "-inf" is a product's word for its minimum, not a number CHECK_FALSE(format::parseNumber("-inf").has_value()); @@ -177,6 +206,13 @@ TEST_CASE("frequencies that are not positive numbers or notes do not parse", "[f } } +TEST_CASE("a frequency with two decimal points does not parse", "[format][parse]") { + for (const auto* junk : {"1.2.3", "1..2", "1.2.3k", "1.5k2"}) { + INFO("text: '" << junk << "'"); + CHECK_FALSE(format::parseFreq(junk).has_value()); + } +} + TEST_CASE("formatted values parse back to what they show", "[format][parse]") { for (const auto hz : {20.0, 56.8, 440.0, 1230.0, 12300.0}) { INFO(hz << " Hz"); From b70ce8e88c7f1b91ed934cda2acbb6c73a89dd78 Mon Sep 17 00:00:00 2001 From: Bailey Pumfleet Date: Mon, 2 Nov 2026 11:36:24 -0600 Subject: [PATCH 05/10] Keep a preset that couldn't be removed, and allow renaming its case Found by the new libs/ui tests: - PresetManager::remove forgot the preset as favourite and as the current one before trying to move it to the trash, so when that failed (no trash for the folder, no permission) the file stayed but lost its star and the header fell back to "Default". It now forgets only once the file is gone. The test makes the folder read-only (Linux and macOS). - rename refused any name whose file already exists, which on a case-insensitive file system (macOS, Windows) includes the preset's own file under another case: "bass" could never become "Bass", and renaming to the same name failed. The target is now a collision only when it's another file; File::moveFileTo handles a case-only move. Only the same-name case can fail on Linux, so CI's macOS and Windows runs are what check the case-only rename. Co-Authored-By: Claude Opus 5.5 --- .../altvane_ui/session/preset_manager.cpp | 9 +++- libs/ui/tests/preset_manager_tests.cpp | 47 +++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/libs/ui/source/altvane_ui/session/preset_manager.cpp b/libs/ui/source/altvane_ui/session/preset_manager.cpp index 305997c7..0a0ea79c 100644 --- a/libs/ui/source/altvane_ui/session/preset_manager.cpp +++ b/libs/ui/source/altvane_ui/session/preset_manager.cpp @@ -206,7 +206,8 @@ namespace altvane::ui { return false; } const auto target = file.getSiblingFile(name + kExtension); - if (target.exists() || !file.moveFileTo(target)) { + // on case-insensitive file systems a new case of the same name is this file, not another preset + if ((target.exists() && target != file) || !file.moveFileTo(target)) { return false; } if (isFavourite(file)) { @@ -220,11 +221,15 @@ namespace altvane::ui { } bool PresetManager::remove(const juce::File& file) { + // a preset that stays (no trash, no permission) stays current and favourite + if (!file.moveToTrash()) { + return false; + } setFavourite(file, false); if (getCurrentFile() == file) { session_.current_preset = juce::File(); } - return file.moveToTrash(); + return true; } juce::File PresetManager::createGroup(const juce::String& name) const { diff --git a/libs/ui/tests/preset_manager_tests.cpp b/libs/ui/tests/preset_manager_tests.cpp index 3dbafe19..53e6a090 100644 --- a/libs/ui/tests/preset_manager_tests.cpp +++ b/libs/ui/tests/preset_manager_tests.cpp @@ -321,6 +321,14 @@ TEST_CASE("renaming only the case of a preset's name works", "[presets][rename]" CHECK(t.presets.getCurrentName() == "Bass"); } +TEST_CASE("renaming a preset to its own name succeeds and changes nothing", "[presets][rename]") { + Presets t; + REQUIRE(t.presets.save(t.file("Bass"))); + CHECK(t.presets.rename(t.file("Bass"), "Bass")); + CHECK(t.names() == juce::StringArray{"Bass"}); + CHECK(t.presets.getCurrentName() == "Bass"); +} + TEST_CASE("renaming to an empty or illegal name", "[presets][rename]") { Presets t; REQUIRE(t.presets.save(t.file("Keep"))); @@ -335,6 +343,45 @@ TEST_CASE("renaming to an empty or illegal name", "[presets][rename]") { CHECK(t.presets.getCurrentFile().existsAsFile()); } +TEST_CASE("removing a preset forgets it as current and favourite", "[presets][remove]") { + Presets t; + REQUIRE(t.presets.save(t.file("Doomed"))); + t.presets.setFavourite(t.file("Doomed"), true); + REQUIRE(t.presets.save(t.file("Kept"))); + REQUIRE(t.presets.load(t.file("Doomed"))); + + REQUIRE(t.presets.remove(t.file("Doomed"))); + CHECK_FALSE(t.file("Doomed").exists()); + CHECK(t.presets.getCurrentFile() == juce::File()); + CHECK(t.presets.getCurrentName() == "Default"); + CHECK(t.settings->getValue("favourite_presets").isEmpty()); + CHECK(t.names() == juce::StringArray{"Kept"}); +} + +TEST_CASE("a preset that can't be removed stays current and favourite", "[presets][remove]") { +#if JUCE_LINUX || JUCE_MAC + Presets t; + const auto locked = Presets::dir().getChildFile("Locked"); + REQUIRE(t.presets.save(t.file("Locked/Stuck"))); + t.presets.setFavourite(t.file("Locked/Stuck"), true); + REQUIRE(chmod(locked.getFullPathName().toRawUTF8(), 0555) == 0); + struct Unlock { + juce::File dir; + ~Unlock() { chmod(dir.getFullPathName().toRawUTF8(), 0755); } + } unlock{locked}; + if (locked.getChildFile("probe").create().wasOk()) { + SKIP("this user can write to a read-only folder (root?)"); + } + + CHECK_FALSE(t.presets.remove(t.file("Locked/Stuck"))); + CHECK(t.file("Locked/Stuck").existsAsFile()); + CHECK(t.presets.getCurrentFile() == t.file("Locked/Stuck")); + CHECK(t.presets.isFavourite(t.file("Locked/Stuck"))); +#else + SKIP("needs POSIX permissions to make a folder read-only"); +#endif +} + TEST_CASE("presets list in natural order with their groups and favourites", "[presets]") { Presets t; for (const auto* name : {"Preset 10", "Preset 2", "Drums/Kick", "Preset 1"}) { From 1b76dbbd4321bfda33c01008873e78aaf2c085fb Mon Sep 17 00:00:00 2001 From: Bailey Pumfleet Date: Mon, 2 Nov 2026 11:43:05 -0600 Subject: [PATCH 06/10] Let the libs/ui tests run under ctest -j and report skips as skips - Each test's temporary folder now has a UUID name: under ctest -j every test case is its own process, and two could pick the same unused name and delete each other's presets. - CTest counted Catch2's SKIP (exit code 4, the decimal-comma locale test where no such locale is installed) as a failure; SKIP_RETURN_CODE 4 makes it a skip. Co-Authored-By: Claude Opus 5.5 --- libs/ui/tests/CMakeLists.txt | 3 ++- libs/ui/tests/fake_processor.hpp | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/libs/ui/tests/CMakeLists.txt b/libs/ui/tests/CMakeLists.txt index d5acb923..a756e3d2 100644 --- a/libs/ui/tests/CMakeLists.txt +++ b/libs/ui/tests/CMakeLists.txt @@ -34,4 +34,5 @@ target_link_libraries(AltvaneUITests ) set_target_properties(AltvaneUITests PROPERTIES FOLDER "Targets" XCODE_GENERATE_SCHEME ON) -catch_discover_tests(AltvaneUITests) +# Catch2 exits with 4 when a test SKIPs (e.g. no decimal-comma locale installed) +catch_discover_tests(AltvaneUITests PROPERTIES SKIP_RETURN_CODE 4) diff --git a/libs/ui/tests/fake_processor.hpp b/libs/ui/tests/fake_processor.hpp index 4b906e4a..37448764 100644 --- a/libs/ui/tests/fake_processor.hpp +++ b/libs/ui/tests/fake_processor.hpp @@ -122,7 +122,8 @@ namespace altvane::ui::test { class TempDir final { public: explicit TempDir(const juce::File& parent = juce::File::getSpecialLocation(juce::File::tempDirectory)) : - dir_(parent.getNonexistentChildFile("altvane_ui_tests", "", false)) { + // unique, not just unused: ctest -j runs each test case in its own process at the same time + dir_(parent.getChildFile("altvane_ui_tests_" + juce::Uuid().toString())) { dir_.createDirectory(); } From bdeb22a2f08c0acdc6a03d364e7b6f9384ab137c Mon Sep 17 00:00:00 2001 From: Bailey Pumfleet Date: Mon, 2 Nov 2026 11:49:45 -0600 Subject: [PATCH 07/10] Address Codex review (round 1) - PresetManager::rename asks the file system whether the target is the preset's own file (File::getFileIdentifier) instead of comparing paths. File's == ignores case on every macOS volume, so on a case-sensitive one renaming "bass" to "Bass" while both exist would have overwritten "Bass". - Sanitizer and Heavy Test: a runner whose every product is excluded keeps them. Otherwise its include entry, matching no combination, becomes a job of its own with no product (dispatching dialogue: the Linux entries configured with no products and failed on the missing .env). Dispatching a single product now builds it on every runner, as before this branch; only the schedule, with every product, skips Dialogue on Linux. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/cmake_heavy_test.yml | 7 +++++++ .github/workflows/sanitizer.yml | 7 +++++++ libs/ui/source/altvane_ui/session/preset_manager.cpp | 7 +++++-- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/.github/workflows/cmake_heavy_test.yml b/.github/workflows/cmake_heavy_test.yml index 85b8f4e1..ca6cd319 100644 --- a/.github/workflows/cmake_heavy_test.yml +++ b/.github/workflows/cmake_heavy_test.yml @@ -68,6 +68,13 @@ jobs: platforms = {line for line in lines if line and not line.startswith("#")} exclude += [{"product": product, "name": name} for name, platform in runners.items() if platform not in platforms] + # a runner must keep a product: matrix include would turn an entry whose name is left in no combination + # into a job of its own, with no product (e.g. dispatching dialogue). Such a runner builds what was asked. + for name in runners: + skipped = [e for e in exclude if e["name"] == name] + if len(skipped) == len(products): + print(f"::warning::{products} not supported on {name} ({runners[name]}); building it there anyway") + exclude = [e for e in exclude if e["name"] != name] with open(os.environ["GITHUB_OUTPUT"], "a") as out: out.write(f"products={json.dumps(products)}\n") out.write(f"names={json.dumps(list(runners))}\n") diff --git a/.github/workflows/sanitizer.yml b/.github/workflows/sanitizer.yml index b97d76c1..20391c13 100644 --- a/.github/workflows/sanitizer.yml +++ b/.github/workflows/sanitizer.yml @@ -68,6 +68,13 @@ jobs: platforms = {line for line in lines if line and not line.startswith("#")} exclude += [{"product": product, "name": name} for name, platform in runners.items() if platform not in platforms] + # a runner must keep a product: matrix include would turn an entry whose name is left in no combination + # into a job of its own, with no product (e.g. dispatching dialogue). Such a runner builds what was asked. + for name in runners: + skipped = [e for e in exclude if e["name"] == name] + if len(skipped) == len(products): + print(f"::warning::{products} not supported on {name} ({runners[name]}); building it there anyway") + exclude = [e for e in exclude if e["name"] != name] with open(os.environ["GITHUB_OUTPUT"], "a") as out: out.write(f"products={json.dumps(products)}\n") out.write(f"names={json.dumps(list(runners))}\n") diff --git a/libs/ui/source/altvane_ui/session/preset_manager.cpp b/libs/ui/source/altvane_ui/session/preset_manager.cpp index 0a0ea79c..a94951e3 100644 --- a/libs/ui/source/altvane_ui/session/preset_manager.cpp +++ b/libs/ui/source/altvane_ui/session/preset_manager.cpp @@ -206,8 +206,11 @@ namespace altvane::ui { return false; } const auto target = file.getSiblingFile(name + kExtension); - // on case-insensitive file systems a new case of the same name is this file, not another preset - if ((target.exists() && target != file) || !file.moveFileTo(target)) { + // on a case-insensitive file system a new case of the same name is this file, not another preset; ask the + // file system (File's == ignores case on every macOS volume, case-sensitive ones too) + const auto id = file.getFileIdentifier(); + const auto same_file = id != 0 && target.getFileIdentifier() == id; + if ((target.exists() && !same_file) || !file.moveFileTo(target)) { return false; } if (isFavourite(file)) { From 7a3ce256ce4ba750bb957567a2b205d3168780aa Mon Sep 17 00:00:00 2001 From: Bailey Pumfleet Date: Mon, 2 Nov 2026 12:05:47 -0600 Subject: [PATCH 08/10] Build the libs/ui tests with the products' JUCE configuration The Windows CI build failed compiling JUCE's bundled libwebp (clang-cl: "attribute 'target' multiversioning cannot be combined with attribute 'always_inline'"). Products never build it: altvane_add_plugin sets JUCE_USE_WEBP=0, as well as JUCE_USE_FLAC=0, JUCE_USE_OGGVORBIS=0 and JUCE_SILENCE_XCODE_15_LINKER_WARNING=1. The test app now does too. Co-Authored-By: Claude Opus 5.5 --- libs/ui/tests/CMakeLists.txt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/libs/ui/tests/CMakeLists.txt b/libs/ui/tests/CMakeLists.txt index a756e3d2..d1d2011d 100644 --- a/libs/ui/tests/CMakeLists.txt +++ b/libs/ui/tests/CMakeLists.txt @@ -12,8 +12,13 @@ target_compile_features(AltvaneUITests PRIVATE cxx_std_20) target_compile_definitions(AltvaneUITests PRIVATE + # the JUCE configuration altvane_add_plugin gives products (libwebp, for one, doesn't build with clang-cl here) JUCE_WEB_BROWSER=0 JUCE_USE_CURL=0 + JUCE_USE_FLAC=0 + JUCE_USE_OGGVORBIS=0 + JUCE_USE_WEBP=0 + JUCE_SILENCE_XCODE_15_LINKER_WARNING=1 # the typing tests pump the message loop, which JUCE only allows with modal loops (as in the product tests) JUCE_MODAL_LOOPS_PERMITTED=1 # what altvane_ui reads from a plugin's settings (juce_add_plugin, altvane_add_plugin) @@ -34,5 +39,5 @@ target_link_libraries(AltvaneUITests ) set_target_properties(AltvaneUITests PROPERTIES FOLDER "Targets" XCODE_GENERATE_SCHEME ON) -# Catch2 exits with 4 when a test SKIPs (e.g. no decimal-comma locale installed) +# Catch2 exits with 4 when a test SKIPs (the read-only folder test on Windows, or when run as root) catch_discover_tests(AltvaneUITests PROPERTIES SKIP_RETURN_CODE 4) From 52b0bf88d42567ee92708767bee8e7dd346f07ed Mon Sep 17 00:00:00 2001 From: Bailey Pumfleet Date: Mon, 2 Nov 2026 12:05:47 -0600 Subject: [PATCH 09/10] Format values with a decimal point whatever the host's C++ locale Found by the locale test on macOS CI: with a global C++ locale that has a decimal comma (std::locale::global, which a host may set), format::freq, gain, q and ms wrote "1,23 kHz" and "-3,50 dB". juce::String(double, n) imbues its stream buffer with the classic locale, but the std::ostream it formats through takes the global one. The formatters now build the digits themselves (rounded integer, then the decimals), which also gives ms its whole milliseconds without the juce::String(v, 0) special case. The test no longer depends on an installed German locale (it skipped on Linux CI): it sets a global C++ locale with a comma numpunct, and the C locale too where one is installed. Before this change it fails on Linux as well. Not fixed here: JUCE's serialiseDouble goes through the same stream, so under such a locale saved state and presets would hold "0,5"-style values. Co-Authored-By: Claude Opus 5.5 --- .../source/altvane_ui/util/value_format.cpp | 34 +++++++++++++----- libs/ui/tests/value_format_tests.cpp | 36 ++++++++++--------- 2 files changed, 46 insertions(+), 24 deletions(-) diff --git a/libs/ui/source/altvane_ui/util/value_format.cpp b/libs/ui/source/altvane_ui/util/value_format.cpp index 9673435c..05d07c1c 100644 --- a/libs/ui/source/altvane_ui/util/value_format.cpp +++ b/libs/ui/source/altvane_ui/util/value_format.cpp @@ -28,9 +28,28 @@ namespace altvane::ui::format { return std::round(v * scale) / scale < limit; } + /** + * v with this many decimals and always a decimal point: juce::String(v, decimals) follows the global C++ + * locale, which a host may set ("1,23" in German), and prints every decimal when decimals is 0 + */ + juce::String fixed(const double v, const int decimals) { + const auto scale = std::pow(10.0, decimals); + if (!std::isfinite(v) || std::abs(v) * scale >= 1e15) { + return std::isnan(v) ? juce::String("nan") : std::isinf(v) ? juce::String(v < 0.0 ? "-inf" : "inf") + : juce::String(v, decimals); + } + const auto n = std::llround(std::abs(v) * scale); + const auto unit = std::llround(scale); + auto s = juce::String(n / unit); + if (decimals > 0) { + s << "." << juce::String(n % unit).paddedLeft('0', decimals); + } + return v < 0.0 && n != 0 ? "-" + s : s; + } + juce::String signedNumber(const double v, const int decimals, const bool force_sign) { const auto rounded = std::round(v * std::pow(10.0, decimals)) / std::pow(10.0, decimals); - const auto magnitude = juce::String(std::abs(rounded), decimals); + const auto magnitude = fixed(std::abs(rounded), decimals); if (rounded < 0.0) { return kMinus + magnitude; } @@ -45,16 +64,16 @@ namespace altvane::ui::format { juce::String s; juce::String unit; if (showsBelow(hz, 100.0, 1)) { - s = juce::String(hz, 1); + s = fixed(hz, 1); unit = " Hz"; } else if (showsBelow(hz, 1000.0, 0)) { s = juce::String(juce::roundToInt(hz)); unit = " Hz"; } else if (showsBelow(hz / 1000.0, 10.0, 2)) { - s = juce::String(hz / 1000.0, 2); + s = fixed(hz / 1000.0, 2); unit = " kHz"; } else { - s = juce::String(hz / 1000.0, 1); + s = fixed(hz / 1000.0, 1); unit = " kHz"; } return with_unit ? s + unit : s; @@ -66,15 +85,14 @@ namespace altvane::ui::format { } juce::String q(const double q) { - return juce::String(q, showsBelow(q, 10.0, 2) ? 2 : 1); + return fixed(q, showsBelow(q, 10.0, 2) ? 2 : 1); } juce::String ms(const double ms) { if (!showsBelow(ms, 1000.0, 0)) { - return juce::String(ms / 1000.0, 2) + " s"; + return fixed(ms / 1000.0, 2) + " s"; } - // juce::String(v, 0) prints every decimal, so whole milliseconds are rounded here - return (showsBelow(ms, 100.0, 1) ? juce::String(ms, 1) : juce::String(juce::roundToInt(ms))) + " ms"; + return fixed(ms, showsBelow(ms, 100.0, 1) ? 1 : 0) + " ms"; } juce::String percent(const double p) { diff --git a/libs/ui/tests/value_format_tests.cpp b/libs/ui/tests/value_format_tests.cpp index 9aad06db..fb58369b 100644 --- a/libs/ui/tests/value_format_tests.cpp +++ b/libs/ui/tests/value_format_tests.cpp @@ -11,8 +11,9 @@ #include #include +#include #include -#include +#include #include #include @@ -25,19 +26,24 @@ namespace { const juce::String kMinus = format::kMinus; /** - * switch the C and C++ locales to one with a decimal comma, if the system has one, for the scope + * numbers as German writes them: 1.234,5 + */ + struct CommaDecimal final : std::numpunct { + char do_decimal_point() const override { return ','; } + char do_thousands_sep() const override { return '.'; } + std::string do_grouping() const override { return "\3"; } + }; + + /** + * for the scope, a global C++ locale with a decimal comma (as a host may set), and the C locale too where the + * system has one installed */ class CommaLocale final { public: - CommaLocale() { + CommaLocale() : cpp_previous_(std::locale::global(std::locale(std::locale::classic(), new CommaDecimal))) { previous_ = std::setlocale(LC_ALL, nullptr); - for (const auto* name : {"de_DE.UTF-8", "de_DE.utf8", "de_DE", "fr_FR.UTF-8", "German_Germany.1252", "de-DE"}) { + for (const auto* name : {"de_DE.UTF-8", "de_DE.utf8", "de_DE", "German_Germany.1252", "de-DE"}) { if (std::setlocale(LC_ALL, name) != nullptr) { - active_ = true; - try { - cpp_previous_ = std::locale::global(std::locale(name)); - } catch (const std::runtime_error&) { - } break; } } @@ -48,12 +54,9 @@ namespace { std::locale::global(cpp_previous_); } - bool isActive() const { return active_; } - private: + std::locale cpp_previous_; std::string previous_; - std::locale cpp_previous_{std::locale()}; - bool active_{false}; }; } @@ -227,9 +230,10 @@ TEST_CASE("formatted values parse back to what they show", "[format][parse]") { TEST_CASE("formatting and parsing ignore the process locale", "[format][parse]") { const CommaLocale locale; - if (!locale.isActive()) { - SKIP("no locale with a decimal comma is installed"); - } + // the test itself: the locale does write a decimal comma + std::ostringstream check; + check << std::fixed << std::setprecision(1) << 1.5; + REQUIRE(check.str() == "1,5"); CHECK(format::freq(1234.0) == "1.23 kHz"); CHECK(format::gain(-3.5) == kMinus + "3.50 dB"); CHECK(format::ms(1500.0) == "1.50 s"); From 7fbcd0f6349128f3973edc33ca4633b63b2235a7 Mon Sep 17 00:00:00 2001 From: Bailey Pumfleet Date: Mon, 2 Nov 2026 12:08:40 -0600 Subject: [PATCH 10/10] Address Codex review (round 2) - A case-only rename now goes through a temporary sibling. JUCE's moveFileTo deletes an existing target first wherever it takes names as case-sensitive (Linux), so on a case-insensitive folder there (casefold ext4) renaming "bass" to "Bass" would have deleted the preset: the target is the same file. If the second move fails, the first is undone. macOS and Windows CI run this path through the case-only rename test. Co-Authored-By: Claude Opus 5.5 --- .../source/altvane_ui/session/preset_manager.cpp | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/libs/ui/source/altvane_ui/session/preset_manager.cpp b/libs/ui/source/altvane_ui/session/preset_manager.cpp index a94951e3..dfe631d1 100644 --- a/libs/ui/source/altvane_ui/session/preset_manager.cpp +++ b/libs/ui/source/altvane_ui/session/preset_manager.cpp @@ -210,7 +210,21 @@ namespace altvane::ui { // file system (File's == ignores case on every macOS volume, case-sensitive ones too) const auto id = file.getFileIdentifier(); const auto same_file = id != 0 && target.getFileIdentifier() == id; - if ((target.exists() && !same_file) || !file.moveFileTo(target)) { + if (target.exists() && !same_file) { + return false; + } + if (same_file && target.getFullPathName() != file.getFullPathName()) { + // only the case changes: go through a temporary name, as moveFileTo deletes an existing target first + // where JUCE takes names as case-sensitive (Linux, which can still have case-insensitive folders) + const auto temp = file.getSiblingFile(juce::String(".rename") + kExtension).getNonexistentSibling(); + if (!file.moveFileTo(temp)) { + return false; + } + if (!temp.moveFileTo(target)) { + temp.moveFileTo(file); + return false; + } + } else if (!file.moveFileTo(target)) { return false; } if (isFavourite(file)) {