Skip to content

fix:add test notification server applet case by gtest#1556

Closed
heysion wants to merge 7 commits intolinuxdeepin:masterfrom
heysion:heysion-dev
Closed

fix:add test notification server applet case by gtest#1556
heysion wants to merge 7 commits intolinuxdeepin:masterfrom
heysion:heysion-dev

Conversation

@heysion
Copy link
Copy Markdown
Member

@heysion heysion commented Apr 15, 2026

Description:

Log:

Copy link
Copy Markdown

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Sorry @heysion, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 15, 2026

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@heysion
Copy link
Copy Markdown
Member Author

heysion commented Apr 15, 2026

I have read the CLA Document and I hereby sign the CLA.

deepin-bot bot added a commit to linuxdeepin/cla that referenced this pull request Apr 15, 2026
heysion added 3 commits April 16, 2026 18:02
Fixed memory leaks in test code by adding proper destructors and cleanup:

1. Added destructor to TestModelA to release DataA objects stored in m_list
2. Added destructor to TestModelB to release DataB objects stored in m_list
3. Fixed QAbstractItemModelTester memory leak in RoleGroupModel.ModelTest
   by adding delete statement

Also added AddressSanitizer and UndefinedBehaviorSanitizer flags to CMake
configuration for memory leak detection:
- Added -fsanitize=undefined,address to CXX_FLAGS and C_FLAGS
- Added -O0 -Wall -g -ggdb3 for debug builds
- Applied flags to both main CMakeLists.txt and tests/CMakeLists.txt

Log: Fixed memory leaks in test code and added sanitizer flags

Influence:
1. Run RoleCombineModel tests to verify no memory leaks
2. Run RoleGroupModel tests to verify QAbstractItemModelTester cleanup
3. Verify AddressSanitizer detects no leaks in test execution
4. Test that debug build works with new compiler flags
5. Verify all existing tests still pass with sanitizer enabled

fix(test): 修复测试代码中的内存泄漏

通过添加适当的析构函数和清理代码修复测试代码中的内存泄漏:

1. 为 TestModelA 添加析构函数以释放 m_list 中存储的 DataA 对象
2. 为 TestModelB 添加析构函数以释放 m_list 中存储的 DataB 对象
3. 在 RoleGroupModel.ModelTest 中添加 delete 语句修复 QAbstractItemModelTester 内存泄漏

同时向 CMake 配置添加了 AddressSanitizer 和 UndefinedBehaviorSanitizer 标志
用于内存泄漏检测:
- 向 CXX_FLAGS 和 C_FLAGS 添加 -fsanitize=undefined,address
- 添加 -O0 -Wall -g -ggdb3 用于调试构建
- 将标志应用到主 CMakeLists.txt 和 tests/CMakeLists.txt

Log: 修复测试代码内存泄漏并添加 sanitizer 标志

Influence:
1. 运行 RoleCombineModel 测试验证无内存泄漏
2. 运行 RoleGroupModel 测试验证 QAbstractItemModelTester 清理
3. 验证 AddressSanitizer 在测试执行中未检测到泄漏
4. 测试调试构建在新编译器标志下正常工作
5. 验证所有现有测试在启用 sanitizer 后仍能通过
Removed AddressSanitizer and UndefinedBehaviorSanitizer flags from
CMakeLists.txt and tests/CMakeLists.txt:
- Removed -fsanitize=undefined,address from CXX_FLAGS and C_FLAGS
- Removed -O0 -Wall -g -ggdb3 debug flags
- Removed sanitizer linker flags

These flags were temporarily added for memory leak detection during
development but are not needed in the main branch.

Log: Remove sanitizer flags from CMake configuration

Influence:
1. Verify project builds successfully without sanitizer flags
2. Check that test compilation works normally
3. Ensure no performance impact from removed debug flags

chore(build): 从 CMakeLists 中移除 sanitizer 标志

从 CMakeLists.txt 和 tests/CMakeLists.txt 中移除了 AddressSanitizer
和 UndefinedBehaviorSanitizer 标志:
- 从 CXX_FLAGS 和 C_FLAGS 中移除 -fsanitize=undefined,address
- 移除 -O0 -Wall -g -ggdb3 调试标志
- 移除 sanitizer 链接器标志

这些标志是在开发期间临时添加用于内存泄漏检测的,但在主分支中不需要。

Log: 从 CMake 配置中移除 sanitizer 标志

Influence:
1. 验证项目在没有 sanitizer 标志的情况下成功构建
2. 检查测试编译是否正常工作
3. 确保移除调试标志后没有性能影响
Added a new CMake option `ENABLE_SANITIZER` to optionally enable
AddressSanitizer and UndefinedBehaviorSanitizer for memory leak
detection and debugging purposes.

Usage:
  cmake -DENABLE_SANITIZER=ON ..

When enabled, the following flags are added:
- -fsanitize=undefined,address for both C and C++
- -O0 -Wall -g -ggdb3 for debug builds
- sanitizer linker flags

This option is OFF by default and does not affect normal builds.

Log: Add optional sanitizer support via CMake option

Influence:
1. Verify project builds normally with ENABLE_SANITIZER=OFF (default)
2. Test build with ENABLE_SANITIZER=ON to verify sanitizer works
3. Check that the option appears in cmake-gui/ccmake
4. Verify no performance impact when option is disabled

feat(build): 添加 ENABLE_SANITIZER 选项用于调试

添加了新的 CMake 选项 `ENABLE_SANITIZER`,用于可选地启用 AddressSanitizer
和 UndefinedBehaviorSanitizer 进行内存泄漏检测和调试。

使用方法:
  cmake -DENABLE_SANITIZER=ON ..

启用时会添加以下标志:
- C 和 C++ 的 -fsanitize=undefined,address
- 调试构建的 -O0 -Wall -g -ggdb3
- sanitizer 链接器标志

此选项默认关闭,不影响正常构建。

Log: 通过 CMake 选项添加可选的 sanitizer 支持

Influence:
1. 验证项目在 ENABLE_SANITIZER=OFF(默认)时正常构建
2. 测试 ENABLE_SANITIZER=ON 时 sanitizer 是否工作
3. 检查选项是否出现在 cmake-gui/ccmake 中
4. 验证选项禁用时没有性能影响
18202781743
18202781743 previously approved these changes Apr 16, 2026
@@ -0,0 +1,52 @@
# SPDX-FileCopyrightText: 2024 - 2026 UnionTech Software Technology Co., Ltd.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

直接是2026,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

DONE

Updated SPDX-FileCopyrightText year range from "2024 - 2026" to "2026"
in notification server test files
Changed SPDX-License-Identifier from CC0-1.0 to GPL-3.0-or-later in
CMakeLists.txt to align with project licensing

Log: Update license headers and copyright year in notification tests

Influence:
1. Verify notification server tests still build correctly
2. Confirm license compatibility with project standards
3. Check copyright year consistency across test files

chore(tests): 更新许可证和版权头信息

将 SPDX-FileCopyrightText 年份范围从 "2024 - 2026" 更新为 "2026"
将 CMakeLists.txt 中的 SPDX-License-Identifier 从 CC0-1.0 更改为
GPL-3.0-or-later,以符合项目许可标准

Log: 更新通知测试中的许可证头和版权年份

Influence:
1. 验证通知服务器测试是否能正确构建
2. 确认许可证与项目标准兼容
3. 检查测试文件中版权年份的一致性
@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: heysion

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

1 similar comment
@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: heysion

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

@18202781743
Copy link
Copy Markdown
Contributor

解决一下license 和编译问题,

Added destructor to RoleGroupModel class to properly release resources
stored in m_map using qDeleteAll() to prevent memory leaks when the
model is destroyed

Added comprehensive memory leak test cases for RoleGroupModel:
- DestructorMemoryLeakTest: verify proper cleanup on destruction
- RebuildTreeSourceMemoryTest: test memory stability during tree rebuilds
- SetSourceModelMemoryTest: verify memory handling when switching source models
- EmptyModelMemoryTest: test boundary conditions with empty models
- LargeDataMemoryStabilityTest: stress test with large datasets
- SetDeduplicationRoleMemoryTest: verify cleanup when changing roles
- RowsInsertedMemoryTest: test memory handling on row insertion
- RowsRemovedWithEmptyGroupMemoryTest: verify cleanup when removing rows
- ModelResetMemoryTest: test memory stability during model reset

Log: Fix memory leak in RoleGroupModel with comprehensive test coverage

Influence:
1. Verify dock taskmanager functionality remains intact
2. Run memory leak tests with AddressSanitizer enabled
3. Test RoleGroupModel lifecycle scenarios
4. Validate memory cleanup in all test cases

fix(dock): 修复 RoleGroupModel 内存泄漏并添加全面测试

为 RoleGroupModel 类添加析构函数,使用 qDeleteAll() 正确释放 m_map
中存储的资源,防止模型销毁时的内存泄漏问题

添加全面的 RoleGroupModel 内存泄漏测试用例:
- DestructorMemoryLeakTest: 验证析构时的正确清理
- RebuildTreeSourceMemoryTest: 测试树重建时的内存稳定性
- SetSourceModelMemoryTest: 验证切换源模型时的内存处理
- EmptyModelMemoryTest: 测试空模型的边界条件
- LargeDataMemoryStabilityTest: 大数据集压力测试
- SetDeduplicationRoleMemoryTest: 验证角色切换时的清理
- RowsInsertedMemoryTest: 测试插入行时的内存处理
- RowsRemovedWithEmptyGroupMemoryTest: 验证删除行时的清理
- ModelResetMemoryTest: 测试模型重置时的内存稳定性

Log: 修复 RoleGroupModel 内存泄漏并添加全面测试覆盖

Influence:
1. 验证 dock 任务管理器功能保持正常
2. 启用 AddressSanitizer 运行内存泄漏测试
3. 测试 RoleGroupModel 生命周期场景
4. 验证所有测试用例的内存清理
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这份代码 diff 主要涉及内存管理优化和测试代码的补充。以下是对代码的详细审查和改进建议:

1. 代码逻辑与语法

1.1 析构函数实现(RoleGroupModel)

RoleGroupModel::~RoleGroupModel()
{
    qDeleteAll(m_map);
}

问题分析

  • 在析构函数中直接使用 qDeleteAll 可能存在潜在问题
  • 如果 m_map 是一个关联容器(如 QMap),qDeleteAll 会删除值对象
  • 没有看到 m_map 的定义,假设它存储的是堆分配的对象指针

改进建议

RoleGroupModel::~RoleGroupModel()
{
    qDeleteAll(m_map);
    m_map.clear();  // 显式清空容器,确保指针无效
}

1.2 测试代码中的内存泄漏测试

TEST(RoleGroupModel, DestructorMemoryLeakTest)
{
    // ...
    EXPECT_NO_THROW({
        delete groupModel;
        groupModel = nullptr;
    });
    // ...
    EXPECT_EQ(groupModel, nullptr);  // 这行检查在删除后没有意义
}

改进建议

TEST(RoleGroupModel, DestructorMemoryLeakTest)
{
    // ...
    delete groupModel;
    groupModel = nullptr;
    // 移除无意义的 EXPECT_EQ
    // 如果存在内存泄漏,ASan/Valgrind 会报告
}

2. 代码质量

2.1 测试代码重复

notifyserverapplet_test.cpp 中存在大量相似的测试用例,如:

TEST_F(NotifyServerAppletTest, ActionInvokedWithBubbleIdTest)
TEST_F(NotifyServerAppletTest, ActionInvokedWithoutBubbleIdTest)

改进建议:考虑使用参数化测试(TEST_P)来减少重复代码。

2.2 测试用例命名

部分测试用例名称过长,如:

TEST(RoleGroupModel, RowsRemovedWithEmptyGroupMemoryTest)

改进建议:简化命名,同时保持清晰度:

TEST(RoleGroupModel, RowsRemovedMemoryTest)

3. 代码性能

3.1 批量删除操作

model.removeRows(0, itemCount / 2);

改进建议:对于大量数据,考虑使用 beginResetModel()endResetModel() 来优化性能:

model.beginResetModel();
model.removeRows(0, itemCount / 2);
model.endResetModel();

4. 代码安全

4.1 空指针检查

notifyserverapplet_test.cpp 中:

TEST_F(NotifyServerAppletTest, AppValueEmptyAppIdTest)
{
    applet->init();
    QVariant result = applet->appValue(QString(), 0);
    (void)result; // Suppress unused warning
}

改进建议:添加返回值验证:

TEST_F(NotifyServerAppletTest, AppValueEmptyAppIdTest)
{
    applet->init();
    QVariant result = applet->appValue(QString(), 0);
    EXPECT_FALSE(result.isValid());  // 验证空ID时返回无效值
}

4.2 边界条件测试

TEST_F(NotifyServerAppletTest, NotificationClosedEdgeCasesTest)
{
    applet->init();
    EXPECT_NO_THROW(applet->notificationClosed(-1, 0, 1));
    // ...
}

改进建议:对于无效输入,应该验证错误处理:

TEST_F(NotifyServerAppletTest, NotificationClosedEdgeCasesTest)
{
    applet->init();
    // 验证无效ID不会导致崩溃,同时验证行为是否符合预期
    EXPECT_NO_THROW(applet->notificationClosed(-1, 0, 1));
    // 可能需要添加更多断言来验证行为
}

5. 其他建议

5.1 测试覆盖率

新增的测试用例很好,但建议:

  1. 添加性能测试用例,特别是对于大数据量的场景
  2. 添加并发测试用例,验证多线程安全性

5.2 文档注释

建议为新增的测试用例添加更详细的注释,说明测试的目的和预期行为:

// 测试析构函数是否正确释放内存(内存泄漏检测)
// 预期:RoleGroupModel 销毁时,所有内部分配的 QList 对象都被正确释放
TEST(RoleGroupModel, DestructorMemoryLeakTest)
{
    // ...
}

5.3 CMakeLists.txt

新添加的测试目录结构合理,但建议:

  1. 添加编译选项控制测试的构建
  2. 考虑将测试代码与生产代码分离得更彻底

总结

这份 diff 主要改进了内存管理,通过添加析构函数和大量内存泄漏测试用例来提高代码的健壮性。主要优点:

  1. 添加了必要的析构函数
  2. 增加了全面的内存管理测试用例
  3. 测试覆盖了多种边界情况

主要改进空间:

  1. 减少测试代码重复
  2. 增强测试断言的有效性
  3. 优化大数据量操作的性能
  4. 完善错误处理和边界条件验证

整体而言,这是一次有价值的代码改进,特别是在内存管理方面。建议在后续开发中继续关注测试用例的质量和覆盖率。

@heysion
Copy link
Copy Markdown
Member Author

heysion commented Apr 17, 2026

dup #1562 close #1556

@heysion heysion closed this Apr 17, 2026
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