Repository navigation
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 52cyb 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 |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates the color selector unit test to assert that the palette binding settles successfully, both eliminating the Qt 6 GCC -Wunused-result warning under -Werror and preventing silent success when the binding fails. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="tests/ut_colorselector.cpp" line_range="56" />
<code_context>
ASSERT_TRUE(QQmlProperty::write(helper.object, "palette.window", QColor(Qt::white)));
- QTest::qWaitFor([&] {
+ ASSERT_TRUE(QTest::qWaitFor([&] {
return background->property("color")
== engine.evaluate("background.color1.normal.common.color()").toVariant();
</code_context>
<issue_to_address>
**Stale palette colors pass**
When the background and selector colors are already equal before the palette update settles, `getColorFromProperty` lets `qWaitFor` return immediately when the background color already equals the selector color, and the final assertion at line 60 checks only that same equality. The test therefore passes even if the palette update never occurs.
Assert that the palette write produces an expected color distinct from the pre-write value, then wait for the background color to reach that expected value.
</issue_to_address>1. Wrap the bare qWaitFor() call added by ccb129a in EXPECT_TRUE() 2. Silence GCC -Wunused-result on Qt 6, which breaks the -Werror build 3. Test now fails when the palette binding never settles Log: No user-facing changes Influence: 1. Build the unit-test target in Debug with warnings treated as errors 2. Run unit-test --gtest_filter='ut_ColorSelector.*', expect 4 passed 3. Break the palette binding on purpose to verify the new assert fires fix: 修复颜色选择器单测 qWaitFor 返回值未使用告警 1. 将 ccb129a 引入的裸 qWaitFor() 调用包进 EXPECT_TRUE() 2. 消除 Qt6 上的 GCC -Wunused-result 告警,避免 -Werror 下编译失败 3. 调色板绑定未生效时用例会断言失败,而不是静默通过 Log: 无用户可见变化 Influence: 1. Debug 配置下构建 unit-test 目标,确认无告警、编译通过 2. 运行 unit-test --gtest_filter='ut_ColorSelector.*',预期 4 个用例通过 3. 临时破坏调色板绑定,确认新加的断言能让用例失败 PMS: TASK-397289
deepin pr auto review🤖 AI 代码审查报告📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价: 语法正确,逻辑清晰 ✅ 通过 潜在问题: 建议: [] 2. 代码质量 ✅评价: 代码结构清晰,注释完整 ✅ 通过 潜在问题: 建议: [] 3. 代码性能 ✅评价: 性能良好,资源使用合理 ✅ 通过 潜在问题: 建议: [] 4. 代码安全 🔒评价: 存在0个安全漏洞 ✅ 通过
安全漏洞详情: 建议: [] 💡 改进建议代码示例// 当前代码已经是正确的修复方式,无需额外修改。 // EXPECT_TRUE(QTest::qWaitFor([&] { // return background->property("color") // == engine.evaluate("background.color1.normal.common.color()").toVariant(); // }));本报告由 AI 代码审查工具自动生成 |
|
TAG Bot New tag: 6.7.52 |
Log: No user-facing changes
Influence:
fix: 修复颜色选择器单测 qWaitFor 返回值未使用告警
Log: 无用户可见变化
Influence:
PMS: TASK-397289
Summary by Sourcery
Ensure the color selector test validates asynchronous palette binding completion while keeping strict-warning builds clean.
Bug Fixes:
Build: