fix(common): fix Qt6 regex porting bug and improve UT coverage - #554
Merged
deepin-bot[bot] merged 1 commit intoJul 20, 2026
Merged
Conversation
Fix QRegularExpression regression in parseNestedQString where spStr
was wrongly assigned a QString, causing split by literal "\\s+".
Also adapt 5 stub-based tests to Qt6 ABI and raise line coverage
from 79.8% to 81.3% by adding DBusManager/NewDspinBox test cases.
修复 parseNestedQString 中 Qt6 正则移植漏改问题,spStr 误赋为
QString 导致按字面字符串 "\\s+" 分割。同时适配 5 个 stub 测试以
兼容 Qt6 ABI,并新增 DBusManager/NewDspinBox 测试用例,将行覆盖
率从 79.8% 提升至 81.3%。
Log: 修复Qt6正则分割bug并提升单元测试覆盖率至80%以上
Influence: 修复后 -e 参数解析恢复正常,427 个单元测试全部通过,
无新增失败,主程序编译正常。
Reviewer's GuideFixes Qt6-specific regex handling in Utils::parseNestedQString to restore -e argument parsing behavior and updates several unit tests and stubs to align with Qt6 ABI while adding new coverage for DBusManager, NewDspinBox, Settings, TermWidgetPage, Service, and RemoteManagementPanel behaviors. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review★ 总体评分：95分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 // src/common/utils.cpp 无需修改，当前实现已为最佳实践。
// 对于 tests/src/main/ut_dbusmanager_test.cpp 中的弱断言测试，建议增强验证逻辑：
TEST_F(UT_Dbusmanager_Test, setConsoleFontFamily)
{
const QString testFamily = QStringLiteral("Noto Sans Mono");
m_pDbusManager->setConsoleFontFamily(testFamily);
// 若环境支持该字体，可验证是否设置成功；若不支持，验证默认值未被破坏
QString currentFamily = m_pDbusManager->consoleFontFamily();
EXPECT_FALSE(currentFamily.isEmpty());
} |
![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 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="tests/src/main/ut_dbusmanager_test.cpp" line_range="111-120" />
<code_context>
+TEST_F(UT_Dbusmanager_Test, consoleFontSize)
</code_context>
<issue_to_address>
**suggestion (testing):** Consider isolating and restoring `Settings` singleton state to avoid inter-test coupling.
These DBusManager tests mutate `Settings::instance()` (font size, opacity, cursor shape/blink, color scheme, etc.) and assume its state. Without resetting these fields, tests can become order-dependent and leak state to other suites.
Please either save and restore the affected `Settings` values per test (or in SetUp/TearDown / an RAII helper) so all modified fields are consistently reset after each test.
Suggested implementation:
```cpp
class SettingsGuard
{
public:
SettingsGuard()
: m_originalFontSize(Settings::instance()->fontSize())
{
}
~SettingsGuard()
{
Settings::instance()->setFontSize(m_originalFontSize);
}
private:
int m_originalFontSize;
};
TEST_F(UT_Dbusmanager_Test, callKDECurrentDesktop)
```
```cpp
TEST_F(UT_Dbusmanager_Test, consoleFontSize)
{
SettingsGuard settingsGuard;
const int expectSize = 18;
Settings::instance()->setFontSize(expectSize);
EXPECT_EQ(m_pDbusManager->consoleFontSize(), expectSize);
}
```
```cpp
TEST_F(UT_Dbusmanager_Test, setConsoleFontSize)
{
SettingsGuard settingsGuard;
const int newSize = 22;
m_pDbusManager->setConsoleFontSize(newSize);
}
```
The `SettingsGuard` currently only preserves and restores `fontSize`. To fully address inter-test coupling for other `Settings` fields (opacity, cursor shape/blink, color scheme, etc.), extend `SettingsGuard` to capture and reset all fields that the DBusManager tests touch, using the corresponding getters/setters (e.g. `opacity()`, `setOpacity()`, etc.), and instantiate `SettingsGuard` in each test that mutates those fields.
If your test fixture (`UT_Dbusmanager_Test`) already has `SetUp`/`TearDown`, you may alternatively move `SettingsGuard` into a member field and construct it in `SetUp` so every test in this suite benefits from automatic state restoration.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+111
to
+120
| TEST_F(UT_Dbusmanager_Test, consoleFontSize) | ||
| { | ||
| const int expectSize = 18; | ||
| Settings::instance()->setFontSize(expectSize); | ||
| EXPECT_EQ(m_pDbusManager->consoleFontSize(), expectSize); | ||
| } | ||
|
|
||
| TEST_F(UT_Dbusmanager_Test, setConsoleFontSize) | ||
| { | ||
| const int newSize = 22; |
There was a problem hiding this comment.
suggestion (testing): Consider isolating and restoring Settings singleton state to avoid inter-test coupling.
These DBusManager tests mutate Settings::instance() (font size, opacity, cursor shape/blink, color scheme, etc.) and assume its state. Without resetting these fields, tests can become order-dependent and leak state to other suites.
Please either save and restore the affected Settings values per test (or in SetUp/TearDown / an RAII helper) so all modified fields are consistently reset after each test.
Suggested implementation:
class SettingsGuard
{
public:
SettingsGuard()
: m_originalFontSize(Settings::instance()->fontSize())
{
}
~SettingsGuard()
{
Settings::instance()->setFontSize(m_originalFontSize);
}
private:
int m_originalFontSize;
};
TEST_F(UT_Dbusmanager_Test, callKDECurrentDesktop)
TEST_F(UT_Dbusmanager_Test, consoleFontSize)
{
SettingsGuard settingsGuard;
const int expectSize = 18;
Settings::instance()->setFontSize(expectSize);
EXPECT_EQ(m_pDbusManager->consoleFontSize(), expectSize);
}
TEST_F(UT_Dbusmanager_Test, setConsoleFontSize)
{
SettingsGuard settingsGuard;
const int newSize = 22;
m_pDbusManager->setConsoleFontSize(newSize);
}
The SettingsGuard currently only preserves and restores fontSize. To fully address inter-test coupling for other Settings fields (opacity, cursor shape/blink, color scheme, etc.), extend SettingsGuard to capture and reset all fields that the DBusManager tests touch, using the corresponding getters/setters (e.g. opacity(), setOpacity(), etc.), and instantiate SettingsGuard in each test that mutates those fields.
If your test fixture (UT_Dbusmanager_Test) already has SetUp/TearDown, you may alternatively move SettingsGuard into a member field and construct it in SetUp so every test in this suite benefits from automatic state restoration.
lzwind
approved these changes
Jul 20, 2026
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Contributor
Author
|
/merge |
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.
Fix QRegularExpression regression in parseNestedQString where spStr was wrongly assigned a QString, causing split by literal "\s+". Also adapt 5 stub-based tests to Qt6 ABI and raise line coverage from 79.8% to 81.3% by adding DBusManager/NewDspinBox test cases.
修复 parseNestedQString 中 Qt6 正则移植漏改问题,spStr 误赋为
QString 导致按字面字符串 "\s+" 分割。同时适配 5 个 stub 测试以
兼容 Qt6 ABI,并新增 DBusManager/NewDspinBox 测试用例,将行覆盖
率从 79.8% 提升至 81.3%。
Log: 修复Qt6正则分割bug并提升单元测试覆盖率至80%以上
Influence: 修复后 -e 参数解析恢复正常,427 个单元测试全部通过,
无新增失败,主程序编译正常。
Summary by Sourcery
Fix argument parsing whitespace splitting regression on Qt6 and update unit tests to improve coverage and Qt6 compatibility.
Bug Fixes:
Enhancements:
Tests: