Skip to content

test: migrate unit tests from Qt5 to Qt6 environment#679

Open
pengfeixx wants to merge 1 commit into
linuxdeepin:masterfrom
pengfeixx:test/qt6-unittest-migration
Open

test: migrate unit tests from Qt5 to Qt6 environment#679
pengfeixx wants to merge 1 commit into
linuxdeepin:masterfrom
pengfeixx:test/qt6-unittest-migration

Conversation

@pengfeixx
Copy link
Copy Markdown
Contributor

Update CMake configuration, test source code, and build scripts to compile and run under Qt6/DTK6.

将单元测试从Qt5迁移至Qt6环境,更新CMake配置、测试源码及构建脚本。

Log: 迁移单元测试至Qt6环境
Influence: 单元测试现在可以在Qt6/DTK6环境下编译运行,修复了Qt5→Qt6 API变更导致的编译错误和运行时崩溃。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 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

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

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

Please try again later or upgrade to continue using Sourcery

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 3, 2026

@pengfeixx pengfeixx force-pushed the test/qt6-unittest-migration branch from 87e76e0 to fb8fd2c Compare June 3, 2026 07:54
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 3, 2026

@pengfeixx pengfeixx force-pushed the test/qt6-unittest-migration branch from fb8fd2c to 9612b23 Compare June 3, 2026 07:59
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 3, 2026

Update CMake configuration, test source code, and build scripts
to compile and run under Qt6/DTK6.

将单元测试从Qt5迁移至Qt6环境,更新CMake配置、测试源码及构建脚本。

Log: 迁移单元测试至Qt6环境
Influence: 单元测试现在可以在Qt6/DTK6环境下编译运行,修复了Qt5→Qt6 API变更导致的编译错误和运行时崩溃。
@pengfeixx pengfeixx force-pushed the test/qt6-unittest-migration branch from 9612b23 to 67ee089 Compare June 3, 2026 08:40
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这份 Git Diff 主要是将项目的单元测试从 Qt5 迁移到 Qt6,并对构建脚本(CMakeLists.txt 和 Shell 脚本)进行了相应的调整和优化。整体来看,迁移工作做得很扎实,针对 Qt6 的 API 变更(如 QMouseEventQProcess::startQRegularExpression::match 等)都进行了正确的适配。

以下是对本次代码变更的详细审查意见,分为语法逻辑、代码质量、代码性能和代码安全四个方面:

一、 语法与逻辑

  1. 版权信息格式错误(重要)

    • 问题:在 ut_devicecpu.cpput_deviceinfo.cpp 等多个文件中,版权信息被修改为:
      // Copyright (C) 2019-2026 ~ 2020 UnionTech Software Technology Co.,Ltd
      这里的 2019-2026 ~ 2020 逻辑上是混乱的。~ 通常表示“至”,这样的写法让人无法理解是 2019 至 2026,还是 2019 至 2020。
    • 建议:统一修改为合理的格式,例如 2019-2026 或保留原来的 2019 ~ 2020
  2. CMake 绝对路径包含(潜在风险)

    • 问题:在 deepin-devicemanager-server/tests/CMakeLists.txt 中添加了:
      include_directories(/usr/include/deepin-qdbus-service)
      硬编码系统绝对路径会破坏跨平台和跨发行版的兼容性。如果该库在其他路径(如 /usr/local/include 或 macOS 上),编译将直接失败。
    • 建议:使用 CMake 的 find_packagepkg_check_modules 来查找该依赖库,并使用其提供的目标或变量来包含头文件和链接库。例如:
      find_package(PkgConfig REQUIRED)
      pkg_check_modules(QDBUS_SERVICE REQUIRED deepin-qdbus-service)
      target_include_directories(your_target PRIVATE ${QDBUS_SERVICE_INCLUDE_DIRS})
      target_link_libraries(your_target ${QDBUS_SERVICE_LIBRARIES})
  3. CMake 中 file(GLOB_RECURSE ...) 的使用(逻辑隐患)

    • 问题deepin-devicemanager/tests/CMakeLists.txt 中使用了 file(GLOB_RECURSE TEST_SRC_CPP ...) 来收集源文件,然后又用 list(REMOVE_ITEM ...) 来排除不需要的文件。
    • 建议:CMake 官方不推荐使用 GLOB 收集源文件,因为新增/删除文件不会自动触发 CMake 重新配置。虽然这里通过 REMOVE_ITEM 解决了旧文件的问题,但如果后续新增测试文件,开发者可能会忘记重新运行 CMake。建议如果项目规模允许,手动列出源文件是最稳妥的做法;如果坚持用 GLOB,请在注释中明确提醒团队。

二、 代码质量

  1. Shell 脚本健壮性提升(优秀)

    • 本次对 test-prj-running.sh 的重构质量很高。引入了 SCRIPT_DIRPROJECT_ROOT 等变量,消除了相对路径 ../../ 带来的不可靠性;使用 mkdir -p 替代 mkdir;使用 $(nproc) 替代硬编码的 -j8;增加了测试文件存在性检查(if [ -f ... ]);以及使用了 || true 来防止非关键命令报错导致脚本中断。这些都是极好的改进。
  2. 测试用例被禁用(需关注)

    • 问题:在 ut_driverlistview.cpp 中,UT_DriverListView_drawRow 被标记为 DISABLED_。注释解释是因为 Qt6 offscreen 模式下 ASan 会导致 SEGV。
    • 建议:虽然这是 Qt6 的环境问题而非业务代码 Bug,但被 DISABLED 的测试用例很容易被永久遗忘。建议:
      1. 在项目的 Issue Tracker 中创建一个 Issue 记录此问题,并在代码注释中附上 Issue 链接。
      2. 考虑是否可以通过调整 ASan 的抑制文件(suppressions)或使用 QT_QPA_PLATFORM=offscreen 外加虚拟帧缓冲(Xvfb)来绕过此崩溃,尽量恢复测试的执行。
  3. 硬编码的测试路径

    • 问题ut_driverlistview.cpp 中存在硬编码路径:
      QString path = "/data/home/jixiaomei/.local/share/Trash/files/deepin-devicemanager_5.6.12.13-1_arm64.deb";
    • 建议:这是开发者的个人本地路径,在其他构建环境(如 CI/CD 或其他开发者机器上)必然不存在,会导致该测试逻辑失效。应替换为项目测试数据目录下的相对路径,或使用 QTemporaryDir 动态创建测试文件。

三、 代码性能

  1. QImage 绘图设备的分配

    • 问题ut_driverlistview.cpp 中为了绕过 Qt6 无 paint engine 的问题,每次测试都创建了一个与 Widget 等大的 QImage
      QImage image(m_DriverListView->size(), QImage::Format_ARGB32);
    • 建议:对于单元测试而言,这种开销是可以接受的。但如果 m_DriverListView->size() 非常大,可能会造成瞬时内存尖峰。作为防御性编程,建议在创建前检查 size 的有效性,或者给一个固定的较小尺寸(如 100x100),只要能满足单元测试覆盖 drawRow 的逻辑即可,无需渲染真实尺寸。
  2. Shell 脚本中的 make -k

    • 问题make -k -j$(nproc) || true 中的 -k(continue after errors)意味着即使编译出错也继续编译其他目标,最后通过 || true 忽略错误继续跑测试。
    • 建议:在 CI 环境中,这会导致编译失败后依然去运行旧的测试二进制文件,掩盖了编译错误。建议在 CI 流水线中,编译阶段不要使用 || true,让错误及时暴露;如果是本地调试脚本,当前写法可以接受。

四、 代码安全

  1. Shell 脚本变量未加引号(高风险)

    • 问题:在 test-prj-running.sh 中,大量使用了未加双引号的变量,如:
      rm -rf $BUILD_DIR
      rm -rf $REPORT_DIR
      cp -r html $REPORT_DIR/ 2>/dev/null || true
      如果 SCRIPT_DIR 或路径中包含空格(如 /home/user/my project/),rm -rf $BUILD_DIR 会被展开成 rm -rf /home/user/my project/,即删除 /home/user/myproject/,造成灾难性的数据破坏。
    • 建议:必须为所有路径变量加上双引号:
      rm -rf "$BUILD_DIR"
      rm -rf "$REPORT_DIR"
      cp -r html "$REPORT_DIR/" 2>/dev/null || true
  2. lcov 覆盖率信息泄露

    • 问题:脚本收集了系统级别的覆盖率信息,然后使用 lcov --extract 提取 */src/*
    • 建议:在提取前,建议增加一步 lcov --remove ./coverage.info '/usr/*' '*/build/*' 以剔除系统头文件和第三方库的干扰,这不仅能提升代码安全性(避免暴露系统库信息),还能加快 genhtml 的生成速度并使覆盖率报告更精准。

总结

本次 Qt6 迁移的代码修改整体思路清晰,对 Qt6 API 变更的修复十分准确。最需要立即修复的是 Shell 脚本中变量未加引号的安全隐患,以及版权信息的笔误。其次,建议将 CMake 中的硬编码绝对路径改为通过 PkgConfig 查找,并清理测试代码中的开发者个人硬编码路径。

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 3, 2026

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