Repository navigation
test: add unit tests for opslog, threads and auth - #565
Merged
Merged
Conversation
Reviewer's GuideThis test-only PR adds broad unit coverage across ops-log exporting, parse-thread lifecycle and log parsing, authentication-related handlers, and DBus interfaces/managers, using temporary filesystem fixtures and targeted stubs to exercise success, failure, boundary, and unavailable-service branches. 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 4 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="tests/src/ut_logauththread.cpp" line_range="830" />
<code_context>
+ stub.set(ADDR(QProcess, exitCode), stub_exitCode);
+ stub.set(ADDR(DLDBusHandler, readLog), stub_authReadLog);
+
+ QString tmpPath = "/tmp/test_auth_ut_dlv.txt";
+ {
+ QFile tmpFile(tmpPath);
</code_context>
<issue_to_address>
**Temporary files overwrite or disrupt other data**
When these tests run concurrently or a fixed `/tmp` path already exists or is a symlink, `QFile::open(QIODevice::WriteOnly)` truncates shared files or follows symlinks, and `QFile::remove` can delete another run's fixture; tests overwrite unrelated data or fail nondeterministically.
Use a unique, securely created temporary file for each test invocation and remove only the file that invocation created.
Also at `tests/src/ut_logauththread.cpp:833-834`, `tests/src/ut_logauththread.cpp:843`, `tests/src/ut_logauththread.cpp:883-899`, `tests/src/ut_logauththread.cpp:904-915`.
</issue_to_address>
### Comment 2
<location path="tests/src/ut_opslogexport.cpp" line_range="402" />
<code_context>
+ ASSERT_TRUE(tmpDir.isValid());
+ OpsLogExport exporter(tmpDir.path().toStdString());
+
+ exporter.exportAptLogs();
+ SUCCEED();
+}
</code_context>
<issue_to_address>
**Export tests use host log trees**
When the tests run on a host with large, inaccessible, or unusual log directories, `exportAptLogs` and the UOS-Ste exporters read and copy real `/var/log` trees, so test runtime and resource use depend on host contents and large trees can exhaust temporary storage.
Stub the filesystem work and exercise the exporters with isolated fixture directories.
Also at `tests/src/ut_opslogexport.cpp:412`, `tests/src/ut_opslogexport.cpp:422`.
</issue_to_address>
### Comment 3
<location path="tests/src/ut_logauththread.cpp" line_range="804" />
<code_context>
+ m_logAuthThread->m_FilePath = QStringList() << "/var/log/audit/audit.log";
+ m_logAuthThread->handleAudit();
+ Utils::runInCmd = false;
+ EXPECT_EQ(m_logAuthThread->m_canRun, true);
+}
+
</code_context>
<issue_to_address>
**Handler tests accept empty results**
When a handler returns normally but fails to parse, emit, or apply its filter to the supplied records, `handleAudit_UT_001` (line 804), `handleAuth_UT_001` (line 841), `handleCoredump_UT_001` (line 877), and both DPKG time-filter tests (lines 942 and 971) only assert that `m_canRun` remains true. The handlers leave it true after successful returns, so dropping every parsed record or ignoring the DPKG filter still passes.
Assert the emitted data and its contents, and verify that the in-range and out-of-range DPKG inputs produce different results.
Also at `tests/src/ut_logauththread.cpp:841`, `tests/src/ut_logauththread.cpp:877`, `tests/src/ut_logauththread.cpp:943`, `tests/src/ut_logauththread.cpp:972`.
</issue_to_address>
### Comment 4
<location path="tests/src/ut_logauththread.cpp" line_range="892" />
<code_context>
+ }
+
+ QString result = m_logAuthThread->readAppLogFromLastLines(tmpPath, 2);
+ EXPECT_TRUE(result.contains("line3") || result.contains("line4"));
+
+ // Test with non-existent file
</code_context>
<issue_to_address>
**Log test accepts only one line**
When `readAppLogFromLastLines` returns only one of the requested final two lines, the OR assertion passes when the result contains `line3` or `line4` alone, so the test does not catch a regression that returns fewer lines than requested.
Assert that the result contains both expected final lines.
</issue_to_address>| stub.set(ADDR(QProcess, exitCode), stub_exitCode); | ||
| stub.set(ADDR(DLDBusHandler, readLog), stub_authReadLog); | ||
|
|
||
| QString tmpPath = "/tmp/test_auth_ut_dlv.txt"; |
There was a problem hiding this comment.
🟠 High · Temporary files overwrite or disrupt other data
When these tests run concurrently or a fixed /tmp path already exists or is a symlink, QFile::open(QIODevice::WriteOnly) truncates shared files or follows symlinks, and QFile::remove can delete another run's fixture; tests overwrite unrelated data or fail nondeterministically.
Use a unique, securely created temporary file for each test invocation and remove only the file that invocation created.
Also at tests/src/ut_logauththread.cpp:833-834, tests/src/ut_logauththread.cpp:843, tests/src/ut_logauththread.cpp:883-899, tests/src/ut_logauththread.cpp:904-915.
Prompt for AI agents
In `tests/src/ut_logauththread.cpp` at line 830:
**Temporary files overwrite or disrupt other data**
When these tests run concurrently or a fixed `/tmp` path already exists or is a symlink, `QFile::open(QIODevice::WriteOnly)` truncates shared files or follows symlinks, and `QFile::remove` can delete another run's fixture; tests overwrite unrelated data or fail nondeterministically.
Use a unique, securely created temporary file for each test invocation and remove only the file that invocation created.
Also at `tests/src/ut_logauththread.cpp:833-834`, `tests/src/ut_logauththread.cpp:843`, `tests/src/ut_logauththread.cpp:883-899`, `tests/src/ut_logauththread.cpp:904-915`.| ASSERT_TRUE(tmpDir.isValid()); | ||
| OpsLogExport exporter(tmpDir.path().toStdString()); | ||
|
|
||
| exporter.exportAptLogs(); |
There was a problem hiding this comment.
🟡 Medium · Export tests use host log trees
When the tests run on a host with large, inaccessible, or unusual log directories, exportAptLogs and the UOS-Ste exporters read and copy real /var/log trees, so test runtime and resource use depend on host contents and large trees can exhaust temporary storage.
Stub the filesystem work and exercise the exporters with isolated fixture directories.
Also at tests/src/ut_opslogexport.cpp:412, tests/src/ut_opslogexport.cpp:422.
Prompt for AI agents
In `tests/src/ut_opslogexport.cpp` at line 402:
**Export tests use host log trees**
When the tests run on a host with large, inaccessible, or unusual log directories, `exportAptLogs` and the UOS-Ste exporters read and copy real `/var/log` trees, so test runtime and resource use depend on host contents and large trees can exhaust temporary storage.
Stub the filesystem work and exercise the exporters with isolated fixture directories.
Also at `tests/src/ut_opslogexport.cpp:412`, `tests/src/ut_opslogexport.cpp:422`.| m_logAuthThread->m_FilePath = QStringList() << "/var/log/audit/audit.log"; | ||
| m_logAuthThread->handleAudit(); | ||
| Utils::runInCmd = false; | ||
| EXPECT_EQ(m_logAuthThread->m_canRun, true); |
There was a problem hiding this comment.
🟡 Medium · Handler tests accept empty results
When a handler returns normally but fails to parse, emit, or apply its filter to the supplied records, handleAudit_UT_001 (line 804), handleAuth_UT_001 (line 841), handleCoredump_UT_001 (line 877), and both DPKG time-filter tests (lines 942 and 971) only assert that m_canRun remains true. The handlers leave it true after successful returns, so dropping every parsed record or ignoring the DPKG filter still passes.
Assert the emitted data and its contents, and verify that the in-range and out-of-range DPKG inputs produce different results.
Also at tests/src/ut_logauththread.cpp:841, tests/src/ut_logauththread.cpp:877, tests/src/ut_logauththread.cpp:943, tests/src/ut_logauththread.cpp:972.
Prompt for AI agents
In `tests/src/ut_logauththread.cpp` at line 804:
**Handler tests accept empty results**
When a handler returns normally but fails to parse, emit, or apply its filter to the supplied records, `handleAudit_UT_001` (line 804), `handleAuth_UT_001` (line 841), `handleCoredump_UT_001` (line 877), and both DPKG time-filter tests (lines 942 and 971) only assert that `m_canRun` remains true. The handlers leave it true after successful returns, so dropping every parsed record or ignoring the DPKG filter still passes.
Assert the emitted data and its contents, and verify that the in-range and out-of-range DPKG inputs produce different results.
Also at `tests/src/ut_logauththread.cpp:841`, `tests/src/ut_logauththread.cpp:877`, `tests/src/ut_logauththread.cpp:943`, `tests/src/ut_logauththread.cpp:972`.| } | ||
|
|
||
| QString result = m_logAuthThread->readAppLogFromLastLines(tmpPath, 2); | ||
| EXPECT_TRUE(result.contains("line3") || result.contains("line4")); |
There was a problem hiding this comment.
🟡 Medium · Log test accepts only one line
When readAppLogFromLastLines returns only one of the requested final two lines, the OR assertion passes when the result contains line3 or line4 alone, so the test does not catch a regression that returns fewer lines than requested.
Assert that the result contains both expected final lines.
Prompt for AI agents
In `tests/src/ut_logauththread.cpp` at line 892:
**Log test accepts only one line**
When `readAppLogFromLastLines` returns only one of the requested final two lines, the OR assertion passes when the result contains `line3` or `line4` alone, so the test does not catch a regression that returns fewer lines than requested.
Assert that the result contains both expected final lines.
pengfeixx
force-pushed
the
ut-coverage-batch-7
branch
from
October 27, 2026 11:30
ce8b9ac to
b031dd1
Compare
pengfeixx
changed the title
pengfeixx
force-pushed
the
ut-coverage-batch-7
branch
from
October 27, 2026 11:34
b031dd1 to
44c86d3
Compare
1. Add ut_opslogexport.cpp, ut_parsethreadkern_kwin.cpp and ut_parsethreadbase.cpp 2. Extend ut_logauththread.cpp auth flows 3. Extend ut_dldbusinterface.cpp and ut_dbusmanager.cpp Log: No user-facing changes Influence: 1. Run deepin-log-viewer-test with --gtest_filter='*Ops*;*Parse*' 2. Verify full suite passes with ASAN_OPTIONS=detect_leaks=0 3. Check lcov function coverage of opslogexport and parse threads test: 新增运维日志、解析线程与鉴权单元测试 1. 新增 ut_opslogexport.cpp、ut_parsethreadkern_kwin.cpp 与 ut_parsethreadbase.cpp 2. 扩展 ut_logauththread.cpp 鉴权流程 3. 扩展 ut_dldbusinterface.cpp 与 ut_dbusmanager.cpp Log: 无用户可见变化 Influence: 1. 运行 deepin-log-viewer-test 过滤 Ops/Parse 套件 2. 以 ASAN_OPTIONS=detect_leaks=0 验证全量用例通过 3. 用 lcov 检查 opslogexport 与解析线程覆盖率
pengfeixx
force-pushed
the
ut-coverage-batch-7
branch
from
October 27, 2026 12:09
44c86d3 to
8cc12ab
Compare
Contributor
|
TAG Bot New tag: 6.5.43 |
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价： 语法正确，逻辑清晰 ✅ 通过 潜在问题： 建议： [] 2. 代码质量 ✅评价： 代码结构清晰，注释完整 ✅ 通过 潜在问题： 建议： ['建议在 ut_parsethreadkern_kwin.cpp 的 HandleKern_PkexecPath_ExitCodeZero 和 HandleKern_PkexecPath_ExitCodeNonZero 测试中，第二个 Utils::runInCmd = false 为冗余赋值（前一行已设为 false），可移除以保持简洁。'] 3. 代码性能 ✅评价： 性能良好，资源使用合理 ✅ 通过 潜在问题： 建议： [] 4. 代码安全 🔒评价： 存在0个安全漏洞 ✅ 通过
安全漏洞详情： 建议： [] 💡 改进建议代码示例// 建议：移除冗余的 Utils::runInCmd = false 赋值 // ut_parsethreadkern_kwin.cpp - HandleKern_PkexecPath_ExitCodeZero TEST_F(ParseThreadKern_UT, HandleKern_PkexecPath_ExitCodeZero) {
Stub stub;
stub.set(ADDR(DLDBusHandler, getFileInfo), stub_getFileInfo_txt);
stub.set(ADDR(DLDBusHandler, getLineCount), stub_getLineCount_100);
stub.set(ADDR(DLDBusHandler, readLogLinesInRange), stub_readLogLinesInRange_kern);
stub.set((void (QProcess::*)(const QString &, const QStringList &, QIODevice::OpenMode))ADDR(QProcess, start), stub_start);
stub.set(ADDR(QProcess, waitForFinished), stub_waitForFinished);
stub.set(ADDR(QProcess, setProcessChannelMode), stub_setProcessChannelMode);
stub.set(ADDR(QProcess, exitCode), stub_exitCode_zero);
stub.set(ADDR(SharedMemoryManager, setRunnableTag), stub_setRunnableTag);
stub.set(ADDR(SharedMemoryManager, getRunnableKey), stub_getRunnableKey);
Utils::runInCmd = false;
m_kern->handleKern();
// 移除冗余的 Utils::runInCmd = false; （上一行已设置为 false）
SUCCEED();
}本报告由 AI 代码审查工具自动生成 |
lzwind
approved these changes
Oct 27, 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 |
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
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.
Part 7/8 of the deepin-log-viewer unit-test coverage series (single-commit PR; all 8 PRs touch disjoint file sets and are mutually independent).
Files in this PR (6 files, +1469/-8 lines)
tests/src/ut_opslogexport.cpp(new)tests/src/ut_parsethreadkern_kwin.cpp(new)tests/src/ut_logauththread.cpptests/src/ut_parsethreadbase.cpp(new)tests/dbusproxy/ut_dldbusinterface.cpptests/src/ut_dbusmanager.cppNotes
tests/src//tests/dbusproxy//tests/ut_logviewerplugin/, globbed automatically bytests/CMakeLists.txt(re-run cmake after pulling).Summary by Sourcery
Increase unit-test coverage across log export, parsing, authentication, and DBus components.
Enhancements:
Tests: