Repository navigation
test: add tests for deepin-reader batch 2 (20 bugs, 38 cases) - #416
Merged
Merged
Conversation
Reviewer's GuideThis test-only batch adds 38 Google Test regressions covering 20 historical sev1/2 bugs across PDF handling, browser interactions, bookmarks, shortcuts, tab management, focus navigation, and application keyboard events. Tests combine real PDF fixtures, mocks/stubs, synthetic Qt events, timing checks, and coverage instrumentation; no production source or build configuration is changed. Review the fixture assumptions, event-state assertions, thread/deadlock test behavior, and ownership/lifetime handling, especially for tests relying on platform-specific paths or sw_64 code that remains untested on x86. 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 5 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="tests/document/ut_pdfmodel.cpp" line_range="904-908" />
<code_context>
+ });
+
+ // 3 秒时限内必须完成(死锁则超时失败)
+ for (int i = 0; i < 300 && !(done1 && done2); ++i)
+ QThread::msleep(10);
+ EXPECT_TRUE(done1.load());
+ EXPECT_TRUE(done2.load());
+ t1.join();
+ t2.join();
+}
</code_context>
<issue_to_address>
**Deadlock test hangs indefinitely**
When either `properties()` worker remains blocked after the polling deadline, `EXPECT_TRUE` records the timeout, but `t1.join()` and `t2.join()` still wait for the blocked workers, so the test process never completes.
Run the deadlock check in a terminable subprocess or use a worker strategy that can be safely stopped after the deadline.
</issue_to_address>
### Comment 2
<location path="tests/document/ut_pdfmodel.cpp" line_range="893-898" />
<code_context>
+{
+ std::atomic<bool> done1(false), done2(false);
+ std::thread t1([this, &done1]() {
+ Properties p = m_tester->properties();
+ Q_UNUSED(p);
+ done1 = true;
</code_context>
<issue_to_address>
**Concurrent test has a data race**
When both worker calls overlap before the property cache is populated, `PDFDocument::properties()` checks and assigns `m_fileProperties` outside the mutex in `DPdfDoc::proeries()`, so concurrent calls access the cache unsynchronized and invoke undefined behavior.
Populate the cache before starting the workers, or otherwise synchronize access to the wrapper cache in this test.
</issue_to_address>
### Comment 3
<location path="tests/browser/ut_browserpage.cpp" line_range="911-912" />
<code_context>
+ QRectF screenRect = m_tester->translateRect(docRect);
+ EXPECT_TRUE(qFuzzyCompare(screenRect.width(), docRect.width() * 1.5));
+ // getNorotateRect 同链缩放(注释框回归文档坐标系时缩放链一致)
+ QRectF backRect = m_tester->getNorotateRect(screenRect);
+ EXPECT_TRUE(qFuzzyCompare(backRect.width(), screenRect.width() * m_tester->m_scaleFactor));
+
+ // 180°: 尺寸不变(仅平移), 缩放链保持
</code_context>
<issue_to_address>
**Coordinate round trip scales twice**
When the inverse coordinate mapping is incorrect, the test describes `getNorotateRect()` as restoring `docRect`, but expects `backRect.width()` to be `screenRect.width() * m_scaleFactor`; an extra scale therefore passes instead of detecting the broken round trip.
Compare the returned rectangle with the original `docRect` rather than expecting another scale multiplication.
</issue_to_address>
### Comment 4
<location path="tests/browser/ut_sheetbrowser.cpp" line_range="2878-2890" />
<code_context>
+ stub.set(ADDR(SheetBrowser, beginViewportChange), beginViewportChange_stub);
+
+ // 大宽度页面: 双页宽度预算未乘 2 时 scale 会反超单页
+ m_tester->m_items.at(0)->m_originSizeF = QSizeF(2000, 1000);
+ m_tester->m_items.at(1)->m_originSizeF = QSizeF(2000, 1000);
+
+ SheetOperation opSingle;
+ opSingle.scaleMode = Dr::FitToPageWidthMode;
+ opSingle.layoutMode = Dr::SinglePageMode;
+ m_tester->deform(opSingle);
+ double scaleSingle = m_tester->m_lastScaleFactor;
+
+ SheetOperation opDouble;
+ opDouble.scaleMode = Dr::FitToPageWidthMode;
+ opDouble.layoutMode = Dr::TwoPagesMode;
+ m_tester->deform(opDouble);
+ double scaleDouble = m_tester->m_lastScaleFactor;
+
</code_context>
<issue_to_address>
**Two-page scale regression passes**
When the tests run with the current fixture setup, `deform()` uses cached `m_maxWidth` and `m_maxHeight`, which these tests do not update after changing `m_originSizeF` (and are zero in the current fixture); the safety fallbacks do not exercise the large-page width budget. Also, `EXPECT_LE` accepts equal scales if the two-page divisor is removed.
Set up valid cached maxima after changing page sizes and assert a scale that distinguishes the two-page width budget from the single-page budget.
Also at `tests/browser/ut_sheetbrowser.cpp:2896`, `tests/browser/ut_sheetbrowser.cpp:2906-2924`.
</issue_to_address>
### Comment 5
<location path="tests/document/ut_pdfmodel.cpp" line_range="865" />
<code_context>
+
+ // 有文字的真实文档: 每个矩形对应一段文本, 数量严格一致
+ EXPECT_GT(charCount, 0);
+ EXPECT_EQ(rects.count(), texts.count());
+ EXPECT_LE(rects.count(), charCount);
+ // 注: DPdfPage 由 DPdfDoc(m_pages) 持有管理, 不可 delete (qDeleteAll 双重释放)
</code_context>
<issue_to_address>
**Empty text results pass alignment check**
When `allTextRects` reports a positive character count but returns empty `rects` and `texts`, `BUG68550_allTextRectsRectTextAligned` allows `rects` and `texts` to both be empty: equal empty lists satisfy the alignment assertion even when the real page has characters, so the test passes if the method returns no selection rectangles or text.
Assert that the output contains at least one rectangle and text entry for this known text-bearing fixture.
</issue_to_address>
Comment on lines
+904
to
+908
| for (int i = 0; i < 300 && !(done1 && done2); ++i) | ||
| QThread::msleep(10); | ||
| EXPECT_TRUE(done1.load()); | ||
| EXPECT_TRUE(done2.load()); | ||
| t1.join(); |
There was a problem hiding this comment.
🟡 Medium · Deadlock test hangs indefinitely
When either properties() worker remains blocked after the polling deadline, EXPECT_TRUE records the timeout, but t1.join() and t2.join() still wait for the blocked workers, so the test process never completes.
Run the deadlock check in a terminable subprocess or use a worker strategy that can be safely stopped after the deadline.
Prompt for AI agents
In `tests/document/ut_pdfmodel.cpp` at lines 904-908:
**Deadlock test hangs indefinitely**
When either `properties()` worker remains blocked after the polling deadline, `EXPECT_TRUE` records the timeout, but `t1.join()` and `t2.join()` still wait for the blocked workers, so the test process never completes.
Run the deadlock check in a terminable subprocess or use a worker strategy that can be safely stopped after the deadline.
Comment on lines
+893
to
+898
| Properties p = m_tester->properties(); | ||
| Q_UNUSED(p); | ||
| done1 = true; | ||
| }); | ||
| std::thread t2([this, &done2]() { | ||
| Properties p = m_tester->properties(); |
There was a problem hiding this comment.
🟡 Medium · Concurrent test has a data race
When both worker calls overlap before the property cache is populated, PDFDocument::properties() checks and assigns m_fileProperties outside the mutex in DPdfDoc::proeries(), so concurrent calls access the cache unsynchronized and invoke undefined behavior.
Populate the cache before starting the workers, or otherwise synchronize access to the wrapper cache in this test.
Prompt for AI agents
In `tests/document/ut_pdfmodel.cpp` at lines 893-898:
**Concurrent test has a data race**
When both worker calls overlap before the property cache is populated, `PDFDocument::properties()` checks and assigns `m_fileProperties` outside the mutex in `DPdfDoc::proeries()`, so concurrent calls access the cache unsynchronized and invoke undefined behavior.
Populate the cache before starting the workers, or otherwise synchronize access to the wrapper cache in this test.
Comment on lines
+911
to
+912
| QRectF backRect = m_tester->getNorotateRect(screenRect); | ||
| EXPECT_TRUE(qFuzzyCompare(backRect.width(), screenRect.width() * m_tester->m_scaleFactor)); |
There was a problem hiding this comment.
🟡 Medium · Coordinate round trip scales twice
When the inverse coordinate mapping is incorrect, the test describes getNorotateRect() as restoring docRect, but expects backRect.width() to be screenRect.width() * m_scaleFactor; an extra scale therefore passes instead of detecting the broken round trip.
Compare the returned rectangle with the original docRect rather than expecting another scale multiplication.
Prompt for AI agents
In `tests/browser/ut_browserpage.cpp` at lines 911-912:
**Coordinate round trip scales twice**
When the inverse coordinate mapping is incorrect, the test describes `getNorotateRect()` as restoring `docRect`, but expects `backRect.width()` to be `screenRect.width() * m_scaleFactor`; an extra scale therefore passes instead of detecting the broken round trip.
Compare the returned rectangle with the original `docRect` rather than expecting another scale multiplication.
Comment on lines
+2878
to
+2890
| m_tester->m_items.at(0)->m_originSizeF = QSizeF(2000, 1000); | ||
| m_tester->m_items.at(1)->m_originSizeF = QSizeF(2000, 1000); | ||
|
|
||
| SheetOperation opSingle; | ||
| opSingle.scaleMode = Dr::FitToPageWidthMode; | ||
| opSingle.layoutMode = Dr::SinglePageMode; | ||
| m_tester->deform(opSingle); | ||
| double scaleSingle = m_tester->m_lastScaleFactor; | ||
|
|
||
| SheetOperation opDouble; | ||
| opDouble.scaleMode = Dr::FitToPageWidthMode; | ||
| opDouble.layoutMode = Dr::TwoPagesMode; | ||
| m_tester->deform(opDouble); |
There was a problem hiding this comment.
🟡 Medium · Two-page scale regression passes
When the tests run with the current fixture setup, deform() uses cached m_maxWidth and m_maxHeight, which these tests do not update after changing m_originSizeF (and are zero in the current fixture); the safety fallbacks do not exercise the large-page width budget. Also, EXPECT_LE accepts equal scales if the two-page divisor is removed.
Set up valid cached maxima after changing page sizes and assert a scale that distinguishes the two-page width budget from the single-page budget.
Also at tests/browser/ut_sheetbrowser.cpp:2896, tests/browser/ut_sheetbrowser.cpp:2906-2924.
Prompt for AI agents
In `tests/browser/ut_sheetbrowser.cpp` at lines 2878-2890:
**Two-page scale regression passes**
When the tests run with the current fixture setup, `deform()` uses cached `m_maxWidth` and `m_maxHeight`, which these tests do not update after changing `m_originSizeF` (and are zero in the current fixture); the safety fallbacks do not exercise the large-page width budget. Also, `EXPECT_LE` accepts equal scales if the two-page divisor is removed.
Set up valid cached maxima after changing page sizes and assert a scale that distinguishes the two-page width budget from the single-page budget.
Also at `tests/browser/ut_sheetbrowser.cpp:2896`, `tests/browser/ut_sheetbrowser.cpp:2906-2924`.|
|
||
| // 有文字的真实文档: 每个矩形对应一段文本, 数量严格一致 | ||
| EXPECT_GT(charCount, 0); | ||
| EXPECT_EQ(rects.count(), texts.count()); |
There was a problem hiding this comment.
🟡 Medium · Empty text results pass alignment check
When allTextRects reports a positive character count but returns empty rects and texts, BUG68550_allTextRectsRectTextAligned allows rects and texts to both be empty: equal empty lists satisfy the alignment assertion even when the real page has characters, so the test passes if the method returns no selection rectangles or text.
Assert that the output contains at least one rectangle and text entry for this known text-bearing fixture.
Prompt for AI agents
In `tests/document/ut_pdfmodel.cpp` at line 865:
**Empty text results pass alignment check**
When `allTextRects` reports a positive character count but returns empty `rects` and `texts`, `BUG68550_allTextRectsRectTextAligned` allows `rects` and `texts` to both be empty: equal empty lists satisfy the alignment assertion even when the real page has characters, so the test passes if the method returns no selection rectangles or text.
Assert that the output contains at least one rectangle and text entry for this known text-bearing fixture.
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
🔍 详细分析1. 语法逻辑 ✓ — 25/25 分评价词： 语法正确，逻辑清晰 潜在问题： 分析详情：
建议： 无 2. 代码质量 ✓ — 23/25 分评价词： 代码结构清晰，注释完整 潜在问题：
分析详情：
建议： 建议将 ut_centraldocpage.cpp 中 BUG20978 系列测试的重复初始化代码提取为辅助函数，减少代码重复 3. 代码性能 ✓ — 20/20 分评价词： 性能良好，资源使用合理 潜在问题： 分析详情：
建议： 无 4. 代码安全 ✓ — 30/30 分评价词： 存在0个安全漏洞
安全漏洞详情： 漏洞对比统计：新增漏洞 0 个，减少漏洞 0 个，持平 0 个 安全扫描结果审计：
建议： 无 💡 改进建议代码示例// 建议将 ut_centraldocpage.cpp 中 BUG20978 系列测试的重复初始化提取为辅助方法
// 在 TestCentralDocPage 类中添加辅助方法：
// DocSheet* setupShortcutTestSheet() {
// Stub s;
// s.set(ADDR(CentralDocPage, getCurSheet), getCurSheet_stub2);
// s.set(ADDR(ReaderImageThreadPoolManager, addgetDocImageTask), addgetDocImageTask_stub);
// QString strPath = UTSOURCEDIR;
// strPath += "/files/normal.pdf";
// g_docsheet = new DocSheet(Dr::FileType::PDF, strPath, nullptr);
// return g_docsheet;
// }本报告由 AI 代码审查工具自动生成 |
Generated by qt-autotest-generator skill (PMS mining, Mode 7 -> Mode 2). Batch 2: SheetBrowser, BrowserPage, BrowserWord, BookMarkWidget, CentralDocPage, DocTabBar, TitleWidget, Application, PDFModel/DPdfDoc. Baseline: master @ 9061ecb "fix: walk the parent chain safely in CentralDocPage" (2026-10-08) Log: 补充 deepin-reader PMS 批次 2 单元测试（38 个回归用例） Influence: 新增 20 个 sev1/2 bug 的回归用例 38 个，38/38 通过； 全量 1264 用例 1263 通过（1 个存量 flaky 与本批次无关） 依据 PMS bug 修复历史补强，按文件分布： - tests/document/ut_pdfmodel.cpp +10 (46911/54210/68550/304111/ 304471/351271/353429) - tests/browser/ut_sheetbrowser.cpp +10 (22093/43216/44136/44264/ 44491/41018) - tests/sidebar/ut_bookmarkwidget.cpp +5 (10332/12219) - tests/uiframe/ut_centraldocpage.cpp +3 (20978) - tests/ut_application.cpp +3 (44137/44264) - tests/browser/ut_browserword.cpp +2 (40946) - tests/browser/ut_browserpage.cpp +2 (25004) - tests/uiframe/ut_doctabbar.cpp +2 (46381) - tests/uiframe/ut_titlewidget.cpp +1 (44136) 覆盖率（lcov, reader/ 范围）：lines 84.7% (13106/15471)， functions 98.1% (1364/1390)；21/21 目标函数 FNDA 全命中。 另 2 个 bug（102352/347387）位于 htmltopdf main() 内 __sw_64__ 条件编译，x86 构建不可达，需申威环境手工验证。 测试中新发现的源码缺陷留本地记录，不入库。
lzwind
force-pushed
the
test/pms-batch2-ut
branch
from
October 9, 2026 02:11
6af5765 to
1ec94bf
Compare
lzwind
approved these changes
Oct 9, 2026
Contributor
Author
|
/merge |
Contributor
|
This pr cannot be merged! (status: unstable) |
lzwind
approved these changes
Oct 9, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: add-uos, lzwind 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 |
Contributor
Author
|
/forcemerge |
Contributor
|
This pr force merged! (status: unstable) |
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
3 participants
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.
Generated by qt-autotest-generator skill (PMS mining, Mode 7 -> Mode 2).
Baseline: master @ 9061ecb
概要
依据 PMS sev1/2 bug 修复历史补强 Google Test 回归用例（批次 2，接续 #411）：
20 个 bug / 38 个用例，38/38 通过；全量 1264 用例 1263 通过。
用例分布
另 2 个 bug（102352/347387）位于 htmltopdf main() 内
__sw_64__条件编译，x86 构建不可达，需申威环境手工验证。覆盖率（lcov, reader/ 范围）
自测
test-deepin-reader二进制运行 1264 用例BUG<id>_<场景>，均带回溯注释（根因/回归意图）Log: 补充 deepin-reader PMS 批次 2 单元测试（38 个回归用例）
Influence: 仅新增/追加测试文件，不修改被测源码与构建配置
Summary by Sourcery
Expand regression test coverage for the second batch of deepin-reader PMS issues without changing production code or build configuration.
Enhancements:
Tests: