Skip to content

test: migrate unit tests from Qt5 to Qt6 compatibility#623

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
pengfeixx:feat/migrate-tests-qt6
Jun 4, 2026
Merged

test: migrate unit tests from Qt5 to Qt6 compatibility#623
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
pengfeixx:feat/migrate-tests-qt6

Conversation

@pengfeixx

Copy link
Copy Markdown
Contributor

Fix QMouseEvent 4-arg constructors removed in Qt6, use 6-arg form with QPointF. Add Qt6 runJavaScript stubs with correct signature. Fix sortAppList iterator out-of-bounds causing heap-use-after-free. Fix fileWatcher test hanging due to unstubbed constructor I/O. Fix test scripts to use absolute paths for portability.

修复QMouseEvent在Qt6下移除4参数构造函数的问题,改用6参数形式。
修复sortAppList迭代器越界导致的use-after-free内存错误。
修复fileWatcher测试因构造函数未stub导致的挂起问题。
修复测试脚本路径问题,使用绝对路径提升可移植性。

Log: 单元测试Qt5迁移至Qt6兼容
Influence: 单元测试可在Qt6环境下编译运行,已验证60+测试通过

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

Please try again later or upgrade to continue using Sourcery

Fix QMouseEvent 4-arg constructors removed in Qt6, use 6-arg form
with QPointF. Add Qt6 runJavaScript stubs with correct signature.
Fix sortAppList iterator out-of-bounds causing heap-use-after-free.
Fix fileWatcher test hanging due to unstubbed constructor I/O.
Fix test scripts to use absolute paths for portability.

修复QMouseEvent在Qt6下移除4参数构造函数的问题,改用6参数形式。
修复sortAppList迭代器越界导致的use-after-free内存错误。
修复fileWatcher测试因构造函数未stub导致的挂起问题。
修复测试脚本路径问题,使用绝对路径提升可移植性。

Log: 单元测试Qt5迁移至Qt6兼容
Influence: 单元测试可在Qt6环境下编译运行,已验证60+测试通过
@pengfeixx pengfeixx force-pushed the feat/migrate-tests-qt6 branch from d91e3f1 to d328b00 Compare June 4, 2026 07:24
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeex。我已经仔细审查了你提供的 git diff,本次修改主要集中在修复迭代器失效导致的未定义行为适配 Qt6 的 API 变更增强 Shell 脚本的健壮性以及完善单元测试的隔离性

整体来看,这是一次质量很高的提交,修复了多个潜在的严重问题。以下是针对语法逻辑、代码质量、代码性能和代码安全四个方面的详细审查意见和改进建议:


一、 语法与逻辑

1. 修复了严重的迭代器失效问题(utils.cpp

  • 原代码:在 ++it; 之后调用 it.value(),由于 it 已经移动到了下一个元素,如果此时 it 到达了 end(),调用 it.value() 将触发未定义行为(UB),极可能导致程序崩溃。
  • 修改后:先输出 it.value(),再执行 ++it;,逻辑完全正确,修复了崩溃隐患。同时删除了循环末尾越界访问的日志,非常准确。
  • 建议:无,这部分修改非常完美。

2. Qt6 的 API 适配(ut_web_window.cpp 等)

  • 修改点
    • QMouseEvent 构造函数由 QPoint 改为 QPointF,适配了 Qt6 的高 DPI 缩放要求。
    • QWebEnginePage::runJavaScript 在 Qt6 中增加了重载,增加了 quint32 worldIdstd::function<void(const QVariant &)> 参数,修改后通过宏分别 Stub 不同版本的 API。
  • 逻辑评价:逻辑正确,解决了 Qt6 下的编译和运行时兼容性问题。
  • 改进建议:在 ut_helpermanager.cpp 中,handleDb 测试用例被 #if (QT_VERSION < QT_VERSION_CHECK(6, 0, 0)) 整体屏蔽,并注释了 TODO。建议在后续迭代中尽快重构该 Stub 策略,以保证 Qt6 下的核心逻辑测试覆盖率。

3. Stub 函数返回值类型修正(ut_filewatcher.h

  • 修改点stub_getSystemManualDir 返回值由 QString 改为 QStringList,并在实现中返回 QStringList() << "./manual-assets"
  • 逻辑评价:与被 Stub 的真实函数 Utils::getSystemManualDir 签名保持一致,避免了因签名不匹配导致的 Stub 失效或调用栈破坏,逻辑正确。

二、 代码质量

1. Shell 脚本的健壮性大幅提升(cmake-lcov-test.sh, test-prj-running.sh

  • 修改点
    • 使用 SCRIPT_DIRPROJECT_ROOT 替代了 ../ 这种依赖于执行路径的相对定位方式。
    • 变量引用加上了双引号(如 "$PROJECT_ROOT/$utdir"),防止路径含空格时出现词法分割错误。
    • rm -r 改为 rm -rfmkdir 改为 mkdir -p,避免首次运行时因目录不存在而报错中断。
    • 并行编译从硬编码 -j16 / -j8 改为 -j$(nproc),自适应机器核心数。
  • 评价:极大地提升了脚本的可移植性和鲁棒性,是非常好的编程习惯。

2. Stub 代码的内存管理(ut_filewatcher.cpp

  • 修改点:在 SetUpnew Stub(),在 TearDowndelete m_stub;
  • 改进建议:虽然手动管理内存在 C++ 中是可行的,但更现代和安全的做法是使用智能指针,防止 SetUpTearDown 之间测试用例抛出异常导致内存泄漏:
    // 在头文件中改为:
    std::unique_ptr<Stub> m_stub;
    
    // 在 cpp 中:
    void ut_fileWatcher::SetUp() {
        m_stub = std::make_unique<Stub>();
        m_stub->set(...);
    }
    // TearDown 中无需手动 delete,智能指针自动释放。

3. 重复的 Qt 版本判断代码(ut_web_window.cpp

  • 现象:关于 QWebEnginePage::runJavaScript 的 Qt6 适配代码,在文件中重复出现了 11 次
  • 改进建议:这严重违反了 DRY(Don't Repeat Yourself)原则。建议封装为一个宏或辅助函数,减少冗余,并方便未来 Qt7 或 API 再次变更时统一修改:
    // 在头文件或 cpp 顶部定义宏
    #if (QT_VERSION < QT_VERSION_CHECK(6, 0, 0))
    #define STUB_RUNJAVASCRIPT(stub_obj, addr) \
        stub_obj.set((void (QWebEnginePage::*)(const QString&))ADDR(QWebEnginePage, runJavaScript), addr)
    #else
    #define STUB_RUNJAVASCRIPT(stub_obj, addr) \
        stub_obj.set((void (QWebEnginePage::*)(const QString&, quint32, const std::function<void(const QVariant &)> &))ADDR(QWebEnginePage, runJavaScript), addr)
    #endif
    
    // 使用时:
    STUB_RUNJAVASCRIPT(s, ADDR(ut_web_window_test, stub_initweb));

三、 代码性能

1. 函数参数按值传递(utils.cpp

  • 现象QList<AppInfo> Utils::sortAppList(QMultiMap<qlonglong, AppInfo> map) 中的 map 参数是按值传递的。
  • 改进建议QMultiMap 内部是隐式共享的,按值传递虽然不会引起深拷贝(COW 机制),但在函数调用时会增加原子引用计数的开销。更重要的是,如果函数内部对 map 进行了修改(遍历本身不修改,但如果有写操作),则会触发深拷贝。建议改为 const 引用传递:
    QList<AppInfo> Utils::sortAppList(const QMultiMap<qlonglong, AppInfo>& map)
    (注意:改为 const 引用后,需要确保迭代器使用的是 QMultiMap::const_iterator 而非 QMultiMap::iterator)

2. 编译并发数 make -j$(nproc)

  • 评价:使用 nproc 获取逻辑核心数是一个好习惯。但在内存较小的机器上(如 4G 内存的虚拟机),nproc 可能导致内存耗尽编译 OOM。如果需要更稳妥,可以使用 $(nproc) 减去 1,或者保持现状(大多数现代机器内存充足,保持现状即可)。

四、 代码安全

1. Shell 脚本中的 rm -rftest-prj-running.sh

  • 现象rm -rf "$PROJECT_ROOT/$builddir"rm -rf "$PROJECT_ROOT/$reportdir"
  • 评价:由于前面通过 SCRIPT_DIRPROJECT_ROOT 对路径进行了绝对定位,且变量加了引号,基本杜绝了 rm -rf / 或路径含空格被截断的惨剧,安全性得到了保障。
  • 潜在风险:如果 PROJECT_ROOT 变量因某种原因为空,rm -rf "/$builddir" 仍然会从根目录开始删除。
  • 改进建议:增加防御性判断,确保关键变量不为空:
    if [ -z "$PROJECT_ROOT" ]; then
        echo "Error: PROJECT_ROOT is empty!"
        exit 1
    fi

2. ASAN 日志拷贝(test-prj-running.sh

  • 修改点cp -r asan*.log* ../$reportdir/asan_deepin-manual.log 改为 cp -r asan*.log* "$PROJECT_ROOT/$reportdir/asan_deepin-manual.log" 2>/dev/null || true
  • 评价:这个修改非常好。当没有 ASAN 日志时,原代码会报错并中断脚本执行(由于脚本可能没有 set -e 但依然不美观),修改后通过 2>/dev/null || true 忽略了无日志时的错误,使得 CI 流水线不会因为不存在 ASAN 错误而失败,安全且符合预期。

总结

本次提交质量很高,修复了迭代器越界等严重 Bug,解决了 Qt6 兼容性问题,并对 Shell 脚本进行了规范化重构。建议采纳上述关于使用智能指针管理 Stub封装 Qt 版本判断宏以及sortAppList 使用 const 引用传参的改进意见,这将使代码更加健壮和易于维护。

@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 930691a into linuxdeepin:master Jun 4, 2026
16 checks passed
@pengfeixx pengfeixx deleted the feat/migrate-tests-qt6 branch June 4, 2026 07:28
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