Skip to content

fix(tests): 修复 Qt6 下 12 个失败用例 - #297

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
pengfeixx:fix/qt6-failing-tests
Jul 21, 2026
Merged

fix(tests): 修复 Qt6 下 12 个失败用例#297
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
pengfeixx:fix/qt6-failing-tests

Conversation

@pengfeixx

@pengfeixx pengfeixx commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

SheetBrowser::deform 用例 (testdeform001-007)

  • 源码引入 safeMaxWidth/safeMaxHeight 防除零保护后, scaleFactor 不再恒为 1.0;将严格相等断言改为 EXPECT_GT/GE。

PDFPage::updateAnnotation 用例

  • Qt6 下 QList::contains 成员函数无法用 stub 替换,改用 stub DPdfPage::annots() 配合全局 g_dAnnotlsit 列表, 让 m_page->annots().contains(annotation->ownAnnotation()) 在 Qt6 下也能命中。

PDFDocument::saveFilter 用例

  • 源码返回值由 "Portable document format (.pdf)" 改为 "Pdf Files (.pdf)",断言改为 contains("*.pdf")。

BaseWidget/TipsWidget 主题色用例

  • 源码 updateWidgetTheme/onUpdateTheme 只设置 DPalette::Window, 测试却校验 ItemBackground,已对齐到 Window。

ScaleMenu::onScaleFactor 用例

  • Qt6 下 QList<QAction*>::indexOf stub 失效,改用 emit m_actionGroup.first()->triggered() 触发真实信号链, 让 onScaleFactor 内 sender() 返回有效 QAction。

Summary by Sourcery

Adjust tests to pass under Qt6 by updating Qt version-specific stubs and expectations.

Bug Fixes:

  • Fix PDF page annotation tests by stubbing DPdfPage::annots under Qt6 instead of QList::contains and cleaning up global annotation state.
  • Update PDF document save filter test assertions to match the new filter text format while still validating the PDF extension.
  • Relax SheetBrowser deform tests' scale factor assertions to only require non-negative or positive values after safe dimension changes.
  • Align BaseWidget and TipsWidget theme color tests with the Window palette role used by the implementation.
  • Update ScaleMenu scale factor test to trigger a real QAction signal under Qt6 instead of stubbing QList<QAction*>::indexOf so sender() is valid.

@sourcery-ai

sourcery-ai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Reviewer's Guide

This PR updates several unit tests to be compatible with Qt6 and with recent behavior changes in the application, mainly by adjusting assertions, working around limitations in stubbing QList methods, and aligning theme/scale expectations with actual implementation.

File-Level Changes

Change Details Files
Adapt PDFPage::updateAnnotation tests to Qt6 by stubbing DPdfPage::annots instead of QList::contains and managing a shared annotation list.
  • Introduce global QList<DPdfAnnot *> g_dAnnotlsit and make annots_stub return it when non-empty.
  • In updateAnnotation tests, for Qt5 keep stubbing QList::contains as before; for Qt6 populate g_dAnnotlsit with the test annotation and stub DPdfPage::annots to annots_stub.
  • After assertions in Qt6 builds, remove the test annotation from g_dAnnotlsit to avoid cross-test contamination.
tests/document/ut_pdfmodel.cpp
Relax PDFDocument::saveFilter expectations to match new filter label while still ensuring a PDF filter is present.
  • Replace strict equality check against "Portable document format (.pdf)" with assertions that saveFilter() is non-empty and its first item contains ".pdf".
tests/document/ut_pdfmodel.cpp
Update SheetBrowser deform tests to assert non-negative scale factors instead of a fixed scale factor of 1.0, reflecting safeMaxWidth/safeMaxHeight behavior.
  • In multiple deform tests, change EXPECT_TRUE(m_lastScaleFactor == 1.0) to EXPECT_GT or EXPECT_GE against 0.0 depending on the scenario, keeping other expectations unchanged.
tests/browser/ut_sheetbrowser.cpp
Align BaseWidget and TipsWidget theme tests with the implementation by checking Window color instead of ItemBackground.
  • In updateWidgetTheme and onUpdateTheme tests, change palette checks from ItemBackground vs Base to Window vs Base.
tests/widgets/ut_basewidget.cpp
tests/widgets/ut_tipswidget.cpp
Make ScaleMenu::onScaleFactor test work under Qt6 by triggering a real QAction signal instead of stubbing QList::indexOf, while preserving Qt5 behavior.
  • For Qt6 builds, assert the action group is non-empty and emit triggered() on the first action so onScaleFactor sees a valid sender().
  • For Qt5 builds, retain stubbing of QList<QAction *>::indexOf and explicitly call onScaleFactor().
tests/widgets/ut_scalemenu.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • The global variable name g_dAnnotlsit appears to be misspelled; consider renaming it to g_dAnnotList for clarity and consistency.
  • The Qt version conditionals around stubbing QList::contains/DPdfPage::annots are duplicated across multiple tests; consider extracting a small helper or macro to reduce repetition and make the Qt5/Qt6 behavior easier to follow.
  • In the SheetBrowser::deform tests, the expectations on m_lastScaleFactor now mix EXPECT_GT and EXPECT_GE; it may be clearer to standardize on a single invariant (e.g., strictly positive or non-negative) across all related test cases.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The global variable name `g_dAnnotlsit` appears to be misspelled; consider renaming it to `g_dAnnotList` for clarity and consistency.
- The Qt version conditionals around stubbing `QList::contains`/`DPdfPage::annots` are duplicated across multiple tests; consider extracting a small helper or macro to reduce repetition and make the Qt5/Qt6 behavior easier to follow.
- In the `SheetBrowser::deform` tests, the expectations on `m_lastScaleFactor` now mix `EXPECT_GT` and `EXPECT_GE`; it may be clearer to standardize on a single invariant (e.g., strictly positive or non-negative) across all related test cases.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@pengfeixx
pengfeixx force-pushed the fix/qt6-failing-tests branch from 4a3ba49 to 2f66a74 Compare July 21, 2026 05:29
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

★ 总体评分:95分

■ 【总体评价】

代码实现了Qt6环境下的单元测试适配,修复了Stub机制失效和断言过于严格的问题
逻辑正确且提高了测试兼容性,但因全局变量使用略有隐患扣5分

■ 【详细分析】

  • 1.语法逻辑(基本正确)✓

代码在 ut_pdfmodel.cpp 中通过 #if QT_VERSION 宏区分 Qt5 和 Qt6 的处理逻辑,正确地替换了失效的 QList::contains Stub,转而 Stub DPdfPage::annots。在 ut_scalemenu.cpp 中,通过 emit m_tester->m_actionGroup.first()->triggered() 替代直接调用,确保了 sender() 在 Qt6 下返回有效对象。
潜在问题:ut_pdfmodel.cpp 中新增的全局变量 g_dAnnotlsit 在并发测试或未执行清理分支时可能导致状态残留。
建议:尽量使用类成员变量或局部静态变量替代全局变量;确保在所有退出路径上清理 g_dAnnotlsit

  • 2.代码质量(良好)✓

代码注释清晰,如在 ut_pdfmodel.cpp 中添加了 "On Qt6 QList::contains stubbing does not work; stub DPdfPage::annots instead." 说明修改原因。断言放宽(如 EXPECT_GTcontains("*.pdf"))使得测试更具鲁棒性,适应不同环境。
潜在问题:全局变量 g_dAnnotlsit 命名略显随意(拼写为 lsit)。
建议:修正拼写错误为 g_dAnnotList,或进一步优化为更安全的作用域。

  • 3.代码性能(无性能问题)✓

测试代码的修改不涉及复杂算法或高频调用,性能影响可忽略不计。Stub 机制的替换不会引入额外性能开销。
建议:保持现有实现方式即可。

  • 4.代码安全(存在0个安全漏洞)✓

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
本次修改仅涉及单元测试代码,不存在外部输入处理、权限校验等安全风险,无安全漏洞。

  • 建议:无需安全修复。

■ 【改进建议代码示例】

// tests/document/ut_pdfmodel.cpp
// 修正全局变量命名,并确保异常安全清理
static QList<DPdfAnnot *> g_dAnnotList; // 修正拼写

QList<DPdfAnnot *> annots_stub()
{
    if (!g_dAnnotList.isEmpty())
        return g_dAnnotList;

    QList<DPdfAnnot *> dannots;
    g_textAnnots = new DPdfTextAnnot();
    dannots.append(g_textAnnots);
    // ...
    return dannots;
}

TEST_F(TestPDFPage, UT_PDFPage_updateAnnotation_001)
{
    // ...
#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
    g_dAnnotList.append(dAnnot);
    s.set(ADDR(DPdfPage, annots), annots_stub);
#endif

    EXPECT_TRUE(m_tester->updateAnnotation(annotation, text, color));
    EXPECT_TRUE(g_funcName == "updateTextAnnot_stub");
    
#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
    g_dAnnotList.removeAll(dAnnot);
#endif
    // ...
}

SheetBrowser::deform (testdeform001-007): source added
safeMaxWidth/safeMaxHeight anti-div-by-zero guard, scaleFactor no
longer equals 1.0; relaxed strict-equal asserts to EXPECT_GT/GE.
PDFPage::updateAnnotation: QList::contains cannot be stubbed on Qt6,
so stub DPdfPage::annots() with a global g_dAnnotlsit list instead.
PDFDocument::saveFilter: source value changed from
"Portable document format (*.pdf)" to "Pdf Files (*.pdf)".
BaseWidget/TipsWidget theme: source sets DPalette::Window only.
ScaleMenu::onScaleFactor: indexOf stub fails on Qt6, emit triggered()
on the first action to drive the real signal chain.

SheetBrowser::deform 用例(testdeform001-007):源码引入
safeMaxWidth/safeMaxHeight 防除零保护后 scaleFactor 不再恒为
1.0,将严格相等断言改为 EXPECT_GT/GE。
PDFPage::updateAnnotation 用例:Qt6 下 QList::contains 无法 stub,
改为 stub DPdfPage::annots() 配合全局 g_dAnnotlsit 命中。
PDFDocument::saveFilter 用例:源码返回值由
"Portable document format (*.pdf)" 改为 "Pdf Files (*.pdf)"。
BaseWidget/TipsWidget 主题色用例:源码只设置 DPalette::Window。
ScaleMenu::onScaleFactor 用例:Qt6 下 indexOf stub 失效,改用
emit triggered() 触发真实信号链让 sender() 返回有效 QAction。

Log: 修复 Qt6 下 12 个失败用例
Influence: 单测全量通过,CI 不再误报这 12 项用例失败。
@pengfeixx
pengfeixx force-pushed the fix/qt6-failing-tests branch from 2f66a74 to f17e43b Compare July 21, 2026 08:08
@deepin-ci-robot

Copy link
Copy Markdown

[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.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@pengfeixx

Copy link
Copy Markdown
Contributor Author

/merge

@deepin-bot
deepin-bot Bot merged commit 061e4fb into linuxdeepin:master Jul 21, 2026
11 checks passed
@pengfeixx
pengfeixx deleted the fix/qt6-failing-tests branch July 21, 2026 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants