Skip to content

Commit 4e32fcb

Browse files
Yaraslautclaude
andcommitted
bank: record the audit's backlog, and document the two shell shapes
The first run reported nine findings; four are a real backlog and five are the audit's documented blind spot met head on. The five: AppShell.qml keeps its page controllers in a `var` array and refreshes the visible page with `controllers[current].refresh()`. Every one of those five `refresh` invokables is reachable and exercised on every page switch, and none is visible to a text scan. They are exempted with that as their reason -- not with an issue number, because there is nothing to fix. Rewriting AppShell into a five-armed switch so a scanner could see the call would be bending the rung around its guard. The four that are real -- TransactionController's `selectedAccount` and its NOTIFY, its `posted`, and PayeeController's `paid` -- are itemised and tracked in morph#296. `selectedAccount` looks like an actual bug: MoveMoneyPage calls the property's WRITE and never reads it back, so the picker cannot show a selection `refresh()` made on its own. Neither direction of the audit reports a broken screen: no .qml file binds a name its controller lacks. Also record in TESTING.md and the bank README what bank taught the guard: the context-property shell shape, the inherited-surface split, and the fact that CI builds no part of native bank today, so this binary runs locally only. Closes #240 for bank -- the last of its three rungs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 1136194 commit 4e32fcb

3 files changed

Lines changed: 150 additions & 10 deletions

File tree

examples/TESTING.md

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -134,18 +134,39 @@ CHECK(findings.isEmpty());
134134
property-bag keys inside an emitted `QVariantMap` (no metaobject exists for
135135
them — the per-rung "bag shape" cases remain the only guard); QML the rung
136136
does not own, such as the shipped `MorphForms` renderer's; dynamic member
137-
access; and whether the shell wires an alias to the class the test bound.
137+
access (`bank`'s `AppShell.qml` calls `controllers[current].refresh()`
138+
through a `var` array, so five reachable invokables sweep as unreferenced);
139+
inherited members, in the sweep direction only; and whether the shell wires
140+
an alias to the class the test bound.
138141
139142
`allowUnbound(alias, member, reason)` exempts one member, with a required
140143
reason. The exemption list is itself audited — a member that has since been
141144
deleted, an alias nobody bound, or a member QML does bind is a finding — so it
142145
can only shrink deliberately.
143146
144-
Adopted by `bookmarks`, `pastebin`, `polls` and `ledger`; `lims`, `kanban`
145-
and `bank` are tracked in morph#240. The audit's own
146-
mutation suite is `examples/common/testkit/test_qml_surface.cpp`: every case
147-
there drives it against a deliberately broken pair and asserts the specific
148-
finding.
147+
Adopted by every rung that has QML — `bookmarks`, `pastebin`, `polls`,
148+
`ledger`, `lims` and `kanban` — and by `examples/bank`, which is not a rung
149+
(morph#240). The audit's own mutation suite is
150+
`examples/common/testkit/test_qml_surface.cpp`: every case there drives it
151+
against a deliberately broken pair and asserts the specific finding.
152+
153+
`bank` is where the two shell shapes diverge, and its adoption is worth
154+
reading before adding a seventh. It publishes its controllers with
155+
`QQmlContext::setContextProperty` rather than `setInitialProperties`, so its
156+
aliases are root-context names: one `bind()` per controller covers all
157+
thirteen files, no `bindIn()` is needed, and its `Connections` blocks name
158+
their target as a bare identifier (`target: app`). A bare target counts only
159+
when it is an alias the audit was handed — an identifier that is not is far
160+
more often a local `id`, and nothing distinguishes the two — so the
161+
unbound-bridge check does not reach this shape. `bank` also has a shared
162+
controller base: a reference written in QML resolves against everything the
163+
bridge can reach, inherited members included, while the unreferenced-member
164+
sweep covers only what the bound class declares itself.
165+
166+
`bank` is not built by CI at all today (only its WebAssembly GUI is, in
167+
`wasm-demo.yml`), so `bank_gui_tests` — like `bank_tests` beside it — runs
168+
only locally, in a `-DMORPH_BUILD_BANK_EXAMPLE=ON -DMORPH_BUILD_BANK_GUI=ON`
169+
configure.
149170
150171
## The dual-mode fixture
151172

examples/bank/README.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,10 @@ cmake --build build --target bank_gui
171171

172172
Structure:
173173

174+
- **`gui/bank_gui_lib`** — a static library holding `BankClient` and every
175+
controller, linking `Qt6::Core` only (no Quick, no Qml). `bank_gui` is
176+
`main.cpp` plus the QML module on top of it; `bank_gui_tests` (see
177+
[Tests](#tests)) is the other consumer.
174178
- **`gui/BankClient`** — owns the worker pool, a `morph::qt::QtExecutor`, the
175179
`Bridge` (local backend), DB setup, and the session. UI-toolkit-agnostic.
176180
- **`gui/controllers/`** — one QObject controller per domain (`AppController`,
@@ -240,6 +244,21 @@ shared on-disk test database. Notable cross-cutting tests:
240244
- `test_offline.cpp` — parks deposits in an `InMemoryOfflineQueue` while "offline" and
241245
replays them via `SyncWorker` on "reconnect".
242246

247+
`tests/gui/` is a second binary, `bank_gui_tests`, built only when
248+
`-DMORPH_BUILD_BANK_GUI=ON` is also set — `bank_tests` links no Qt at all and
249+
is built in configures that have none. It holds `test_bank_qml_surface.cpp`,
250+
which points the ladder testkit's `QmlSurfaceAudit`
251+
(`examples/common/testkit/qml_surface.hpp`, `examples/TESTING.md`) at the six
252+
controllers and the thirteen `.qml` files, and fails on any name that exists on
253+
one side only: a renamed `Q_INVOKABLE`, a `Connections` handler for a signal
254+
that is gone, a property read that would resolve to `undefined`. None of those
255+
is a compile error or a QML warning; the pane just stays empty.
256+
257+
```sh
258+
cmake --build build --target bank_gui_tests
259+
./build/examples/bank/bank_gui_tests
260+
```
261+
243262
## Status
244263

245264
Models, tests, CLI, the Qt 6 GUI, and a self-contained WebAssembly build (hosted

examples/bank/tests/gui/test_bank_qml_surface.cpp

Lines changed: 104 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,43 @@
33
// The QML-visible surface of all six bank GUI controllers, audited against
44
// `gui/qml/` itself.
55
//
6-
// TEMPORARY BASELINE FORM: no exemptions, so the first run prints the real
7-
// backlog. Replaced by the recorded form once the findings are known.
6+
// The bank GUI had no test of any kind before this file: `bank_tests` links
7+
// `bank_lib` and drives the models, and nothing linked `gui/controllers/*` at
8+
// all. So the thirteen `.qml` files under `gui/qml/` and the six `QObject`s
9+
// they bind by string have never been checked against each other. QML binds by
10+
// string, which means a renamed `Q_INVOKABLE`, a `Connections` handler for a
11+
// signal that no longer exists, or a property read resolving to `undefined` is
12+
// not a compile error, not a test failure, and not a QML warning — the pane
13+
// simply stays empty.
14+
//
15+
// This file is that check, and it is not hand-written: it points
16+
// `morph::ladder::testkit::QmlSurfaceAudit`
17+
// (`examples/common/testkit/qml_surface.hpp`) at the GUI's own QML and lets
18+
// those files be the expectation, in both directions. See that header for what
19+
// the audit does and does not cover.
20+
//
21+
// The alias mapping below is the one per-rung fact the audit cannot derive,
22+
// and bank's is the simplest possible shape — for a reason worth stating,
23+
// because it is not the shape the ladder rungs have. `gui/main.cpp` publishes
24+
// each controller with `QQmlContext::setContextProperty`, not
25+
// `setInitialProperties`: they are root-context names, visible under the same
26+
// name in every one of the thirteen files, and no sub-view re-exposes one
27+
// under a property of its own. So one `bind()` per controller covers every
28+
// file, and no `bindIn()` is needed — unlike ledger, whose sub-views each call
29+
// their own bridge plain `bridge`.
30+
//
31+
// bank is not a ladder rung (it is absent from `examples/rungs.txt` and never
32+
// calls `morph_add_rung()`), so this binary is wired by hand in
33+
// `examples/bank/CMakeLists.txt` rather than by the rung glob. It exists only
34+
// in a `-DMORPH_BUILD_BANK_GUI=ON -DMORPH_BUILD_TESTS=ON` configure.
835

936
#include <QString>
1037
#include <QStringList>
1138
#include <catch2/catch_test_macros.hpp>
1239
#include <filesystem>
40+
#include <initializer_list>
1341
#include <string>
42+
#include <utility>
1443

1544
#include "BankClient.hpp"
1645
#include "controllers/AccountController.hpp"
@@ -27,7 +56,14 @@ using morph::ladder::testkit::QmlSurfaceAudit;
2756

2857
} // namespace
2958

30-
TEST_CASE("bank baseline", "[bank][gui][qml-surface]") {
59+
TEST_CASE("Every bank controller exposes exactly the surface gui/qml binds, and nothing more",
60+
"[bank][gui][qml-surface]") {
61+
// A real BankClient, because every controller holds `BridgeHandler`s
62+
// constructed from one. Nothing here dispatches an action — the audit reads
63+
// metaobjects and text — but `BankClient`'s constructor runs the schema
64+
// migrations, so it needs a database like any other bank test does. Its own
65+
// file, not the one `bank_tests` shares, so the two binaries can run
66+
// concurrently.
3167
const auto dbPath = std::filesystem::temp_directory_path() / "morph_bank_qml_surface.db";
3268
bankgui::BankClient client{"DRIVER=SQLite3;Database=" + dbPath.string()};
3369

@@ -46,8 +82,72 @@ TEST_CASE("bank baseline", "[bank][gui][qml-surface]") {
4682
audit.bind(QStringLiteral("payees"), payeeController);
4783
audit.bind(QStringLiteral("loans"), loanController);
4884

85+
// ── `refresh`, reached only through a `var` array ─────────────────────
86+
// Not a backlog and not a defect: this is the audit's documented blind
87+
// spot, met head on. `AppShell.qml` holds the five page controllers in
88+
//
89+
// readonly property var controllers: [accounts, txns, cards, payees, loans]
90+
//
91+
// and refreshes the visible page with
92+
// `shell.controllers[shell.current].refresh()` (AppShell.qml:12-14). That
93+
// is dynamic member access — the scanner sees an index expression, never
94+
// the name `refresh` against an alias — so every one of these five is
95+
// reachable, exercised on every page switch, and invisible to a text scan.
96+
// `app` is not in that array and has no `refresh`, which is why only five
97+
// controllers appear here.
98+
//
99+
// Recorded rather than worked around: rewriting AppShell to a five-armed
100+
// switch purely so a scanner can see the call would be bending the rung
101+
// around its guard.
102+
const QString dynamic = QStringLiteral(
103+
"called dynamically via AppShell.qml's `controllers[current].refresh()`; "
104+
"dynamic member access is outside what a text scan can see");
105+
for (const char* alias : {"accounts", "txns", "cards", "payees", "loans"}) {
106+
audit.allowUnbound(QString::fromLatin1(alias), QStringLiteral("refresh"), dynamic);
107+
}
108+
109+
// ── The pre-existing backlog, recorded rather than swallowed ──────────
110+
// The first run of this audit reported four members these six controllers
111+
// publish that no file under gui/qml/ binds — over and above the five
112+
// dynamic `refresh` calls above. The other direction was clean: no QML file
113+
// binds a name its controller lacks, so no screen is broken. Each is either
114+
// dead surface or a missing control, and deciding which is per-member work
115+
// this file does not do. They are listed here so the guard goes live now
116+
// and catches the *next* drift in either direction, with the backlog
117+
// itemised instead of hidden behind a lowered bar.
118+
//
119+
// The list is checked in both directions too: an exemption for a member
120+
// that has since been deleted, or one QML has since bound, fails this test
121+
// (testkit/qml_surface.hpp). It can only shrink deliberately.
122+
//
123+
// Same shape as ledger's (morph#239), lims' (morph#287) and kanban's
124+
// (morph#291).
125+
const QString backlog = QStringLiteral("unbound controller surface, tracked in morph#296");
126+
for (const auto& [alias, member] : std::initializer_list<std::pair<const char*, const char*>>{
127+
// `txns.selectAccount(id)` is called, but the property it writes
128+
// is never read back, so the account picker cannot reflect a
129+
// selection the controller made itself (TransactionController.cpp
130+
// auto-selects the first account on refresh).
131+
{"txns", "selectedAccount"},
132+
{"txns", "selectedChanged"},
133+
// Emitted on every deposit/withdraw/transfer, and on every bill
134+
// payment — no QML handles either.
135+
{"txns", "posted"},
136+
{"payees", "paid"},
137+
}) {
138+
audit.allowUnbound(QString::fromLatin1(alias), QString::fromLatin1(member), backlog);
139+
}
140+
49141
const QStringList findings = audit.run();
50142
INFO(findings.join(QStringLiteral("\n")).toStdString());
51143
CHECK(findings.isEmpty());
52-
CHECK(audit.scannedFiles() == QStringList{});
144+
145+
// The audit is only as good as the files it found: the thirteen gui/qml
146+
// ships, which is also the list gui/CMakeLists.txt hands qt_add_qml_module.
147+
CHECK(audit.scannedFiles() ==
148+
QStringList{QStringLiteral("AccountsPage.qml"), QStringLiteral("AppButton.qml"),
149+
QStringLiteral("AppShell.qml"), QStringLiteral("CardsPage.qml"), QStringLiteral("Field.qml"),
150+
QStringLiteral("LoansPage.qml"), QStringLiteral("Login.qml"), QStringLiteral("Main.qml"),
151+
QStringLiteral("MoveMoneyPage.qml"), QStringLiteral("Panel.qml"),
152+
QStringLiteral("PayeesPage.qml"), QStringLiteral("Picker.qml"), QStringLiteral("Pill.qml")});
53153
}

0 commit comments

Comments
 (0)