diff options
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 31 | ||||
| -rw-r--r-- | tests/CMakeLists.txt | 21 | ||||
| -rw-r--r-- | tests/test_querycompleter.cpp | 23 |
3 files changed, 74 insertions, 1 deletions
diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md index cefdfc3..6405e75 100644 --- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md +++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md @@ -100,7 +100,7 @@ taking that too literally. | 49 | Sync runs every account regardless of what changed | workflow | M | open | | 50 | Esc blanks the pane but leaves the row selected | workflow | XS | open | | 51 | Clicking a subject scrolls the list sideways | presentation | XS | open | -| 52 | `test_querycompleter` fails under Wayland, passes offscreen | testing | XS | open | +| 52 | `test_querycompleter` fails under Wayland, passes offscreen | testing | XS | **done** | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -2942,6 +2942,35 @@ of them, the same shape as item 46: - Whatever is chosen must hold for `test_mainwindow` and `test_messageview` too, which create widgets and could grow the same dependency. +### Outcome (done 2026-08-07) + +**The warning named the wrong cause, and the real one was worse.** Wayland's +"failed to create grabbing popup" message points at a transientParent, so the +plan above proposed setting one. Instrumenting the test first showed what +actually reaches the assertion: the popup viewport measured **1278x0**. The zero +height is why the grab returned a null pixmap, but the 1278 is the important +half. This test sizes a line edit to 550px and exists to prove the description +survives a popup that size; under Wayland it was handed a popup more than twice +that wide, so a working grab would have measured a different popup and **passed +while proving nothing**. Offscreen gives 548x40, the geometry the test means. + +So pinning the platform is the correct fix rather than the cheap one, and the +preference recorded above was based on a misreading. Deterministic geometry is a +requirement of the test, not a convenience: a compositor is entitled to size a +popup how it likes. + +- `set_tests_properties(... ENVIRONMENT QT_QPA_PLATFORM=offscreen)` is applied + in `add_qtmaildir_test()`, so it covers every test including `test_mainwindow` + and `test_messageview` as the constraint required, and any test added later. +- **The test also guards its own geometry now**, since the CMake setting only + covers `ctest` and the binary is often run directly. It asserts the viewport + has a non-zero height and is no wider than 700px, each with a message naming + the cause. A bare `QVERIFY(!shot.isNull())` reported nothing useful; the guard + now says "popup viewport has no height (1278x0)". +- Both guards were **verified by mutation**: widening the line edit to 1200px + makes the width guard fail with its explanation, where before the change that + case would have passed. + ## Deferred, unsized, or split out Items noted while triaging but not part of the original list. Same numbering diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index c09ef78..a7bb670 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -5,10 +5,31 @@ # a global initialiser the linker then drops. Library code reads :/fonts/ when # it builds the placeholder pane, so a test that never links the qrc would # exercise only the missing-resource fallback and pass against a broken build. +# +# Every test runs under the OFFSCREEN platform, set here rather than left to +# whatever the invoking session happens to use. This is not tidiness: a test +# that renders needs deterministic geometry, and a real windowing system does +# not provide it. +# +# The case that forced it (item 52). theDescriptionSurvivesAModestPopupWidth +# sizes a line edit to 550px and asserts a description survives the popup that +# results. Under Wayland the compositor gave the popup a viewport of 1278x0: the +# zero height made the grab return a null pixmap, which is what failed, but the +# 1278 width is the worse half. Had the grab succeeded, the test would have +# measured a popup more than twice the width it claims to be testing and passed +# while proving nothing about the 550px case. +# +# So the platform is pinned for correctness, not to dodge a red result. `ctest` +# sets no QT_QPA_PLATFORM of its own, so without this the suite's verdict +# depends on how it was invoked: it passed for a developer exporting `offscreen` +# by hand and failed under `ctest` in the same tree, which cost one wrong +# diagnosis before the cause was found. function(add_qtmaildir_test name) add_executable(test_${name} test_${name}.cpp ${CMAKE_SOURCE_DIR}/src/resources.qrc) target_link_libraries(test_${name} PRIVATE qtmaildir_lib Qt6::Test) add_test(NAME ${name} COMMAND test_${name}) + set_tests_properties(${name} PROPERTIES + ENVIRONMENT "QT_QPA_PLATFORM=offscreen") endfunction() add_qtmaildir_test(keymap) diff --git a/tests/test_querycompleter.cpp b/tests/test_querycompleter.cpp index 6cf9651..4c1e866 100644 --- a/tests/test_querycompleter.cpp +++ b/tests/test_querycompleter.cpp @@ -818,6 +818,29 @@ void TestQueryCompleter::theDescriptionSurvivesAModestPopupWidth() QVERIFY(row.isValid()); QCOMPARE(row.data(Qt::DisplayRole).toString(), QStringLiteral("path:")); + // The geometry this test depends on, asserted rather than assumed. + // + // The whole point here is that the description survives a MODEST popup, so + // a popup the windowing system decided to make wide would pass while + // proving nothing about the 550px case. Under Wayland the compositor gave + // this popup a 1278x0 viewport: the zero height made the grab below return + // a null pixmap and the failure was at least loud, but the width would have + // been a silent false pass had the height been usable. The tests are pinned + // to the offscreen platform in tests/CMakeLists.txt for this reason; this + // guard is what makes a run outside ctest fail honestly instead. + const QSize viewport = popup->viewport()->size(); + QVERIFY2(viewport.height() > 0, + qPrintable(QStringLiteral("popup viewport has no height (%1x%2): " + "the platform never laid the popup out, " + "so any grab of it is empty") + .arg(viewport.width()).arg(viewport.height()))); + QVERIFY2(viewport.width() <= 700, + qPrintable(QStringLiteral("popup viewport is %1px wide, far more " + "than the ~550px this test exists to " + "check: it would measure a different " + "popup and pass for the wrong reason") + .arg(viewport.width()))); + const QRect rect = popup->visualRect(row); QVERIFY(rect.isValid()); QPixmap shot = popup->viewport()->grab(rect); |