Repository navigation
test(ut): add PMS regression unit tests for severity 1/2 bug hotspots - #628
Conversation
Reviewer's GuideThis test-only PR adds 88 PMS-traceable GTest regressions for severity 1/2 hotspot behavior, emphasizing lifecycle safety, persistence, editor interactions, undo/redo, large-file/error paths, and UI state consistency; it also ignores generated test data and reports a full 22-target pass. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning
详情 {
"unset": {
"tests/editor_core/test_dtextedit_misc.cpp": {
"b": [
"// \u573a\u666f\uff1a\u5207\u6362\u6df1\u6d45\u4e3b\u9898\u6587\u5b57\u989c\u8272\uff0865bc75a3\uff09\u5173\u8054\u884c\uff1apasteText \u539f\u751f\u7c98\u8d34\u540e unsetMark\uff0c",
" // Assert: \u526a\u8d34\u677f\u5185\u5bb9\u63d2\u5165\u3001\u6807\u8bb0\u590d\u4f4d\uff08pasteText \u672b\u5c3e unsetMark\uff09"
]
}
}
} |
![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="tests/editor_core/test_dtextedit_cursor.cpp" line_range="898-905" />
<code_context>
+ ASSERT_GT(bar->maximum(), 0);
+ bar->setValue(bar->maximum() / 2);
+
+ // Act: 大文本滚动状态下获取首可见块
+ const int blockId = edit->getFirstVisibleBlockId();
+ const QTextBlock block = edit->firstVisibleBlock();
+
+ // Assert: 返回合法且非首块的可见块（视图已滚动），firstVisibleBlock 同步
+ ASSERT_TRUE(block.isValid());
+ EXPECT_GT(blockId, 0);
+ EXPECT_GT(block.blockNumber(), 0);
+
+ // Act: jumpToLine 跳转中部行（keepLineAtCenter=false）
</code_context>
<issue_to_address>
**issue (testing):** The test expects `getFirstVisibleBlockId()` and `firstVisibleBlock()` to report a nonzero block after scrolling, but the current implementation calculates the scroll position with integer division (`height() / maximum()` or `value() / maximum()`), producing a zero-point for the midpoint in the tested range; the assertions therefore fail instead of providing a passing regression test.
**Triggers:** When the vertical scrollbar maximum is greater than one and the document is scrolled to `maximum() / 2`.
**Suggested fix:** Either fix the production calculation to use floating-point division, or change this test to assert the behavior currently intended by the implementation and separately add a regression for the integer-division defect.
</issue_to_address>
### Comment 2
<location path="tests/common2/test_fileloadthread.cpp" line_range="456-460" />
<code_context>
+ EXPECT_TRUE(spy.at(0).at(2).toBool());
+ EXPECT_FALSE(spy.at(0).at(3).toBool());
+ }
+ // 当前代码：catch 分支提前 return（:88），deleteLater 仅在正常路径末尾（:132）注册，
+ // 故此处 guard 未清空属预期；手动补 deleteLater 防泄漏后断言回收
+ t->deleteLater();
+ QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete);
+ EXPECT_TRUE(guard.isNull());
+}
</code_context>
<issue_to_address>
**issue (testing):** The test manually calls `deleteLater()` after the `bad_alloc` path and then asserts destruction, so it masks the production lifecycle behavior: a regression where `run()` emits the error and returns without scheduling cleanup still passes because the test supplies the missing cleanup itself.
**Triggers:** When `FileLoadThread::run()` catches the read allocation failure and returns before its normal-path cleanup.
**Suggested fix:** Assert that the error path itself schedules or performs object cleanup, or use the same ownership mechanism as production and verify the object is destroyed without calling `deleteLater()` from the test.
</issue_to_address>| // Act: 大文本滚动状态下获取首可见块 | ||
| const int blockId = edit->getFirstVisibleBlockId(); | ||
| const QTextBlock block = edit->firstVisibleBlock(); | ||
|
|
||
| // Assert: 返回合法且非首块的可见块（视图已滚动），firstVisibleBlock 同步 | ||
| ASSERT_TRUE(block.isValid()); | ||
| EXPECT_GT(blockId, 0); | ||
| EXPECT_GT(block.blockNumber(), 0); |
There was a problem hiding this comment.
issue (testing): The test expects getFirstVisibleBlockId() and firstVisibleBlock() to report a nonzero block after scrolling, but the current implementation calculates the scroll position with integer division (height() / maximum() or value() / maximum()), producing a zero-point for the midpoint in the tested range; the assertions therefore fail instead of providing a passing regression test.
Triggers: When the vertical scrollbar maximum is greater than one and the document is scrolled to maximum() / 2.
Suggested fix: Either fix the production calculation to use floating-point division, or change this test to assert the behavior currently intended by the implementation and separately add a regression for the integer-division defect.
| // 当前代码：catch 分支提前 return（:88），deleteLater 仅在正常路径末尾（:132）注册， | ||
| // 故此处 guard 未清空属预期；手动补 deleteLater 防泄漏后断言回收 | ||
| t->deleteLater(); | ||
| QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete); | ||
| EXPECT_TRUE(guard.isNull()); |
There was a problem hiding this comment.
issue (testing): The test manually calls deleteLater() after the bad_alloc path and then asserts destruction, so it masks the production lifecycle behavior: a regression where run() emits the error and returns without scheduling cleanup still passes because the test supplies the missing cleanup itself.
Triggers: When FileLoadThread::run() catches the read allocation failure and returns before its normal-path cleanup.
Suggested fix: Assert that the error path itself schedules or performs object cleanup, or use the same ownership mechanism as production and verify the object is destroyed without calling deleteLater() from the test.
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： [] 2. 代码质量 ✅评价： 优秀 ✅ 通过 潜在问题：
建议： ['建议在 test_insertblockbytextcommand.cpp 中使用 EditWrapper 的真实构造或 Mock 对象替代 reinterpret_cast，避免潜在的未定义行为。'] 3. 代码性能 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： [] 4. 代码安全 🔒评价： 优秀 ✅ 通过
安全漏洞详情： 建议： [] 💡 改进建议代码示例// 暂无代码示例本报告由 AI 代码审查工具自动生成 |
Add 88 GTest regression cases for PMS defect hotspots across editor core/wrapper/undo/widgets/settings/common modules, with work order and defect analysis under tests/.ut-pms/ (not committed). 新增88个GTest回归用例，覆盖编辑器核心、撤销命令、控件、设置、 公共模块的PMS缺陷热点；用例工作单与缺陷分析存于tests/.ut-pms/。 Log: 补充PMS缺陷热点回归单元测试 Influence: 仅测试文件变更，不影响应用功能；bug 回归用例覆盖提升。
6d08e6b to
e9042ea
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: add-uos 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 |
1 similar comment
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: add-uos 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 |
|
Warning
详情 {
"unset": {
"tests/editor_core/test_dtextedit_misc.cpp": {
"b": [
"// \u573a\u666f\uff1a\u5207\u6362\u6df1\u6d45\u4e3b\u9898\u6587\u5b57\u989c\u8272\uff0865bc75a3\uff09\u5173\u8054\u884c\uff1apasteText \u539f\u751f\u7c98\u8d34\u540e unsetMark\uff0c",
" // Assert: \u526a\u8d34\u677f\u5185\u5bb9\u63d2\u5165\u3001\u6807\u8bb0\u590d\u4f4d\uff08pasteText \u672b\u5c3e unsetMark\uff09"
]
}
}
} |
|
/forcemerge |
|
This pr force merged! (status: blocked) |
3 participants
概述 / Summary
补充 PMS 缺陷热点（severity 1/2）回归单元测试，共 88 个 GTest 用例，仅涉及
tests/目录，不改任何源码。内容 / Contents
说明 / Notes
// PMS:注释（bug-view 链接 + 修复 commit sha），便于回溯TEST_F(<Fixture>, BUG<id>_<场景>)，Arrange/Act/Assert 三段式验证 / Verification
Summary by Sourcery
Expand unit-test coverage for PMS severity 1/2 regression hotspots without modifying production code.
Enhancements:
Tests:
Chores: