Skip to content

feat: add unit tests for core modules and protocol integration - #1211

Draft
Groveer wants to merge 3 commits into
masterfrom
feat/wm29-combined-unit-tests
Draft

feat: add unit tests for core modules and protocol integration#1211
Groveer wants to merge 3 commits into
masterfrom
feat/wm29-combined-unit-tests

Conversation

@Groveer

@Groveer Groveer commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Consolidates the unit-test work from #1046 (feat/add-unit-tests) and #1093 (refactor/greeter-virtual-methods) into a single, single-commit PR, per the WM-29 request to merge the two PRs and minimize commit count. The two branches shared one base commit and diverged only in a null-guard fix; this PR is their union squashed into one commit on top of the current base.

Note: the original PR heads live on the deepin-wm fork, which the current runtime account has no push access to, so the consolidated branch was pushed to linuxdeepin/treeland directly. #1046 and #1093 are closed in favor of this PR.

What's included

Testability support (production code, minimal / non-behavioral):

  • CMakePresets: excludeLabel for testability
  • lockscreen: primaryOutputName() getter
  • output: constrainToValidArea made public static
  • xresource: splitXResourceLine public static
  • xsettings: testMode constructor + expose protected methods
  • backlight: basePath parameter for test injection
  • greeterproxy: testMode constructor, virtual methods + null guards
  • rootsurfacecontainer / shellhandler / helper / workspace / surfacewrapper: TODO(unit-test) markers

New unit tests (13 executables): test_backlight, test_gestures, test_greeterproxy, test_lockscreen, test_output_scale, test_outputconfigstate, test_protocol_integration, test_shortcutcontroller, test_togglablegesture, test_wallpaperconfig, test_windowconfigstore, test_xresource, test_xsettings

Fixes:

  • fix(surface): guard null parentContainer() in ensureQmlContext() (was a null-pointer deref / SEGFAULT)
  • fix(greeter): switchUser() is a Qt signal and cannot be virtual (MOC) — removed virtual, mock uses QSignalSpy
  • fix(test): pass a non-null parent SurfaceContainer to LockScreen
  • fix(test): value-initialize WallpaperWorkspaceConfig in test_wallpaperconfig (-Werror=maybe-uninitialized on Deepin)

Base branch

Targets master-backup-20260717 — the same staging base the original PRs used (a stable snapshot while master churns) — to preserve the previously-green CI. The base has since advanced by one commit (fix(greeter): guard null socket in isConnected + test_effect_glass), which produced a single additive conflict in tests/CMakeLists.txt, resolved by keeping both test_effect_glass and the new test entries. The greeterproxy changes combined cleanly.

If you'd prefer this retargeted at master to land directly, say the word — it only needs the same tests/CMakeLists.txt resolution (plus a CI re-run).

Review status

The underlying changes were already code-reviewed and CI-green on the original PRs (Arch Linux + Deepin crimson). This PR re-runs CI on the squashed commit / current base.

Ref: WM-29 (Multica issue c885e061-87b5-4415-8284-47bc31e0efe0)
Supersedes #1046 and #1093.

Summary by Sourcery

Add extensive unit test coverage for core compositor modules and Wayland protocol integration, along with minor production changes to enable testability and harden backlight handling.

New Features:

  • Introduce a protocol integration test harness that spins up an in-process WServer and exercises wallpaper color, window management, virtual output, and prelaunch splash Wayland protocols.
  • Add unit tests for gesture recognition and togglable gestures, keyboard shortcut handling, lockscreen behavior, greeter proxy logic, output scaling and window positioning, wallpaper configuration, window configuration persistence, backlight control, X resource parsing, X settings serialization, and output configuration state tracking.

Bug Fixes:

  • Prevent a potential null-pointer dereference in SurfaceContainer::ensureQmlContext() by guarding against a null parent container.
  • Make backlight handling more robust by tolerating missing or invalid sysfs files and avoiding division by zero when computing brightness.

Enhancements:

  • Make Backlight configurable with an injectable base path to support testing against temporary directories.
  • Expose Output::constrainToValidArea as a static helper for use in tests and other call sites.
  • Introduce a test-mode constructor and virtual hooks in GreeterProxy to allow isolated testing of lock and shutdown view behavior.
  • Expose internal XSettings and XResource helpers (constructors and parsing utilities) to enable direct unit testing without a live X11 connection.
  • Add TODO(unit-test) annotations to several tightly coupled core classes (Helper, RootSurfaceContainer, ShellHandler, SurfaceWrapper, Workspace) to document future testability refactors.
  • Expose LockScreen::primaryOutputName via a getter for verification in tests.

Tests:

  • Register 13 new test targets in CTest covering backlight, gestures, greeterproxy, lockscreen, output scale, output configuration state, protocol integration, shortcut controller, togglable gestures, wallpaper config, window config store, XResource, and XSettings.
  • Add per-test CMake configurations, dependencies, and environment settings (including Wayland protocol code generation for the protocol integration test) to compile and run the new unit tests.

@deepin-ci-robot

Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@sourcery-ai

sourcery-ai Bot commented Jul 28, 2026

Copy link
Copy Markdown

Reviewer's Guide

Adds a suite of unit tests for core components and Wayland protocol integration, with small production changes to make components testable (backlight I/O, greeter proxy, lockscreen, output geometry helpers, XSettings/XResource), plus a null-guard fix in SurfaceContainer and a few test/greeter robustness fixes.

Sequence diagram for GreeterProxy.lock with testMode

sequenceDiagram
    actor Tester
    participant GreeterProxy
    participant Helper
    participant SessionManager
    participant Session

    Tester->>GreeterProxy: lock()
    alt [m_testMode]
        GreeterProxy->>GreeterProxy: setLock(true)
        GreeterProxy-->>Tester: return
    else [!m_testMode]
        GreeterProxy->>Helper: instance()
        Helper-->>GreeterProxy: Helper*
        GreeterProxy->>SessionManager: sessionManager()
        SessionManager-->>GreeterProxy: SessionManager*
        GreeterProxy->>SessionManager: activeSession()
        SessionManager-->>GreeterProxy: QSharedPointer<Session>
        GreeterProxy-->>Tester: return
    end
Loading

File-Level Changes

Change Details Files
Make Backlight safer and testable via injectable base path and graceful error handling.
  • Extend Backlight constructor to accept an optional basePath and store it on the instance.
  • Replace hard-coded /sys paths with basePath-derived paths and convert Q_ASSERTs on I/O to qCWarning + early returns.
  • Guard brightness() against zero/invalid maxBrightness and check write success in setBrightness().
src/output/backlight.cpp
src/output/backlight.h
Expose Output geometry helper and add tests for scaling and placement constraints.
  • Promote constrainToValidArea from an instance slot to a public static method on Output, adjusting the header accordingly.
  • Add unit tests to cover calcPreferredScale for a variety of DPI/size combinations and constrainToValidArea edge cases.
src/output/output.h
tests/test_output_scale/main.cpp
tests/test_output_scale/CMakeLists.txt
Introduce a test mode path for GreeterProxy and remove invalid virtual signal usage.
  • Add a GreeterProxy(bool testMode, QObject*) constructor that skips runtime setup and records testMode.
  • Change isLocked(), showShutdownView(), setShowShutdownView(), and lock() to be virtual and adapt lock() to short-circuit in test mode by toggling internal state instead of talking to real sessions.
  • Add a focused unit test exercising basic property defaults, showShutdownView change notifications, test-mode lock behavior, and switchUser emission; ensure switchUser remains a Qt signal (not virtual).
src/greeter/greeterproxy.cpp
src/greeter/greeterproxy.h
tests/test_greeterproxy/main.cpp
tests/test_greeterproxy/CMakeLists.txt
Make XSettings and XResource internals testable and add serialization/parse coverage.
  • Add a protected XSettings(QObject*) ctor and move depopulateSettings/populateSettings into protected so tests can subclass and call them.
  • Make XResource::splitXResourceLine a public static helper and expose copyright years updates.
  • Add tests that round-trip XSettings integer/string/color values and validate key-to-byte-array mapping, plus tests that validate XResource line splitting semantics and toByteArray mapping.
src/xsettings/xsettings.h
src/xsettings/xsettings.cpp
src/xsettings/xresource.h
src/xsettings/xresource.cpp
tests/test_xsettings/main.cpp
tests/test_xsettings/CMakeLists.txt
tests/test_xresource/main.cpp
tests/test_xresource/CMakeLists.txt
Fix a SurfaceContainer null dereference and adjust LockScreen to be more testable.
  • Guard SurfaceContainer::ensureQmlContext() against a null parentContainer() before calling qmlEngine/setQmlEngine.
  • Expose LockScreen::primaryOutputName() as a simple getter and wire tests that use a minimal SurfaceContainer(parentItem) to avoid QML engine setup, along with a mock ILockScreen and GreeterProxy for behavior coverage.
  • Add unit tests that validate LockScreen visibility/lock/shutdown/switch-user flows and locking state computation.
src/surface/surfacecontainer.cpp
src/core/lockscreen.h
tests/test_lockscreen/main.cpp
tests/test_lockscreen/CMakeLists.txt
Annotate complex, runtime-coupled classes with unit-test TODOs to guide future refactors.
  • Add TODO(unit-test) comments to Helper, RootSurfaceContainer, ShellHandler, Workspace, and SurfaceWrapper explaining why they are currently hard to unit-test and sketching refactor directions (dependency injection, protocol logic extraction, separation of rendering vs logic).
src/seat/helper.h
src/core/rootsurfacecontainer.h
src/core/shellhandler.h
src/surface/surfacewrapper.h
src/workspace/workspace.h
Add targeted unit tests for gestures, shortcut controller, wallpaper config, backlight, output config state, window config, and togglable gestures.
  • Create test_gestures covering SwipeGesture defaults, finger counts, deltas, position constraints, and GestureRecognizer start/update/cancel/end flows including axis lock and hold gestures.
  • Create test_togglablegesture covering state machine transitions, partial progress/regress behavior, threshold semantics, and stop/toggle API.
  • Create test_shortcutcontroller validating normalization rules, allowed/invalid combinations, registration conflicts, event dispatch, and modifierForAction.
  • Add tests for WallpaperWorkspaceConfig/WallpaperOutputConfig JSON round-trip, OutputConfigState primary-screen/copy-mode flags, WindowConfigStore construction and saveLastSize edge cases, and the new Backlight behavior using a temporary sysfs-like directory.
tests/test_gestures/main.cpp
tests/test_gestures/CMakeLists.txt
tests/test_togglablegesture/main.cpp
tests/test_togglablegesture/CMakeLists.txt
tests/test_shortcutcontroller/main.cpp
tests/test_shortcutcontroller/CMakeLists.txt
tests/test_wallpaperconfig/main.cpp
tests/test_wallpaperconfig/CMakeLists.txt
tests/test_outputconfigstate/main.cpp
tests/test_outputconfigstate/CMakeLists.txt
tests/test_windowconfigstore/main.cpp
tests/test_windowconfigstore/CMakeLists.txt
tests/test_backlight/main.cpp
tests/test_backlight/CMakeLists.txt
Add an end-to-end Wayland protocol integration test harness and plug all new tests into CTest.
  • Add test_protocol_integration which spins up a minimal WServer + WSocket, attaches wallpaper-color, window-management, virtual-output-manager, and prelaunch-splash interfaces, and exercises client-side Wayland interactions via generated protocol stubs and socketpair transport; mark the test as 'manual' and increase timeout.
  • Register all new test targets in tests/CMakeLists.txt, ensuring they run offscreen via QT_QPA_PLATFORM and coexisting with existing tests like test_effect_glass.
  • Add dedicated CMakeLists for each new test that link against libtreeland and Qt::Test, and in the protocol case also TreelandProtocols and wayland-client tooling.
tests/CMakeLists.txt
tests/test_protocol_integration/main.cpp
tests/test_protocol_integration/CMakeLists.txt
tests/test_output_scale/main.cpp
tests/test_output_scale/CMakeLists.txt
tests/test_wallpaperconfig/main.cpp
tests/test_wallpaperconfig/CMakeLists.txt
tests/test_outputconfigstate/main.cpp
tests/test_outputconfigstate/CMakeLists.txt
tests/test_shortcutcontroller/main.cpp
tests/test_shortcutcontroller/CMakeLists.txt
tests/test_togglablegesture/main.cpp
tests/test_togglablegesture/CMakeLists.txt
tests/test_gestures/main.cpp
tests/test_gestures/CMakeLists.txt
tests/test_greeterproxy/main.cpp
tests/test_greeterproxy/CMakeLists.txt
tests/test_lockscreen/main.cpp
tests/test_lockscreen/CMakeLists.txt
tests/test_windowconfigstore/main.cpp
tests/test_windowconfigstore/CMakeLists.txt
tests/test_backlight/main.cpp
tests/test_backlight/CMakeLists.txt
tests/test_xresource/main.cpp
tests/test_xresource/CMakeLists.txt
tests/test_xsettings/main.cpp
tests/test_xsettings/CMakeLists.txt

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@Groveer
Groveer force-pushed the feat/wm29-combined-unit-tests branch from c5ed18d to 02bbcdc Compare July 28, 2026 04:14
@Groveer
Groveer marked this pull request as ready for review July 28, 2026 04:32

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @Groveer, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@Groveer
Groveer force-pushed the feat/wm29-combined-unit-tests branch from 02bbcdc to 7f328e4 Compare July 28, 2026 04:34
@Groveer
Groveer changed the base branch from master-backup-20260717 to master July 28, 2026 04:34
@github-actions

Copy link
Copy Markdown

TAG Bot

TAG: 0.8.16
EXISTED: yes
DISTRIBUTION: unstable

@Groveer Groveer closed this Jul 28, 2026
@Groveer Groveer reopened this Jul 28, 2026
@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Groveer

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Groveer and others added 3 commits July 28, 2026 20:59
Consolidates the unit-test work from #1046 and #1093 into a single
commit. Adds testability hooks to core modules and a suite of unit
tests, plus the production bugfixes uncovered while writing the tests.

Testability support (production code, minimal / non-behavioral changes):
- CMakePresets: add excludeLabel for testability
- lockscreen: add primaryOutputName() getter
- output: make constrainToValidArea public static
- xresource: add splitXResourceLine public static
- xsettings: add testMode constructor and expose protected methods
- backlight: add basePath parameter for test injection
- greeterproxy: add testMode constructor, virtual methods and null guards
- rootsurfacecontainer / shellhandler / helper / workspace / surfacewrapper:
  add TODO(unit-test) markers

New unit tests (12 executables):
- test_backlight, test_gestures, test_greeterproxy, test_lockscreen,
  test_output_scale, test_protocol_integration, test_shortcutcontroller,
  test_togglablegesture, test_wallpaperconfig, test_windowconfigstore,
  test_xresource, test_xsettings

Fixes:
- fix(surface): guard null parentContainer in ensureQmlContext() --
  constructing SurfaceContainer with a null parent dereferenced a null
  pointer from parentContainer(); add an early return.
- fix(greeter): switchUser() is a Qt signal and cannot be virtual (MOC
  generates non-virtual implementations); remove `virtual` and observe
  the signal with QSignalSpy in the mock instead.
- fix(test): pass a non-null parent SurfaceContainer (QQuickItem ctor
  path) to LockScreen to avoid triggering ensureQmlContext() with a
  null parent.
- fix(test): adapt test_wallpaperconfig to the current
  WallpaperWorkspaceConfig / WallpaperOutputConfig (the `enable` field was
  removed upstream since the tests were authored) and value-initialize
  aggregates where members are left unset, silencing
  -Werror=maybe-uninitialized on the Deepin deb build.
- fix(greeter): guard null m_socket in isConnected() — the testMode
  constructor does not create a socket, so isConnected() would dereference
  a null pointer; add an early return (same fix as the backup branch had).
- fix(test): drop test_outputconfigstate — OutputConfigState was removed
  from the codebase on master; the test is obsolete.

Ref: WM-29
Supersedes #1046 and #1093.
unregisterSwipeGesture/unregisterHoldGesture previously called
deleteLater() on the gesture object. This is dangerous because:
1. The QObject::destroyed signal handler also calls unregister,
   creating a circular dependency.
2. Callers may still reference the gesture after unregister.

Add removeSwipeGesture/removeHoldGesture that call unregister then
deleteLater, and update InputDevice callers to use the new methods.

WM-29
WallpaperWorkspaceConfig and WallpaperOutputConfig had uninitialized
members (workspaceId, desktopWallpapertype, lockScreenWallpapertype)
which led to undefined behavior when default-constructed, triggering
-Werror=maybe-uninitialized in the Deepin deb build.

Add default member initializers. Note: the 'enable' field referenced
in the original PR #1092 was removed from the codebase on master, so
it is not included here.

Ref: WM-29. Supersedes #1092.
@Groveer
Groveer force-pushed the feat/wm29-combined-unit-tests branch from 7846c0b to ac3ff34 Compare July 28, 2026 13:00
@deepin-bot

deepin-bot Bot commented Jul 31, 2026

Copy link
Copy Markdown

TAG Bot

New tag: 0.8.17
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1229

@deepin-bot

deepin-bot Bot commented Aug 14, 2026

Copy link
Copy Markdown

TAG Bot

New tag: 0.8.18
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1286

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants