Skip to content

test: add unit tests for session, discover and phone modules - #766

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
pengfeixx:feat/unit-tests-session-phone-discover
Jul 14, 2026
Merged

test: add unit tests for session, discover and phone modules#766
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
pengfeixx:feat/unit-tests-session-phone-discover

Conversation

@pengfeixx

@pengfeixx pengfeixx commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Add tests for session/proto (59 tests), discover controller (13 tests) and phone GUI (36 tests). Coverage improved across 3 modules.

为 session、discover 和 phone 模块补充单元测试,新增约 108 个测试用例, session 覆盖率提升至 40%,phone 提升至 65%,discover 补充数据操作覆盖。

Log: 补充 session/discover/phone 模块单元测试
Influence: 不影响现有功能,仅新增测试代码,提升模块测试覆盖率。

Summary by Sourcery

Add new unit test suites to improve coverage for session core networking, discovery controller data handling, and phone GUI/VNC components.

Tests:

  • Add session core tests covering proto client/server endpoints, IP mapping, heartbeat, ping handling, and cooperation core utilities.
  • Extend discover controller tests to validate device filtering, history management, search device integration, and JSON parsing behavior.
  • Add phone GUI tests for VNC viewer, VNC receive/send workers, and screen mirroring/lock screen windows, including basic lifecycle and event handling paths.
  • Update coop and coop_gui test CMake configurations to include the new session core and phone GUI test files and necessary include paths.

@sourcery-ai

sourcery-ai Bot commented Jul 14, 2026

Copy link
Copy Markdown

Reviewer's Guide

Adds ~108 new unit tests to increase coverage for the session core protocol (ProtoClient/ProtoServer/ProtoEndpoint, CooperationCore/Discover), and phone GUI VNC components (VncViewer, VNCRecvThread/VNCSendWorker, ScreenMirroringWindow/LockScreenWidget), wiring them into the coop and coop_gui test executables and extending includes as needed.

File-Level Changes

Change Details Files
Extend VncViewer GUI tests to cover rendering, event handling, and configuration paths without crashing.
  • Add tests for connection info storage, stop() early-return behavior, and safe image/surface updates when connected or not
  • Exercise paint, resize, frame timer, and background brush/scaling behavior to ensure they are crash-free
  • Verify close events emit fullWindowCloseSignal and that mouse press/move/release events are accepted via QApplication::sendEvent
tests/coop_gui/vncviewer_test.cpp
Wire new session-core and discover tests into the coop test binary and adjust its include paths.
  • Register new protoendpoint, protoclient_server, cooperationcore, and discover extra tests in coop_tests executable sources
  • Add session-core and tests/common include directories so new tests can include core headers
tests/coop/CMakeLists.txt
Wire new phone GUI tests into the coop_gui test binary.
  • Register new vncrecvthread/vncsendworker and screenmirroringwindow tests in coop_gui_tests executable sources
tests/coop_gui/CMakeLists.txt
Add unit tests for ProtoClient/ProtoServer behavior and their shared ProtoEndpoint interface.
  • Introduce a shared AsioService-based test fixture to start/stop IO service around each test
  • Cover ProtoClient construction, connection/heartbeat flags, real IP handling, callback registration, ping timer stop, and disconnect/stop lifecycle
  • Cover ProtoServer creation, session and real/remote IP mapping logic, callbacks, and ping tracking behavior
tests/session_core/protoclient_server_test.cpp
Add focused tests for ProtoEndpoint behavior via ProtoClient/ProtoServer instances, including sync and disconnect requests.
  • Reuse an AsioService-based fixture and a MockSessionCallback implementing SessionCallInterface
  • Verify default hasConnected/connectReplyed states, real IP setting, and callback wiring on client and server
  • Test syncRequest and sendDisRequest behavior when there is no active connection to ensure they fail gracefully and return empty/harmless results
tests/session_core/protoendpoint_test.cpp
Add tests for DiscoverController’s non-signal data manipulation utilities and device validation logic.
  • Reset singleton DiscoverController internal state before/after each test to keep tests isolated
  • Cover config and app-attribute change handlers with invalid inputs to ensure they are safely ignored
  • Test device validation with IP filters and history-device bypass, offline-history collection, search-device injection, and JSON parsing robustness
tests/session_core/discover_extra_test.cpp
Add tests for CooperationCore-related utilities around DeviceInfo and DiscoverController JSON parsing.
  • Check that default command-line arguments do not trigger minimize behavior
  • Add round-trip tests for DeviceInfo::fromVariantMap and for DiscoverController::parseDeviceJson with valid and invalid JSON payloads
tests/session_core/cooperationcore_test.cpp
Add tests for phone-side VNC receive thread and send worker classes, using libvncclient stubs.
  • Stub libvncclient pointer/key/framebuffer functions to avoid real network I/O
  • Exercise VNCRecvThread lifecycle methods (construct, startRun/stopRun, repeated stopRun) on a fake rfbClient
  • Exercise VNCSendWorker mouse and key update methods to ensure they call through the stubs without crashing
tests/coop_gui/vncrecvthread_vncsendworker_test.cpp
Add tests for ScreenMirroringWindow and LockScreenWidget behavior around sizing and painting.
  • Ensure ScreenMirroringWindow construction/destruction, including VncViewer teardown, does not crash
  • Cover initialization by view size, VNC server connection attempts, orientation-dependent size handling, and idempotent size-change handling
  • Verify buttonClicked signal baseline state and that LockScreenWidget repaint/paintEvent are crash-free
tests/coop_gui/screenmirroringwindow_test.cpp

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

@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.

Hey - I've left some high level feedback:

  • Several tests rely on shared singletons (e.g., DiscoverController::instance()) and mutate internal state; to avoid cross-test interference or flakiness, consider encapsulating state reset into a helper and ensuring all relevant fields (including any added later) are consistently restored for each test.
  • The VNCRecvThread tests depend on QThread::msleep-based timing; if these become unstable on slower CI, consider introducing a more deterministic mechanism (e.g., injecting a test hook or using a short loop with a timeout that waits for a state flag) instead of fixed sleeps.
  • parseDeviceJson("not a json") is tested in both discover_extra_test and cooperationcore_test; you could consolidate this case in one place to reduce duplication and keep future behavior changes easier to maintain.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Several tests rely on shared singletons (e.g., DiscoverController::instance()) and mutate internal state; to avoid cross-test interference or flakiness, consider encapsulating state reset into a helper and ensuring all relevant fields (including any added later) are consistently restored for each test.
- The VNCRecvThread tests depend on QThread::msleep-based timing; if these become unstable on slower CI, consider introducing a more deterministic mechanism (e.g., injecting a test hook or using a short loop with a timeout that waits for a state flag) instead of fixed sleeps.
- parseDeviceJson("not a json") is tested in both discover_extra_test and cooperationcore_test; you could consolidate this case in one place to reduce duplication and keep future behavior changes easier to maintain.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@pengfeixx
pengfeixx force-pushed the feat/unit-tests-session-phone-discover branch 2 times, most recently from 25d8365 to 6aeb5ab Compare July 14, 2026 09:02
Add tests for session/proto (59 tests), discover controller (13 tests)
and phone GUI (36 tests). Coverage improved across 3  modules.

为 session、discover 和 phone 模块补充单元测试,新增约 108 个测试用例,
session 覆盖率提升至 40%,phone 提升至 65%,discover 补充数据操作覆盖。

Log: 补充 session/discover/phone 模块单元测试
Influence: 不影响现有功能,仅新增测试代码,提升模块测试覆盖率。
@pengfeixx
pengfeixx force-pushed the feat/unit-tests-session-phone-discover branch from 6aeb5ab to c8c3e3f Compare July 14, 2026 09:13
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

★ 总体评分:85分

■ 【总体评价】

代码扩展了VNC和Cooperation模块的单元测试覆盖,测试结构清晰但存在固定延时等待的稳定性隐患
逻辑正确且无安全漏洞,因测试中使用固定睡眠时间及冗余memset操作扣15分

■ 【详细分析】

  • 1.语法逻辑(基本正确)✓

测试代码使用GTest框架,语法符合规范,CMakeLists.txt中新增测试文件的编译配置正确。但在vncrecvthread_vncsendworker_test.cpp中对rfbClient使用memset清零,若该结构体包含非POD成员可能导致未定义行为。
潜在问题:memset操作可能破坏非POD对象的内部状态
建议:移除memset操作,依赖std::make_unique的值初始化或显式调用初始化函数

  • 2.代码质量(良好)✓

测试用例命名清晰,注释说明了测试意图,代码结构符合GTest最佳实践。SetUp和TearDown方法正确清理了单例状态。
潜在问题:memset清零操作属于冗余代码
建议:移除不必要的memset调用

  • 3.代码性能(存在性能问题)✕

在VNCRecvThreadTest.StartAndStopCycle测试中使用了QThread::msleep(100)进行固定延时等待,这会导致测试执行效率降低且可能在慢速机器上出现竞态条件。
潜在问题:固定延时可能导致测试不稳定
建议:使用QSignalSpy或条件变量替代固定睡眠,通过信号或事件驱动等待线程状态变更

  • 4.代码安全(存在0个安全漏洞)✓

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
本次新增的测试代码仅涉及本地对象构造和内存操作,未引入外部输入处理或网络通信逻辑,不存在命令注入、缓冲区溢出等安全风险。

  • 建议:无需特殊安全修复

■ 【改进建议代码示例】

--- a/tests/coop_gui/vncrecvthread_vncsendworker_test.cpp
+++ b/tests/coop_gui/vncrecvthread_vncsendworker_test.cpp
@@ -33,11 +33,9 @@ TEST(VNCRecvThreadTest, StartAndStopCycle)
 {
     VNCRecvThread t;
     auto fakeClient = std::make_unique<rfbClient>();
-    memset(fakeClient.get(), 0, sizeof(rfbClient));
 
     t.startRun(fakeClient.get());
-    // Thread starts, WaitForMessage on fake client fails quickly
-    QThread::msleep(100);
+    // 直接请求停止并等待线程退出,避免固定延时导致的测试不稳定
 
     t.stopRun();
     ASSERT_TRUE(t.wait(2000)) << "Thread did not stop in time";

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lzwind, pengfeixx

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

@pengfeixx

Copy link
Copy Markdown
Contributor Author

/merge

@deepin-bot
deepin-bot Bot merged commit 2da37c3 into linuxdeepin:master Jul 14, 2026
19 checks passed
@pengfeixx
pengfeixx deleted the feat/unit-tests-session-phone-discover branch July 14, 2026 09:31
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