fix(tests): migrate unit tests to Qt6 and fix DEADLYSIGNAL crash#479
Conversation
Migrate test build system from Qt5 to Qt6 (dtk6, Qt6 modules), fix cmake source path, test data paths in run script, and move UNITTEST guard before workspaceWindows() to prevent ASan crash caused by DForeignWindow::fromWinId() in worker thread. 将单元测试构建系统从 Qt5 迁移至 Qt6(dtk6、Qt6 模块), 修复运行脚本中 cmake 源码路径、测试数据路径问题, 并将 UNITTEST 保护移至 workspaceWindows() 之前, 防止工作线程中 DForeignWindow::fromWinId() 导致 ASan 崩溃。 Log: 修复单元测试构建和运行崩溃问题 Influence: 修复后全部 101 个单元测试用例可正常构建并通过, 覆盖 Qt5→Qt6 迁移、cmake 路径、脚本路径、线程安全等修复。
There was a problem hiding this comment.
Sorry @pengfeixx, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
deepin pr auto review这是一次将项目从 Qt5 迁移到 Qt6/DTK6 并对单元测试进行适配的代码审查。整体来看,迁移方向正确,解决了很多 Qt6 中的 Breaking Changes(如 QCamera、QWheelEvent、高 DPI 属性等),并且对测试脚本的健壮性做了一定提升。 以下是针对语法逻辑、代码质量、代码性能和代码安全方面的详细审查意见和改进建议: 1. 语法与逻辑1.1 CMake 版本与 Project 声明顺序文件: 1.2 CMake 文件遍历与排除逻辑的脆弱性文件: file(GLOB_RECURSE SRC_CXXSOURCES ../src/src/*.cpp)
foreach(src_file ${SRC_CXXSOURCES})
get_filename_component(src_name ${src_file} NAME)
if(NOT src_name STREQUAL "videosurface.cpp")
list(APPEND CXXSOURCES ${src_file})
endif()
endforeach()这种做法在逻辑上没问题,但如果未来需要排除更多文件(Qt6 中可能还有其他不兼容的文件), file(GLOB_RECURSE SRC_CXXSOURCES ../src/src/*.cpp)
list(FILTER SRC_CXXSOURCES EXCLUDE REGEX "videosurface\\.cpp$")
list(APPEND CXXSOURCES ${SRC_CXXSOURCES})1.3 DTK 头文件包含路径硬编码文件: #include <dtk6/DWidget/DApplication>
#include <dtk6/DCore/DLog>这破坏了 DTK 原本设计的通过 #include <DApplication>
#include <DLog>如果是因为 DTK5 和 DTK6 头文件冲突被迫硬编码,建议在 CMake 中通过 1.4 拼写错误修正文件: 1.5 QWheelEvent 构造函数适配文件: QWheelEvent wheelEvent(QPointF(p), QPointF(slider->mapToGlobal(p)), QPoint(0, 40), QPoint(0, 40), Qt::NoButton, Qt::NoModifier, Qt::ScrollBegin, false);建议: 逻辑正确,但注意 2. 代码质量2.1 宏定义控制单元测试逻辑(代码坏味道)文件: while (!isInterruptionRequested()) {
#ifdef UNITTEST
break;
#endif
auto list = workspaceWindows();
// ...这违反了“生产代码不应感知测试代码”的原则。如果
2.2 Qt 版本兼容宏的统一管理文件: 多个测试文件 3. 代码性能3.1 CMake 中链接库的重复与冗余文件: target_link_libraries(${TARGET_NAME} PRIVATE
${3rd_lib_LIBRARIES}
gtest pthread Qt6::Test dl va va-x11 imagevisualresult6
)
target_link_libraries(${TARGET_NAME} PRIVATE
Qt6::Core Qt6::Gui Qt6::Widgets Qt6::DBus Qt6::Concurrent
Qt6::Multimedia Qt6::PrintSupport Qt6::Svg Qt6::SvgWidgets Qt6::OpenGLWidgets
)
target_link_libraries(${TARGET_NAME} PRIVATE
dtk6core dtk6gui dtk6widget
)建议:
4. 代码安全4.1 Shell 脚本中的命令执行安全文件: cp -r /data/source/deepin-camera/jpegtest/*.jpg ~/Pictures/相机/ 2>/dev/null || true建议:
if [ -d "/data/source/deepin-camera/jpegtest" ]; then
mkdir -p ~/Pictures/相机/ ~/Pictures/Camera/
cp -r /data/source/deepin-camera/jpegtest/*.jpg ~/Pictures/相机/ || echo "Warning: Failed to copy jpg to ~/Pictures/相机/"
fi4.2 CMake 中的硬编码系统路径文件: set(PROJECT_INCLUDE
...
/usr/include/libusb-1.0
...
)硬编码 pkg_check_modules(LIBUSB REQUIRED libusb-1.0)
target_include_directories(${TARGET_NAME} PRIVATE ${LIBUSB_INCLUDE_DIRS})
target_link_libraries(${TARGET_NAME} PRIVATE ${LIBUSB_LIBRARIES})总结本次 Diff 的核心目标(Qt6 迁移)已基本达成,对 Qt6 API 变更的修复是正确的。最需要关注的是 DTK 头文件硬编码引入的移植性风险 和 CMake 中硬编码系统路径 的问题,建议优先修复。生产代码中的 |
|
Note |
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/merge |
Migrate test build system from Qt5 to Qt6 (dtk6, Qt6 modules), fix cmake source path, test data paths in run script, and move UNITTEST guard before workspaceWindows() to prevent ASan crash caused by DForeignWindow::fromWinId() in worker thread.
将单元测试构建系统从 Qt5 迁移至 Qt6(dtk6、Qt6 模块),
修复运行脚本中 cmake 源码路径、测试数据路径问题,
并将 UNITTEST 保护移至 workspaceWindows() 之前,
防止工作线程中 DForeignWindow::fromWinId() 导致 ASan 崩溃。
Log: 修复单元测试构建和运行崩溃问题
Influence: 修复后全部 101 个单元测试用例可正常构建并通过,
覆盖 Qt5→Qt6 迁移、cmake 路径、脚本路径、线程安全等修复。