Repository navigation
Add libs/ui tests (and fix what they found); schedule upstream watch, sanitizers and heavy tests - #21
Merged
Merged
Conversation
- scripts/upstream_behind.sh <product> lists the upstream commits a product's vendored engine is missing (on upstream main since the pin in products/<product>/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 <noreply@anthropic.com>
- 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
- 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 <noreply@anthropic.com>
- 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
- 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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two pieces of infrastructure, independent of each other (happy to split them if that reviews better).
1. Tests for
libs/ui(AltvaneUITests, 78 Catch2 cases)libs/ui/testsis a Catch2 console app built underALTVANE_BUILD_TESTSand registered with CTest, so CI's existingctest --test-dir Buildsstep runs it on Linux, macOS and Windows with no workflow change.altvane_uiis an INTERFACE library, so the app compiles it and its JUCE modules itself, under a stand-in plugin name (Altvane UI Tests).SessionandParamEditorstand in for a product.value_format(formatting, parsing, units, signs, either minus, decimal comma, note names, junk, infinities, round trips, and a decimal-comma C locale);ParamEditor(setNow, drags, cancel, clamping and snapping, toggle, undo and redo order, undo mid-edit, gestures balanced throughout); typing into a boundKnobthrough its realTextEditorand the message loop, accessibilitysetValueAsString, and a wheel's lateon_end;SessionA/B (both directions, the first switch, history cleared, copy to the other slot, an out-of-range slot);UISettings;PresetManagerin a temporary folder (factory presets, save and load, modified tracking, six kinds of corrupt file, saving over a name, rename collisions, case-only rename, illegal names, remove, groups, favourites, natural order, step, initialise, the JSON codec);SpscQueue,TriBuffer,FreqScaleandGainScale.PresetManager::setDirectoryOverride, which only the tests call. It keeps their presets away from the user's.Bugs the tests found, fixed in separate commits:
format::ms()printed 100 ms to 1 s with every decimal, becausejuce::String(v, 0)means "as many decimals as needed". So 250.4 ms read "250.4 ms" and 333.333 ms read "333.333 ms". This shows in the Limiter's, EQ's and De-Esser's time controls. (1bb3cc9)std::locale::global,freq,gain,qandmswrote "1,23 kHz".juce::String(double, n)imbues its stream buffer with the classic locale, but thestd::ostreamit writes through still uses the global one. The formatters now build the digits themselves. I found this on macOS CI and then reproduced it on Linux. (52b0bf8)parseNumberread text with junk before the number as 0. Typingx5orgain 3into any control bound withbindValueset the parameter to 0, as an undoable edit.parseFreqread1.2.3as 1.2 Hz. (1bb3cc9)PresetManager::removecleared the favourite and the current preset before trying to move the file to the trash. When that failed, the file stayed but lost its star and the header said "Default". (b70ce8e)PresetManager::renametreated the preset's own file as a collision. On macOS and Windows that means "bass" could never become "Bass", and renaming to the same name failed on every platform. It now asks the file system whether the target is the same file (File::getFileIdentifier), notFile::operator==. That operator ignores case even on case-sensitive macOS volumes, where a path comparison would overwrite a distinct "Bass". (b70ce8e, bdeb22a, 7fbcd0f: a case-only rename goes through a temporary name, because on LinuxmoveFileTodeletes the target first, and on a casefold folder the target is the same file.)2. Scheduled CI
Upstream Watch (new,
.github/workflows/upstream_watch.yml, Mondays 06:17 UTC plusworkflow_dispatch): for everyproducts/*/UPSTREAM,scripts/upstream_behind.shlists the upstreammaincommits since the pin. The workflow keeps one issue labelledupstream-watchwith a table and per-product commit lists. It creates the issue when something is behind, rewrites the body each week, comments only when the counts change, and closes it once everything is current. Permissions arecontents: readandissues: write. It doesn't touch code or open PRs. Commit subjects go in code spans so upstream's@mentionsand#numbersdon't ping or link anything here.Current lag (2026-11-02):
2f7b45c3ce598e412c844Sanitizer (Saturdays) and Heavy Test (Sundays), both at 02:23 UTC, keep
workflow_dispatchand itsproductinput.product, so they would have configured with-DALTVANE_PRODUCTS=(nothing built) and failed on a missing.env.productsjob now picks the input's product, or every product on the schedule.build_and_testruns product × runner. On the schedule it skips runners a product'sPLATFORMSfile rules out (Dialogue on Linux), the same way the top-level CMakeLists does. A dispatched single product still builds on every runner, as before. Excluding every combination for a runner would turn itsincludeentry into a job with no product. The existingincludeentries are untouched.${{ github.ref }}alone, the scheduled runs onmain, pushes tomainand the other scheduled workflow would cancel one another.Testing
-DALTVANE_BUILD_TESTS=ON, De-Esser plus the UI tests):xvfb-run ctest -j6passes 128 of 128. The UI tests are 78 cases and 1151 assertions. The locale test sets its own commanumpunctlocale, so it runs everywhere, and it fails on Linux without 52b0bf8.actionlint1.7.7 passes on all workflows except one existing error:sanitizer.yml's unreachable Windows step usesmatrix.vcvars64_path, which isn't defined. I left it alone, since another change is editing those files.codex exec review --base origin/main, four runs:altvane_add_plugingives products, includingJUCE_USE_WEBP=0(7a3ce25). The macOS run found the locale bug above.gh workflow run --refreturns 404 until the file is onmain. So I ran its report step locally in a shallow clone, asactions/checkoutmakes one; it produced the table above. I ran its issue step against a stand-inghin five situations: behind with no issue, unchanged counts (edit only), changed counts (edit and comment), current with an issue open (close with a comment), and current with no issue (nothing).productsjob's script locally for both workflows, scheduled and withdialogueandeqinputs.Review
JucePlugin_Name,PRODUCT_NAME_WITHOUT_VERSIONandVERSIONthemselves. Ifaltvane_uistarts reading another plugin macro, this target needs it too.~/.cacherather than/tmp. gio only trashes files on the home folder's file system, and/tmpis often tmpfs. They also pointXDG_DATA_HOMEat a trash of their own, so nothing lands in the user's trash. On macOS and Windows, the remove test uses the real Trash or Recycle Bin, with a file namedDoomed.json.serialiseDouble, which saves state and XML attributes, goes through the same stream. Under a comma locale, saved state and presets would hold "0,5"-style values. That belongs in the saved-state work or upstream in JUCE.parseNumberstill ignores what follows the number, because products rely on that (parseMsreads "20 ms"). It still treats a comma as a decimal point, so "1,000" reads as 1. And-infdoesn't parse. The tests pin all three behaviours.sanitizer.ymlandcmake_heavy_test.ymlare now listed twice: in theincludeentries and in theproductsjob'sRUNNERS. A new entry needs both.main. GitHub disables them after 60 days without repository activity.🤖 Generated with Claude Code