Skip to content

test(theme): cover v2 scope tree, primitives, compound fonts (#3179). Principle VIII.#3180

Merged
ten9876 merged 1 commit into
mainfrom
aetherclaude/issue-3179
May 26, 2026
Merged

test(theme): cover v2 scope tree, primitives, compound fonts (#3179). Principle VIII.#3180
ten9876 merged 1 commit into
mainfrom
aetherclaude/issue-3179

Conversation

@aethersdr-agent

Copy link
Copy Markdown
Contributor

Summary

Fixes #3179

What was changed

test(theme): cover v2 scope tree, primitives, compound fonts (#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)


🤖 aethersdr-agent · cost: $20.7094 · model: claude-opus-4-7

… 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.
@aethersdr-agent aethersdr-agent Bot requested a review from a team as a code owner May 26, 2026 08:36
@aethersdr-agent aethersdr-agent Bot enabled auto-merge (squash) May 26, 2026 08:36

@aethersdr-agent aethersdr-agent Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 setColor to trigger saveActiveTheme, 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 against ThemeManager.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 by ThemeManager.cpp:741-747.
  • The compound-font cssFragment virtual lookup test (font.size.freq falling through to the embedded size on font.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::GenericConfigLocation being routed via XDG_CONFIG_HOME (set at the top of main). 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

@ten9876 ten9876 merged commit 67f765e into main May 26, 2026
5 checks passed
@ten9876 ten9876 deleted the aetherclaude/issue-3179 branch May 26, 2026 15:07
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(theme): add coverage for v2 scope tree + primitives + compound fonts

1 participant