Skip to content

refactor(greeter): make key methods virtual for testability - #1093

Closed
deepin-wm wants to merge 4 commits into
linuxdeepin:master-backup-20260717from
deepin-wm:refactor/greeter-virtual-methods
Closed

refactor(greeter): make key methods virtual for testability#1093
deepin-wm wants to merge 4 commits into
linuxdeepin:master-backup-20260717from
deepin-wm:refactor/greeter-virtual-methods

Conversation

@deepin-wm

Copy link
Copy Markdown
Contributor

Make isLocked(), showShutdownView(), setShowShutdownView(), lock(), and switchUser() virtual to allow test subclasses to override behavior without requiring a running DDM connection.

Extracted from #1046. Ref: WM-29

@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 @deepin-wm, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: deepin-wm

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

@deepin-wm
deepin-wm force-pushed the refactor/greeter-virtual-methods branch from f7d498e to 28e77aa Compare July 2, 2026 11:25
deepin-wm added 2 commits July 2, 2026 19:26
Add testability support and unit tests for treeland core modules:
- 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: add TODO(unit-test)
- surfacewrapper: add TODO(unit-test) comment
- Add 14 test directories covering core modules
Qt signals cannot be declared virtual — MOC generates non-virtual
implementations, so adding virtual to a signal causes undefined
behavior when a mock tries to override it (SEGFAULT in test_lockscreen).
@deepin-wm
deepin-wm force-pushed the refactor/greeter-virtual-methods branch from 28e77aa to 8789080 Compare July 2, 2026 12:13
deepin-wm added 2 commits July 2, 2026 20:32
Since switchUser() is no longer virtual (signals cannot be virtual),
MockGreeterProxy::switchUser() override causes a compile error.
Replace with QSignalSpy to observe the base class signal emission.
When SurfaceContainer is constructed with a null parent,
ensureQmlContext() would dereference a null pointer from
parentContainer(). Add early return for null.

Also update test_lockscreen to pass a non-null parent
SurfaceContainer (via QQuickItem constructor path) to
avoid triggering ensureQmlContext() with a null parent.

Also value-initialize WallpaperWorkspaceConfig in test_wallpaperconfig
to silence -Werror=maybe-uninitialized on the Deepin deb build.
@deepin-wm
deepin-wm force-pushed the refactor/greeter-virtual-methods branch from 785f4da to 4cff91e Compare July 2, 2026 14:00
@deepin-bot

deepin-bot Bot commented Jul 10, 2026

Copy link
Copy Markdown

TAG Bot

New tag: 0.8.14
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1123

@deepin-bot

deepin-bot Bot commented Jul 17, 2026

Copy link
Copy Markdown

TAG Bot

New tag: 0.8.15
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1163

@Groveer

Groveer commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Closed in favor of #1211, which consolidates this PR with #1046 into a single squashed commit per the WM-29 request to merge the two PRs and minimize commit count. This branch lives on the deepin-wm fork, which the current runtime account has no push access to, so the consolidated commit was pushed to linuxdeepin/treeland:feat/wm29-combined-unit-tests instead. CI is being verified on #1211.

@Groveer Groveer closed this Jul 28, 2026
Groveer added a commit that referenced this pull request Jul 28, 2026
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 (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() --
  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.

Ref: WM-29
Supersedes #1046 and #1093.
Groveer added a commit that referenced this pull request Jul 28, 2026
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 (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() --
  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.

Ref: WM-29
Supersedes #1046 and #1093.
Groveer added a commit that referenced this pull request Jul 28, 2026
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(test): drop test_outputconfigstate — OutputConfigState was removed
  from the codebase on master; the test is obsolete.

Ref: WM-29
Supersedes #1046 and #1093.
Groveer added a commit that referenced this pull request Jul 28, 2026
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.
Groveer added a commit that referenced this pull request Jul 28, 2026
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.
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.

3 participants