test(theme): cover v2 scope tree, primitives, compound fonts (#3179). Principle VIII.#3180
Merged
Conversation
… Principle VIII. Adds smoke coverage for the v2 surface area introduced in #3176 that the existing flat-token tests didn't reach. All additions land in tests/theme_manager_test.cpp — the CMake target already links every TU the new assertions touch (ThemeManager.cpp, AppSettings.cpp, LogManager.cpp, AsyncLogWriter.cpp), so no CMakeLists change is needed. New blocks: JSON loader/writer - v2 primitives + {alias} resolution end-to-end through color() / cssFragment() / resolve() — guards against the cssFragment-empty-for- ThemeFont class of bug that surfaced mid-PR. - flattenTokens discriminator routing: a single v1 file with a gradient ({"type":...}), a compound font ({"family":...}), and a plain nested group is imported and each leaf is type-checked. Pins the discriminator order in flattenTokens(). - v1 → v2 migration round-trip: import v1 file, mutate via setColor (triggers auto-save), re-read on-disk JSON to assert schemaVersion=2 + scopes.root.tokens wrapper, then reload and confirm values survive. Scope tree (pure in-memory on the built-in Default Dark; saveActiveTheme silently fails for built-ins so each block leaves no disk side-effects) - setColor at a nested scope does not touch root. - Inheritance walk: token defined at scopeB read from scopeB/leaf resolves to scopeB's value. - removeOverride at a nested scope drops the override; reads fall back to parent. - removeOverride at root is the defensive no-op (would otherwise delete the token tree-wide). - scopeOrCreate("a/b/c") wires up the full chain, verified via containerPaths() + isOverriddenAt() on each intermediate. - Each scope-tree block uses a unique container path (scopeA / scopeB / scopeC / deep) so state from earlier blocks can't bleed into later assertions. Compound font tokens - setFontToken → fontToken round-trip preserves all three fields. - value() on a ThemeFont downgrades to .family (the ~35 legacy callers). - cssFragment("font.size.freq") + sizing("font.size.freq") virtual lookups return the compound's embedded size — the bug fixed mid-PR. - Persistence: the v1→v2 migration block doubles as the compound-on-disk check (asserts {family, size, color} JSON shape, not a nested scope). Editor-surface smoke (deferred-commit + per-theme recent-colors) is deferred to a follow-up that can stand up the GUI dialog layer in its own test target. Blast radius: skipped — additive test-only change, no production symbols touched.
14 tasks
Contributor
Author
There was a problem hiding this comment.
Thanks for the test coverage — this fills in some genuinely useful gaps around the v2 schema rewrite.
A few things I like about how this is structured:
- The discriminator test pins flattenTokens' field-order assumption (
type→ gradient,family→ font, else recurse) which is exactly the kind of brittle implicit contract that's worth nailing down before someone refactors it. Good comment explaining why the order matters. - The v1 → v2 migration round-trip is the most valuable addition here. Importing a v1 file, mutating via
setColorto triggersaveActiveTheme, then asserting on the on-disk JSON shape (schemaVersion=2,scopes.root.tokens, compound-font{family,size,color}shape) is exactly what catches future loader changes that would silently break theme persistence. Verified againstThemeManager.cpp:1001-1056— the assertions match the writer's actual output shape. - The scope tree blocks correctly use unique container paths (
scopeA,scopeB,scopeC,deep/middle/leaf) so mutations don't bleed between blocks. The "built-ins don't persist" assumption is verified byThemeManager.cpp:741-747. - The compound-font cssFragment virtual lookup test (
font.size.freqfalling through to the embedded size onfont.family.freq) directly guards the mid-PR bug referenced in the comment — that's the right kind of regression test.
Minor observations, none blocking:
- The root-scope
removeOverride("", ...)test asserts the no-op behavior but doesn't capture the warning emission. Probably fine — Qt's logging isn't easily testable here and the visible behavior (value unchanged) is what users actually care about. - The migration test relies on
QStandardPaths::GenericConfigLocationbeing routed viaXDG_CONFIG_HOME(set at the top ofmain). That's already established by the existing tests, so this just inherits the isolation guarantee.
The new tests are all read-side assertions and don't touch production code paths, so the blast radius is limited to the test binary. Looks good as-is.
🤖 Generated by AetherClaude
🤖 aethersdr-agent · cost: $6.6040 · model: claude-opus-4-7
G6PWY-Chris
pushed a commit
to G6PWY-Chris/AetherSDR
that referenced
this pull request
Jun 22, 2026
…dr#3179). Principle VIII. (aethersdr#3180) ## Summary Fixes aethersdr#3179 ### What was changed test(theme): cover v2 scope tree, primitives, compound fonts (aethersdr#3179). Principle VIII. ### Files modified - `tests/theme_manager_test.cpp` ``` tests/theme_manager_test.cpp | 330 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 330 insertions(+) ``` --- Generated by AetherClaude (automated agent for AetherSDR) --- <sub>🤖 aethersdr-agent · cost: $20.7094 · model: claude-opus-4-7</sub> Co-authored-by: aethersdr-agent[bot] <273844287+aethersdr-agent[bot]@users.noreply.github.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
Fixes #3179
What was changed
test(theme): cover v2 scope tree, primitives, compound fonts (#3179). Principle VIII.
Files modified
tests/theme_manager_test.cppGenerated by AetherClaude (automated agent for AetherSDR)
🤖 aethersdr-agent · cost: $20.7094 · model: claude-opus-4-7