From d62a9eb9694beee1674f9c63f9a77029a81627ee Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 7 Aug 2026 18:02:51 +0200 Subject: test: pin the offscreen platform, and let the popup test check its own geometry ctest sets no QT_QPA_PLATFORM, so the suite's verdict depended on how it was invoked: green for anyone exporting `offscreen` by hand, red under ctest in the same tree. That cost a wrong diagnosis before the cause was found, an unrelated change blamed for a failure that reproduces on a clean checkout. The Wayland warning blames a missing transientParent, which is misleading. Instrumenting the test showed the popup viewport arriving as 1278x0. The zero height is why the grab returned a null pixmap, but the width is the worse half: this test sizes a line edit to 550px and exists to prove a description survives a popup that size, so a working grab would have measured a popup twice that wide and passed while proving nothing. Offscreen gives 548x40, the geometry the test means. Pinning the platform is therefore the correct fix rather than an opt-out. It is set in add_qtmaildir_test so it covers every test, including the two others that create widgets, and anything added later. The test now also asserts its own geometry, since CMake only governs ctest and the binary is often run directly. A bare !shot.isNull() said nothing useful; it now reports "popup viewport has no height (1278x0)" or names a popup too wide to be the case under test. Both guards were verified by mutation. --- .../plans/2026-08-03-post-0.1.0-usability.md | 31 +++++++++++++++++++++- tests/CMakeLists.txt | 21 +++++++++++++++ tests/test_querycompleter.cpp | 23 ++++++++++++++++ 3 files changed, 74 insertions(+), 1 deletion(-) 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); -- cgit v1.2.3