Repository navigation
test: 补充 dde-launchpad 单元测试 - #813
Merged
deepin-bot[bot] merged 1 commit intoOct 23, 2026
Merged
Conversation
|
Hi @MyLeeJiEun. Thanks for your PR. I'm waiting for a linuxdeepin member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Reviewer's GuideThis PR substantially expands dde-launchpad’s Qt6 unit-test coverage across core models and utilities, integrates 15 CTest targets through the BUILD_TESTING option, and instruments test-linked object libraries to support an aggregate lcov coverage workflow. Flow diagram for Qt6 test and coverage executionflowchart LR
Configure["cmake -B build -DBUILD_TESTING=ON"] --> Build["cmake --build build"]
Build --> CTest["ctest --test-dir build --output-on-failure"]
CTest --> Coverage["lcov filtered coverage report"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="CMakeLists.txt" line_range="9" />
<code_context>
project(dde-launchpad VERSION 0.7.0)
-option(BUILD_TEST "Whether or not to build the tests" OFF)
+option(BUILD_TESTING "Whether or not to build the tests" OFF)
option(CMAKE_EXPORT_COMPILE_COMMANDS "clangd support" ON)
</code_context>
<issue_to_address>
**issue (broader_impact):** Existing invocations that configure the project with `-DBUILD_TEST=ON` no longer build or register any tests because the old option is silently ignored and the new condition checks only `BUILD_TESTING`. This contradicts the documented verification command in the change description and breaks downstream CI or packaging scripts that still use the old option.
**Triggers:** When a caller continues to pass the previous `BUILD_TEST` option.
**Suggested fix:** Preserve a compatibility alias from `BUILD_TEST` to `BUILD_TESTING`, or update every supported build invocation and fail clearly when the deprecated option is used.
```suggestion
option(BUILD_TESTING "Whether or not to build the tests" OFF)
option(BUILD_TEST "Deprecated alias for BUILD_TESTING" OFF)
if(BUILD_TEST)
set(BUILD_TESTING ON)
endif()
```
</issue_to_address>
### Comment 2
<location path="tests/CMakeLists.txt" line_range="123-143" />
<code_context>
+find_program(GENHTML_BIN genhtml)
+set(_run_test_coverage "")
+if(LCOV_BIN AND GENHTML_BIN)
+ set(_run_test_coverage
+ COMMAND ${LCOV_BIN} --capture --directory ${CMAKE_BINARY_DIR}
+ --rc lcov_branch_coverage=1 --output-file ${CMAKE_BINARY_DIR}/coverage.info
+ COMMAND ${LCOV_BIN} --extract ${CMAKE_BINARY_DIR}/coverage.info
+ --rc lcov_branch_coverage=1
+ "${CMAKE_SOURCE_DIR}/src/models/*"
+ "${CMAKE_SOURCE_DIR}/src/utils/*"
+ "${CMAKE_SOURCE_DIR}/src/gioutils/*"
+ --output-file ${CMAKE_BINARY_DIR}/coverage_filtered.info
+ COMMAND ${GENHTML_BIN} ${CMAKE_BINARY_DIR}/coverage_filtered.info
+ --branch-coverage --output-directory ${CMAKE_BINARY_DIR}/coverage_report
+ )
+endif()
+add_custom_target(coverage
</code_context>
<issue_to_address>
**issue (bug_risk):** The `coverage` target succeeds after running the tests without generating any report when either `lcov` or `genhtml` is unavailable, because `_run_test_coverage` is left empty and no failure or warning is emitted. Consumers therefore receive a green coverage build with the promised `coverage_report/index.html` missing.
**Triggers:** When lcov or genhtml is not installed on the build host.
**Suggested fix:** Make the coverage target fail with a clear dependency error, or explicitly report that coverage generation was skipped instead of presenting the target as successful.
</issue_to_address>
Comment on lines
+123
to
143
| set(_run_test_coverage | ||
| COMMAND ${LCOV_BIN} --capture --directory ${CMAKE_BINARY_DIR} | ||
| --rc lcov_branch_coverage=1 --output-file ${CMAKE_BINARY_DIR}/coverage.info | ||
| COMMAND ${LCOV_BIN} --extract ${CMAKE_BINARY_DIR}/coverage.info | ||
| --rc lcov_branch_coverage=1 | ||
| "${CMAKE_SOURCE_DIR}/src/models/*" | ||
| "${CMAKE_SOURCE_DIR}/src/utils/*" | ||
| "${CMAKE_SOURCE_DIR}/src/gioutils/*" | ||
| --output-file ${CMAKE_BINARY_DIR}/coverage_filtered.info | ||
| COMMAND ${GENHTML_BIN} ${CMAKE_BINARY_DIR}/coverage_filtered.info | ||
| --branch-coverage --output-directory ${CMAKE_BINARY_DIR}/coverage_report | ||
| ) | ||
| endif() | ||
| add_custom_target(coverage | ||
| COMMAND ${CMAKE_CTEST_COMMAND} --output-on-failure | ||
| ${_run_test_coverage} | ||
| DEPENDS ${LAUNCHPAD_TEST_TARGETS} | ||
| WORKING_DIRECTORY ${CMAKE_BINARY_DIR} | ||
| COMMENT "Run all tests and generate coverage report" | ||
| VERBATIM | ||
| ) |
There was a problem hiding this comment.
issue (bug_risk): The coverage target succeeds after running the tests without generating any report when either lcov or genhtml is unavailable, because _run_test_coverage is left empty and no failure or warning is emitted. Consumers therefore receive a green coverage build with the promised coverage_report/index.html missing.
Triggers: When lcov or genhtml is not installed on the build host.
Suggested fix: Make the coverage target fail with a clear dependency error, or explicitly report that coverage generation was skipped instead of presenting the target as successful.
MyLeeJiEun
force-pushed
the
agent/dde/launchpad-tests
branch
from
September 15, 2026 08:14
ddcced3 to
43f1b2a
Compare
BLumia
reviewed
Sep 17, 2026
MyLeeJiEun
force-pushed
the
agent/dde/launchpad-tests
branch
from
September 17, 2026 03:43
43f1b2a to
738b51c
Compare
18202781743
reviewed
Sep 17, 2026
MyLeeJiEun
force-pushed
the
agent/dde/launchpad-tests
branch
from
September 17, 2026 05:24
738b51c to
9f56913
Compare
Contributor
|
ci没过， |
MyLeeJiEun
force-pushed
the
agent/dde/launchpad-tests
branch
from
September 17, 2026 07:00
9f56913 to
14f5896
Compare
Contributor
Author
|
MyLeeJiEun
force-pushed
the
agent/dde/launchpad-tests
branch
from
September 21, 2026 02:16
14f5896 to
0a3396f
Compare
|
TAG Bot New tag: 2.0.48 |
|
TAG Bot New tag: 2.0.49 |
|
TAG Bot New tag: 2.0.50 |
|
TAG Bot New tag: 2.0.51 |
MyLeeJiEun
added a commit
to MyLeeJiEun/dde-launchpad
that referenced
this pull request
Oct 22, 2026
- Rebase PR linuxdeepin#813 onto current upstream/master - Keep upstream's Qt DBus component in root CMakeLists.txt - Align test desktopId assertions with upstream's normalizedDesktopId semantics (pages store suffixed ids, AppsModel strips the suffix) Co-Authored-By: Claude Code <noreply@anthropic.com>
MyLeeJiEun
force-pushed
the
agent/dde/launchpad-tests
branch
from
October 22, 2026 06:40
0a3396f to
15b3e5b
Compare
MyLeeJiEun
added a commit
to MyLeeJiEun/dde-launchpad
that referenced
this pull request
Oct 22, 2026
- Rebase PR linuxdeepin#813 onto current upstream/master - Keep upstream's Qt DBus component in root CMakeLists.txt - Align test desktopId assertions with upstream's normalizedDesktopId semantics (pages store suffixed ids, AppsModel strips the suffix) Co-Authored-By: Claude Code <noreply@anthropic.com>
MyLeeJiEun
force-pushed
the
agent/dde/launchpad-tests
branch
from
October 22, 2026 06:54
15b3e5b to
3bf6fd4
Compare
MyLeeJiEun
added a commit
to MyLeeJiEun/dde-launchpad
that referenced
this pull request
Oct 22, 2026
- Rebase PR linuxdeepin#813 onto current upstream/master - Keep upstream's Qt DBus component in root CMakeLists.txt - Align test desktopId assertions with upstream's normalizedDesktopId semantics (pages store suffixed ids, AppsModel strips the suffix)
MyLeeJiEun
force-pushed
the
agent/dde/launchpad-tests
branch
from
October 22, 2026 06:58
3bf6fd4 to
1599279
Compare
MyLeeJiEun
added a commit
to MyLeeJiEun/dde-launchpad
that referenced
this pull request
Oct 22, 2026
- Rebase PR linuxdeepin#813 onto current upstream/master - Keep upstream's Qt DBus component in root CMakeLists.txt - Align test desktopId assertions with upstream's normalizedDesktopId semantics (pages store suffixed ids, AppsModel strips the suffix)
MyLeeJiEun
force-pushed
the
agent/dde/launchpad-tests
branch
from
October 22, 2026 06:59
1599279 to
8135ab3
Compare
BLumia
approved these changes
Oct 23, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: BLumia, MyLeeJiEun 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 |
- Rebase PR linuxdeepin#813 onto current upstream/master - Keep upstream's Qt DBus component in root CMakeLists.txt - Align test desktopId assertions with upstream's normalizedDesktopId semantics (pages store suffixed ids, AppsModel strips the suffix)
MyLeeJiEun
force-pushed
the
agent/dde/launchpad-tests
branch
from
October 23, 2026 08:34
8135ab3 to
70cffb7
Compare
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价： 语法正确，逻辑清晰 ✅ 通过 潜在问题： 建议： ['无需修改，语法逻辑完全正确'] 2. 代码质量 ✅评价： 代码结构清晰，注释完整 ✅ 通过 潜在问题： 建议： ['删除空 namespace {} 块以保持代码整洁', '可考虑将重复的 SourceRoles 枚举提取到 tests/ 下的公共头文件中，减少维护成本'] 3. 代码性能 ✅评价： 性能良好，资源使用合理 ✅ 通过 潜在问题： 建议： ['无需修改，性能表现合理'] 4. 代码安全 🔒评价： 存在0个安全漏洞 ✅ 通过
安全漏洞详情： 建议： ['无需修改，安全合规'] 💡 改进建议代码示例// 建议删除空 namespace 块，例如在 countlimitproxymodeltest.cpp 中：
// 删除以下空命名空间：
// namespace {
// }
//
// 如果未来 SourceRoles 重复变得难以维护，可创建 tests/common/sourceroles.h：
//
// #pragma once
// #include <QtUserRole>
//
// enum SourceRoles {
// SrcDesktopIdRole = Qt::UserRole + 1,
// SrcNameRole,
// SrcIconNameRole,
// SrcNoDisplayRole,
// SrcDDECategoryRole,
// SrcInstalledTimeRole,
// SrcLastLaunchedTimeRole,
// SrcLaunchedTimesRole,
// SrcAutoStartRole,
// SrcCategoriesRole,
// SrcVendorRole,
// SrcGenericNameRole,
// };本报告由 AI 代码审查工具自动生成 |
Contributor
Author
|
/forcemerge |
|
This pr force merged! (status: blocked) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
补充 dde-launchpad 单元测试
关联 Multica 任务: DDE-109 (补充 dde-launchpad 项目测试代码)
变更内容
src/models和src/utils核心模块itemspage和searchfilter代理模型测试tests/CMakeLists.txt，注册全部 15 个测试 target 并启用 gcov 覆盖率编译选项测试与覆盖率
验证方式
Summary by Sourcery
Expand dde-launchpad’s Qt unit-test suite and integrate automated coverage reporting for core models and utilities.
Enhancements:
Build:
Tests:
Chores: