tests: core-model / value-processor / midi unit tests (+3 fixes)#2126
Open
jcelerier wants to merge 6 commits into
Open
tests: core-model / value-processor / midi unit tests (+3 fixes)#2126jcelerier wants to merge 6 commits into
jcelerier wants to merge 6 commits into
Conversation
9 CI-runnable, value-asserting Catch2 unit tests over pre-existing core code (independent of the graphics work; feeds the Coveralls coverage job): - test_unit_state_serialization (899 assert): Address/Message/Domain DataStream + JSON round-trips over the full ossia value matrix + polymorphic port loads. - test_unit_process_model (155): process/port/cable model + round-trips. - test_unit_curve_sample / _roundtrip: segment valueAt vs closed-form + serialize. - test_unit_automation_values / _mapping_values / _spline_values: sampled values. - test_unit_midi_message (202): libremidi encoding, sequencer UMP, SMF, fuzz. - test_unit_dataflow_value (59): cable + port round-trips, value propagation. Fixes for bugs the tests surfaced (all ASAN-validated): - Curve PowerSegment/EasingSegment::valueAt normalise x to the segment-local [0,1] ratio before pow()/easing, so model sampling agrees with the ossia execution engine for a segment that doesn't start at 0 (was correct only for a full-range segment; diverged in Automation/Tempo state-value queries of a multi-segment curve). - Define Process::CableData::operator== (declared SCORE_LIB_PROCESS_EXPORT in CableData.hpp but never defined -> link error for any caller). - Export the curve PowerSegmentData/EasingData QDataStream operators (SCORE_SERIALIZE_DATASTREAM_DECLARE was missing the export macro). Investigated + intentionally NOT changed: ControlInlet::loadData's DontReloadValue flag is misnamed vs its effect but is by design (not data loss — default callers re-derive the value from their own source; document save/load bypasses loadData); the right cleanup is a rename, not a polarity flip. Tests document this. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
The two stereo GistState overloads sized g1 to channel 1's frame count but called g1.processAudioFrame(data(c0), ...) — feeding channel 0's samples into the right-channel analyzer. So stereo Pitch and onset/pulse reported the LEFT channel's value for the right channel. data(c0) -> data(c1) at both sites (GistState.hpp g1 no-gain + gated overloads). Found by the P3R3 analysis unit test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
The exprtk "x" and "px" vector_views are bound once to the heap buffers of
State::cur_in / prev_in (math_expression::add_vector captures the data pointer).
The per-sample std::swap(cur_in, prev_in) exchanged those buffers underneath the
views, so on every odd sample index the expression saw x = in[i-1] and px = in[i]
(x/px effectively swapped) — e.g. a gain on the ramp {1,2,3,4} produced
{0.5,0.5,1.5,1.5} instead of {0.5,1,1.5,2}, and any expression using x ran at an
effective half sample rate. Replace the swap with prev_in = cur_in (same pattern
as the sibling MathMapping_generic; sizes are equal so no allocation), keeping
the intended px = previous-sample semantics on every sample.
Found by the P3R3 fx unit test; mechanism confirmed by a paper trace.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
The (deprecated) Envelope node computed rms = sqrt(sum_of_squares); rms /= N, i.e. sqrt(Sum x^2)/N — a factor of sqrt(N) too small. RMS is sqrt(mean of squares) = sqrt(sum/N). Divide by N before the sqrt. Found by the P3R3 fx unit test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
6 CI-runnable value-asserting Catch2 tests, all ASAN-clean: - test_unit_avnd_audio_effects: ao::Gain/AudioSum/MonoMix/StereoMixer pan law/Silence (exact). - test_unit_fx_audio_dsp: MathAudioFilter (exprtk, per-sample exact), Envelope (peak+RMS), LFO v2 (sine/square/saw/triangle closed-form) — asserts the corrected DSP. - test_unit_audio_gain_pan: ossia::audio_outlet::post_process gain*pan stage + ossia::mix summing. - test_unit_avnd_audio_roundtrip: ao::Gain + MathAudioFilter DataStream + JSON save/load round-trip. - test_unit_analysis_gist: Gist RMS/FFT-bin/centroid/Yin-pitch/envelope vs reference math (incl. the corrected stereo right-channel analysis) + fuzz. - test_unit_faust_dsp: libfaust JIT identity/gain/one-pole/os.osc/fi.lowpass transfer function + malformed-program graceful failure. (score_plugin_faust isn't built under sanitizers, so the test links system libfaust directly with the plugin's exact options.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
The flag was inverted relative to its name: passing DontReloadValue is what *reloads* the control value in ControlInlet::loadData, while the default drops it. Analysis (P3R2) established this is by-design, not a bug — so the correct cleanup is a rename, not a polarity flip. Pure rename + a clarifying comment on the enum; no behavior change. The single caller that wants the value applied (LoadPresetCommand::redo) now reads PortLoadDataFlags::ReloadValue. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE
jcelerier
force-pushed
the
tests/core-unit
branch
from
July 18, 2026 13:37
5e2c099 to
d55694c
Compare
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
1 participant
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.
9 CI-runnable, value-asserting Catch2 unit tests over pre-existing core code — independent of the gfx-interop decomposition stack, so this sits directly on
tests/infrastructure(feeds the Coverage job once merged) rather than restacking the gfx PRs.Tests (all green + ASAN-clean; ~1400 assertions)
test_unit_state_serialization(899)test_unit_process_model(155)test_unit_curve_sample/_curve_roundtripvalueAtvs closed-form + serialize round-triptest_unit_automation_values/_mapping_values/_spline_valuestest_unit_midi_message(202)test_unit_dataflow_value(59)Fixes for bugs these tests surfaced (all ASAN-validated)
PowerSegment/EasingSegment::valueAtnow normalise x to the segment-local [0,1] ratio beforepow()/easing, so model sampling agrees with the ossia execution engine for a segment that doesn't start at 0. It was correct only for a full-range segment; for a multi-segment curve the model'svalueAt(used by Automation/Tempo state-value queries) diverged from playback. Playback, and the visual curve, were always correct — this only affected the numeric state snapshot.Process::CableData::operator==— declaredSCORE_LIB_PROCESS_EXPORTin the header but never defined (link error for any caller).PowerSegmentData/EasingDataQDataStream operators (theSCORE_SERIALIZE_DATASTREAM_DECLAREwas missing its export macro).Investigated, intentionally NOT changed
ControlInlet::loadData'sDontReloadValueflag is misnamed vs its effect (default drops the value; the flag restores it) but this is by design, not data loss: the only caller that wants the value passes the flag, every default-flag caller re-derives the value from its own source (script defaults / lilv state /loadPreset), and document save/load bypassesloadDataentirely (value preserved via the visitor path). The correct cleanup is a rename, not a polarity flip — the tests document this.🤖 Generated with Claude Code