-
Notifications
You must be signed in to change notification settings - Fork 55
Revert "fix: 主要是处理打包单元测试报错问题。" #1368
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This reverts commit 774c00e.
Reviewer's guide (collapsed on small PRs)Reviewer's GuideReverts a previous packaging change by restoring debian/rules to its earlier state, effectively undoing prior modifications related to unit test build error handling. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
CI构建没过( |
1. Added rolecombinemodel.h and rolecombinemodel.cpp source files to
rolecombinemodel_tests target
2. Added rolegroupmodel.h and rolegroupmodel.cpp source files to
rolegroupmodel_tests target
3. Added Qt${QT_VERSION_MAJOR}::Gui dependency to both test targets
4. Removed dock-taskmanager library dependency from
rolecombinemodel_tests
5. Commented out dock-taskmanager library dependency from
rolegroupmodel_tests
These changes fix the test build configuration by including the actual
source files directly in the test targets instead of linking against the
dock-taskmanager library. This ensures the tests can properly compile
and link the rolecombinemodel and rolegroupmodel implementations. The Qt
Gui module was added as a dependency since these models likely require
GUI components.
Influence:
1. Verify that rolecombinemodel_tests and rolegroupmodel_tests can be
built successfully
2. Test that all unit tests pass after the build configuration changes
3. Ensure no linking errors occur due to missing dependencies
4. Confirm that the test executables run without crashes
fix: 更新角色模型的测试CMakeLists配置
1. 将rolecombinemodel.h和rolecombinemodel.cpp源文件添加到
rolecombinemodel_tests目标
2. 将rolegroupmodel.h和rolegroupmodel.cpp源文件添加到
rolegroupmodel_tests目标
3. 为两个测试目标添加Qt${QT_VERSION_MAJOR}::Gui依赖
4. 从rolecombinemodel_tests中移除dock-taskmanager库依赖
5. 从rolegroupmodel_tests中注释掉dock-taskmanager库依赖
这些更改修复了测试构建配置,通过直接将实际源文件包含在测试目标中,而不
是链接到dock-taskmanager库。这确保测试能够正确编译和链接rolecombinemodel
和rolegroupmodel的实现。添加了Qt Gui模块依赖,因为这些模型可能需要GUI
组件。
Influence:
1. 验证rolecombinemodel_tests和rolegroupmodel_tests能够成功构建
2. 测试构建配置更改后所有单元测试是否通过
3. 确保不会因缺少依赖而发生链接错误
4. 确认测试可执行文件运行不会崩溃
deepin pr auto review我来对这个diff进行详细审查:
-#DEB_CMAKE_EXTRA_FLAGS += -DCMAKE_SKIP_BUILD_RPATH=ON
+DEB_CMAKE_EXTRA_FLAGS += -DCMAKE_SKIP_BUILD_RPATH=ON这个修改启用了 CMAKE_SKIP_BUILD_RPATH 选项。从安全角度来看,这是一个好的改动:
对于 rolecombinemodel_tests: + ${CMAKE_SOURCE_DIR}/panels/dock/taskmanager/rolecombinemodel.h
+ ${CMAKE_SOURCE_DIR}/panels/dock/taskmanager/rolecombinemodel.cpp
- dock-taskmanager
+ Qt${QT_VERSION_MAJOR}::Gui对于 rolegroupmodel_tests: + ${CMAKE_SOURCE_DIR}/panels/dock/taskmanager/rolegroupmodel.h
+ ${CMAKE_SOURCE_DIR}/panels/dock/taskmanager/rolegroupmodel.cpp
- dock-taskmanager
+ Qt${QT_VERSION_MAJOR}::Gui代码质量方面的问题和建议:
# 建议的改进方案
add_library(test_common STATIC
${CMAKE_SOURCE_DIR}/panels/dock/taskmanager/rolecombinemodel.cpp
${CMAKE_SOURCE_DIR}/panels/dock/taskmanager/rolegroupmodel.cpp
)
target_link_libraries(test_common
Qt${QT_VERSION_MAJOR}::Core
Qt${QT_VERSION_MAJOR}::Gui
)
add_executable(rolecombinemodel_tests
combinemodela.cpp
combinemodela.h
# ... 其他测试文件
)
target_link_libraries(rolecombinemodel_tests
test_common
GTest::GTest
GTest::Main
Qt${QT_VERSION_MAJOR}::Test
)这样的改进可以:
总的来说,这个diff的主要目的是改进测试的构建方式,虽然当前实现方式可以工作,但还有改进空间。建议考虑重构测试构建系统,使其更加模块化和可维护。 |
再看看,之前去掉rpath,test没有通过是因为它link了taskmanager这个so,它不在默认路径下, |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, BLumia 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 |
This reverts commit 774c00e.
Summary by Sourcery
Revert previous changes to the Debian packaging rules related to unit test build handling.