diff --git a/gui/CMakeLists.txt b/gui/CMakeLists.txt index 3200ed3..139c938 100644 --- a/gui/CMakeLists.txt +++ b/gui/CMakeLists.txt @@ -1,24 +1,24 @@ # velox-gui — the Qt 6 Widgets client. Lane GUI. # # CLAUDE.md §3: zero download logic here. This target links libveloxproto (the wire -# types, ADR 0009) and NEVER libveloxcore — a grep for curl|pwrite|sqlite under gui/ -# must come back empty. +# types, ADR 0009) and NEVER libveloxcore — enforced by tests/ (see the +# gui_no_download_logic test). # # The root CMakeLists.txt add_subdirectory()s this unconditionally once the file exists, -# so it must stay configurable even before CORE has landed veloxproto. Until that target -# exists we announce and bail; the moment `core/` merges its libveloxproto, the GUI -# lights up with no edit here (same contract as the root file's EXISTS() guards). +# so it must stay configurable even if veloxproto is ever absent again. Until that target +# exists we announce and bail. if(NOT TARGET veloxproto) - message(STATUS "velox-gui: libveloxproto has not landed yet — GUI target skipped. " - "It builds automatically once core/ merges the veloxproto target (ADR 0009).") + message(STATUS "velox-gui: libveloxproto target missing — GUI target skipped. " + "It builds automatically once core/ provides the veloxproto target (ADR 0009).") return() endif() set(CMAKE_AUTOMOC ON) -add_executable(velox-gui - src/main.cpp +# Everything except main() lives in a static lib so the tests can link the real widgets +# and models rather than a reimplementation. +add_library(velox-gui-lib STATIC src/rpc/RpcConnection.cpp src/rpc/RpcClient.cpp src/models/DownloadTableModel.cpp @@ -26,29 +26,25 @@ add_executable(velox-gui src/mainwindow/MainWindow.cpp ) -target_include_directories(velox-gui PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/src) - -target_compile_features(velox-gui PRIVATE cxx_std_23) - -# Warnings at target scope, not via CMAKE_CXX_FLAGS — the dev/tsan presets overwrite that -# cache variable wholesale (same rationale as core/CMakeLists.txt). -target_compile_options(velox-gui PRIVATE -Wall -Wextra -Wpedantic -Werror) - -target_link_libraries(velox-gui PRIVATE +target_include_directories(velox-gui-lib PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/src) +target_compile_features(velox-gui-lib PUBLIC cxx_std_23) +target_compile_options(velox-gui-lib PRIVATE -Wall -Wextra -Wpedantic -Werror) +target_link_libraries(velox-gui-lib PUBLIC velox::proto Qt6::Widgets Qt6::Svg Qt6::Network ) -set_target_properties(velox-gui PROPERTIES - WIN32_EXECUTABLE OFF - MACOSX_BUNDLE OFF -) +add_executable(velox-gui src/main.cpp) +target_compile_options(velox-gui PRIVATE -Wall -Wextra -Wpedantic -Werror) +target_link_libraries(velox-gui PRIVATE velox-gui-lib) +set_target_properties(velox-gui PROPERTIES WIN32_EXECUTABLE OFF MACOSX_BUNDLE OFF) # --- i18n ------------------------------------------------------------------------------- # Every string in the GUI goes through tr(); the Arabic stub exists to prove the RTL -# layout survives (GUI DoD). lrelease/lupdate come from Qt6::LinguistTools, found by root. +# layout survives (GUI DoD), exercised by tests/tst_rtl. lrelease also drops the .qm at +# ${CMAKE_CURRENT_BINARY_DIR}/velox_ar.qm, which the RTL test loads by path. qt_add_translations(velox-gui TS_FILES i18n/velox_ar.ts) # --- tests --------------------------------------------------------------------------- diff --git a/gui/docs/pkg-qa-requests-m1.md b/gui/docs/pkg-qa-requests-m1.md new file mode 100644 index 0000000..dc93dfe --- /dev/null +++ b/gui/docs/pkg-qa-requests-m1.md @@ -0,0 +1,115 @@ +# GUI → PKG/QA requests (M1) + +Filed by lane GUI. These touch PKG/QA-owned files (`tools/bootstrap.sh`, the CI jobs, +`docs/agents/AGENT-PKG-QA.md`) and the root `README.md`, so GUI is not making the edits — +CLAUDE.md §1. Everything below is apply-ready. + +--- + +## R1 — `libqt6svg6-dev` is not a real package on 26.04 (blocks the GUI DoD) + +`apt-cache policy libqt6svg6-dev` on a clean Ubuntu 26.04 box gives `Candidate: (none)`. +The Qt 6 SVG **dev** package in the 26.04 archive is **`qt6-svg-dev`** (currently +`6.10.2-2`). `libqt6svg6` (no `-dev`) exists as the runtime lib but carries no headers or +CMake config, so `find_package(Qt6 COMPONENTS Svg)` fails without `qt6-svg-dev`. + +I checked the other 38 apt names in `bootstrap.sh` against `apt-cache policy` on 26.04 — +`libqt6svg6-dev` is the only one with no candidate. Everything else resolves. + +**Why it passed here anyway:** this dev box already has `qt6-svg-dev` installed (it came in +with a `qt6-base-dev` recommends chain at some point), so `pkg-config --exists Qt6Svg` +succeeds and `--check` is green. On a clean VM the `apt-get install` step aborts before +`--check` ever runs. + +### Fix — three files, same one-line change + +**`tools/bootstrap.sh`** (in `APT_GUI`, ~line 87): +```diff + APT_GUI=( + qt6-base-dev + qt6-tools-dev + qt6-tools-dev-tools # lrelease/lupdate for i18n +- libqt6svg6-dev # GUI: SVG icon rendering ++ qt6-svg-dev # GUI: SVG icon rendering (was libqt6svg6-dev — no such ++ # package on 26.04; this is the one with headers + ++ # the Qt6SvgConfig.cmake find_package needs) + ) +``` + +**`docs/agents/AGENT-PKG-QA.md`** (~line 17): +```diff +- `libqt6svg6-dev`, `libsecret-1-dev`, `nodejs`/`npm` and (optionally) `clang` are ++ `qt6-svg-dev`, `libsecret-1-dev`, `nodejs`/`npm` and (optionally) `clang` are +``` + +**`README.md`** (toolchain table, ~line 106): +```diff + sudo apt update && sudo apt install -y \ +- libqt6svg6-dev \ # GUI: SVG icon rendering ++ qt6-svg-dev \ # GUI: SVG icon rendering + libsecret-1-dev \ # DAEMON: Secret Service for site logins +``` + +--- + +## R2 — `--check` verifies the outcome, not the thing that can break + +`--check` today runs the "verify toolchain" block: `command -v` for the binaries, +`pkg-config --exists` for the dev libs (`Qt6Core`, `Qt6Widgets`, `Qt6Svg`, `libcurl`, +`sqlite3`, `libssl`, `libsecret-1`, `libavformat`), plus the cmake/g++/node version +floors. Every one of those checks a *result* on a box that already installed everything. +None of them touch the apt package **names** in `APT_*`, which is the only part of the +script that can be wrong on a clean machine — as R1 just demonstrated. So `--check` +reported green for a script that fails `apt-get install` on its target OS. + +### Proposed fix — make `--check` validate the install list it would actually run + +Add this to the verify block (runs in **both** modes; in `--check` mode it is the point of +the exercise). It needs neither root nor network — `apt-cache policy` reads the local +package lists that `apt-get update` already populated: + +```bash +# Every apt name this script would install must be a real, installable package on THIS +# release. This is the check that would have caught libqt6svg6-dev before the VM did. +log "validating apt package names (${#PKGS[@]})" +missing_pkgs=() +for pkg in "${PKGS[@]}"; do + # `apt-cache policy ` prints "Candidate: (none)" for a name with no installable + # version, and exits 0 either way — so grep the candidate line, don't trust $?. + cand="$(apt-cache policy "$pkg" 2>/dev/null | sed -n 's/^ Candidate: //p')" + if [[ -z "$cand" || "$cand" == "(none)" ]]; then + printf ' \033[31m✗\033[0m %-20s no installable candidate on this release\n' "$pkg" + missing_pkgs+=("$pkg") + fail=1 + fi +done +if (( ${#missing_pkgs[@]} == 0 )); then + printf ' \033[32m✓\033[0m %-20s all %d resolve\n' "apt names" "${#PKGS[@]}" +fi +``` + +Notes for whoever applies it: + +* `PKGS` is already assembled above the `CHECK_ONLY` branch, and it already respects + `--with-clang` / `--with-packaging`, so the loop covers exactly what would be installed. +* Belt-and-braces alternative: `apt-get install -s --no-install-recommends "${PKGS[@]}"` + (`-s` = simulate, no root) also catches an *un-satisfiable dependency*, not just a + missing name. Downside: it needs the apt lists reasonably fresh and is noisier to parse. + `apt-cache policy` is enough to catch the whole class of bug in R1; pick whichever you + want to own. +* CI: the `bootstrap-script` job already runs `./tools/bootstrap.sh --check` after the + install. With this change that job would have gone red on `libqt6svg6-dev` **at the + `--check` step on the 24.04 runner** even before the install step failed, because the + name has no candidate there either. + +--- + +## R3 (smaller) — a GUI CI job + +There is no job that builds `velox-gui` on its own or runs its offscreen smoke. The +`build` matrix will pick up the target and the `gui_downloadtablemodel` ctest once +`gui/` merges, but the GUI DoD items — 10k rows at 60 fps, flat memory over 10 min, +`--slow`/`--flaky`/`--drop-connection` recovery, `grep -r 'curl\|pwrite\|sqlite' gui/` +empty, RTL layout — need somewhere to run. GUI can write the harness +(`gui/tests/…` + a script); wiring it into `.github/workflows/ci.yml` is PKG/QA. Say the +word and I'll send the harness as a follow-up request with the job stanza pre-written. diff --git a/gui/tests/CMakeLists.txt b/gui/tests/CMakeLists.txt index 52f3649..ccc64f8 100644 --- a/gui/tests/CMakeLists.txt +++ b/gui/tests/CMakeLists.txt @@ -1,22 +1,44 @@ # GUI unit tests. Lane GUI. # -# CLAUDE.md §5: a feature with no test does not exist. The model is the piece with real -# logic that can be tested headless — the progress patch must be a narrow dataChanged and -# must never reset the model (docs/03-gui-spec.md §1). +# CLAUDE.md §5: a feature with no test does not exist. Each test below has a concrete +# failure it catches — that is the bar, not "it runs green". set(CMAKE_AUTOMOC ON) find_package(Qt6 REQUIRED COMPONENTS Test) +# tst_downloadtablemodel — the model's logic, headless. +# Red when: a progress patch widens past the value columns, a reset slips into +# applyProgress, applyProgress starts synthesizing rows for unknown ids, or add/remove +# stops emitting rowsInserted/rowsRemoved. add_executable(tst_downloadtablemodel tst_downloadtablemodel.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../src/models/DownloadTableModel.cpp ) - target_include_directories(tst_downloadtablemodel PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/../src) target_compile_features(tst_downloadtablemodel PRIVATE cxx_std_23) target_compile_options(tst_downloadtablemodel PRIVATE -Wall -Wextra -Wpedantic -Werror) target_link_libraries(tst_downloadtablemodel PRIVATE Qt6::Core Qt6::Test) - add_test(NAME gui_downloadtablemodel COMMAND tst_downloadtablemodel) set_tests_properties(gui_downloadtablemodel PROPERTIES LABELS "gui") + +# tst_rtl — the real RTL check the GUI DoD asks for (the .ts stub alone verifies nothing). +# Red when: the main window stops propagating Qt::RightToLeft to its children, or the +# offline-banner layout is pinned so it does not mirror under RTL. +add_executable(tst_rtl tst_rtl.cpp) +target_compile_features(tst_rtl PRIVATE cxx_std_23) +target_compile_options(tst_rtl PRIVATE -Wall -Wextra -Wpedantic -Werror) +target_link_libraries(tst_rtl PRIVATE velox-gui-lib Qt6::Widgets Qt6::Test) +add_test(NAME gui_rtl COMMAND tst_rtl) +set_tests_properties(gui_rtl PROPERTIES + LABELS "gui" + ENVIRONMENT "QT_QPA_PLATFORM=offscreen") + +# gui_no_download_logic — CLAUDE.md §3 as an executable check, not a hope. +# Red when: a download-logic token (curl, raw pwrite, sqlite, QSqlDatabase) appears +# under gui/src. grep exits 0 only when it finds a match, so a hit fails the test. +add_test(NAME gui_no_download_logic + COMMAND ${CMAKE_COMMAND} + -DGUI_SRC=${CMAKE_CURRENT_SOURCE_DIR}/../src + -P ${CMAKE_CURRENT_SOURCE_DIR}/no_download_logic.cmake) +set_tests_properties(gui_no_download_logic PROPERTIES LABELS "gui") diff --git a/gui/tests/no_download_logic.cmake b/gui/tests/no_download_logic.cmake new file mode 100644 index 0000000..524d29f --- /dev/null +++ b/gui/tests/no_download_logic.cmake @@ -0,0 +1,39 @@ +# Fails if any download-logic token appears under gui/src. Run as a ctest (see +# tests/CMakeLists.txt). CLAUDE.md §3: "A grep for curl|pwrite|sqlite in gui/ must come +# back empty." +# +# Invoked with -DGUI_SRC=. + +if(NOT DEFINED GUI_SRC) + message(FATAL_ERROR "GUI_SRC not set") +endif() + +file(GLOB_RECURSE sources "${GUI_SRC}/*.cpp" "${GUI_SRC}/*.hpp") + +# Word-ish boundaries so this file's own prose and e.g. "curly" do not trip it, and so a +# comment mentioning the rule is fine but a real call is not. +set(forbidden + "curl_[a-z_]+" # libcurl + "\\bpwrite\\b" # raw positioned writes — the transfer path, not the GUI + "sqlite3?_[a-z_]+" # sqlite C API + "QSqlDatabase" # or via Qt + "QNetworkAccessManager" # the GUI talks to the daemon over QLocalSocket, nothing else +) + +set(hits "") +foreach(file ${sources}) + file(STRINGS "${file}" matched REGEX "(curl_[a-z_]+|\\bpwrite\\b|sqlite3?_[a-z_]+|QSqlDatabase|QNetworkAccessManager)") + foreach(line ${matched}) + string(STRIP "${line}" line) + list(APPEND hits "${file}: ${line}") + endforeach() +endforeach() + +list(LENGTH hits n) +if(n GREATER 0) + string(REPLACE ";" "\n " pretty "${hits}") + message(FATAL_ERROR + "download-logic token(s) found under gui/src — CLAUDE.md §3:\n ${pretty}") +endif() + +message(STATUS "gui/src is clean of download-logic tokens (${forbidden})") diff --git a/gui/tests/tst_rtl.cpp b/gui/tests/tst_rtl.cpp new file mode 100644 index 0000000..dbd198b --- /dev/null +++ b/gui/tests/tst_rtl.cpp @@ -0,0 +1,70 @@ +// RTL layout test. Lane GUI. +// +// The GUI DoD asks for "a stub Arabic .ts proves the RTL layout survives". A .ts file on +// its own proves nothing — this is the check with teeth: build the real main window, flip +// the layout direction, and assert (a) the direction propagates to the children and +// (b) a layout we own actually mirrors, by comparing a widget's position LTR vs RTL. + +#include +#include +#include +#include + +#include "mainwindow/MainWindow.hpp" +#include "rpc/RpcClient.hpp" + +using velox::gui::MainWindow; + +namespace { + +int settledLeftOf(QWidget *w) { + QCoreApplication::processEvents(); + QTest::qWait(30); + QCoreApplication::processEvents(); + return w->mapTo(w->window(), QPoint(0, 0)).x(); +} + +} // namespace + +class TstRtl : public QObject { + Q_OBJECT + + private slots: + void directionPropagatesAndLayoutMirrors(); +}; + +void TstRtl::directionPropagatesAndLayoutMirrors() { + velox::gui::rpc::RpcClient client(QStringLiteral("/nonexistent/velox-rtl-test.sock")); + MainWindow window(&client); // never started — the window must build without a daemon + window.resize(900, 500); + + window.setLayoutDirection(Qt::LeftToRight); + window.show(); + + auto *bannerLabel = window.findChild(QStringLiteral("offlineBannerLabel")); + QVERIFY2(bannerLabel != nullptr, "offline banner label not found"); + QVERIFY2(bannerLabel->isVisible(), "offline banner should be visible while disconnected"); + + const int leftLtr = settledLeftOf(bannerLabel); + + window.setLayoutDirection(Qt::RightToLeft); + + // Propagation: the window and its central widget must both flip. + QCOMPARE(window.layoutDirection(), Qt::RightToLeft); + QVERIFY2(window.centralWidget() != nullptr, "no central widget"); + QCOMPARE(window.centralWidget()->layoutDirection(), Qt::RightToLeft); + + const int leftRtl = settledLeftOf(bannerLabel); + + // The banner is [label][stretch] in a QHBoxLayout we build ourselves. LTR pins the + // label near x=0; RTL must push it to the right by (roughly) the free space. If the + // layout ignored direction this delta would be ~0. + QVERIFY2(leftRtl - leftLtr > 100, + qPrintable(QStringLiteral("offline banner did not mirror under RTL: " + "label x LTR=%1 RTL=%2") + .arg(leftLtr) + .arg(leftRtl))); +} + +QTEST_MAIN(TstRtl) +#include "tst_rtl.moc"