Skip to content

test(trollshell): the binary crate has no system-tests bucket, so ~50 RefCell-abort fixes shipped with zero regression coverage #674

Description

@vibechoom

Surfaced by PR #673 while finishing #643. Filed because it is the reason an entire bug class in this repo has shipped fixes with zero regression coverage.

The gap

The system-tests cargo feature — the mechanism this repo uses to gate tests needing a display server or a real dbus-daemon — exists in four crates: hytte-bus, hytte-reactive, hytte-services, hytte-ui. It does not exist in trollshell. Verified: grep system-tests trollshell/Cargo.toml returns nothing.

So the binary crate has no gated bucket, xvfb-run cargo test -p trollshell --features system-tests is not a thing, and flake.nix's system-tests check never exercises it.

Why that matters right now

The RefCell-borrow-held-across-a-GTK-call cluster (#627, #630, #631, #632, #638, #643) has now shipped roughly 50 fixes across #644, #663 and #673. Its failure mode is not a wrong pixel — a BorrowMutError panicking through a glib callback aborts the process, which #663 demonstrated for real with a SIGABRT.

Of those ~50 sites, the ones in hytte-ui got a genuine regression test (close_catchers_tolerates_a_reentrant_teardown_from_destroy, using #[gtk::test] in that crate's gated bucket). Every site in trollshell itself got none — not because nobody tried, but because there is nowhere to put one. modal.rs's own test module says as much.

That is the whole binary: every overlay, the drawer, and the bar widgets.

The Waker harness is not the answer here, but it is closer than #663 thought

#663 concluded the futures-signals Waker trick did not apply to its sites, because every reentrancy trigger was a GTK emission rather than a Mutable wake. #673 found that is not universally true — at least two sites do have a Mutable wake inside the borrow:

  • wire_retract_finish's open_state.set(false)
  • show_panel_active's set(true)

Both are reachable in principle by the pure-std RawWaker harness. What blocks them is that reaching either needs a populated PANELS — i.e. real GTK windows — which puts you back in display-server territory regardless.

So the honest framing is: a display-backed bucket unlocks both styles of test, and nothing unlocks them without one.

Proposal

  1. Add a system-tests feature to trollshell/Cargo.toml, matching the four existing crates' convention exactly (#[cfg(feature = "system-tests")] on the mod tests, not #[ignore], so the default cargo test does not even compile them).
  2. Follow hytte-ui's #[gtk::test] pattern for the harness shape — it already works under xvfb-run.
  3. Wire it into flake.nix's existing system-tests check so CI actually runs it. Adding the feature without wiring the check would be the worst of both worlds: a bucket that exists and never runs.
  4. Seed it with regression tests for the two Mutable-wake sites above, since those are the cheapest real ones and they pin the exact mechanism the cluster is about.

Worth checking whether a trollshell GTK test can stand up enough of PANELS without a full App, or whether this needs a test-only constructor. If it needs one, that design question is the actual work here — the Cargo plumbing is trivial by comparison.

Severity

Medium. No user-visible defect; this is test debt. It is rated above low because the class it fails to cover aborts the process, has produced six issues so far, and every fix to date has been merged on the strength of code review alone. The next regression in this area will be found by a user, not by CI.

Refs #673, #663, #644, #643, #631.

Metadata

Metadata

Assignees

No one assigned

    Labels

    code-climateGrounded finding from a code-climate / architecture audit sweep

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions