Skip to content

[CI][BugFix] Isolate controller mock to instance level#8092

Merged
EmmonsCurse merged 1 commit into
PaddlePaddle:developfrom
EmmonsCurse:fix_test_cache_controller
Jul 2, 2026
Merged

[CI][BugFix] Isolate controller mock to instance level#8092
EmmonsCurse merged 1 commit into
PaddlePaddle:developfrom
EmmonsCurse:fix_test_cache_controller

Conversation

@EmmonsCurse

Copy link
Copy Markdown
Collaborator

Motivation

The cache controller unit test was affected by cross-test interference caused by mocking _free_host_cache at the class level or in a shared scope.

When MagicMock is applied globally, previously created controller instances (from earlier tests or garbage collection remnants) may still reference the original method or a shared mock state. This leads to non-deterministic behavior in __del__() execution paths and inconsistent call tracking.

Modifications

  • Moved MagicMock assignment from global/class scope to instance scope:
    • controller._free_host_cache = MagicMock()
  • Ensured the mock is stored in the instance __dict__, so only the current test instance is affected.
  • Preserved original behavior for other controller instances that do not define _free_host_cache, allowing them to execute the real class method.
  • Isolated destructor behavior (__del__) to avoid interference from stale or previously collected objects.
  • Improved test determinism by fully decoupling mock lifecycle from other instances and tests.

Usage or Command

N/A

Accuracy Tests

N/A

Checklist

  • Add at least a tag in the PR title.
    • Tag list: [[FDConfig],[APIServer],[Engine], [Scheduler], [PD Disaggregation], [Executor], [Graph Optimization], [Speculative Decoding], [RL], [Models], [Quantization], [Loader], [OP], [KVCache], [DataProcessor], [BugFix], [Docs], [CI], [Optimization], [Feature], [Benchmark], [Others], [XPU], [HPU], [GCU], [DCU], [Iluvatar], [Metax]]
    • You can add new tags based on the PR content, but the semantics must be clear.
  • Format your code, run pre-commit before commit.
  • Add unit tests. Please write the reason in this PR if no unit tests.
  • Provide accuracy results.
  • If the current PR is submitting to the release branch, make sure the PR has been submitted to the develop branch, then cherry-pick it to the release branch with the [Cherry-Pick] PR tag.

@EmmonsCurse

Copy link
Copy Markdown
Collaborator Author

/skip-ci ci_iluvatar
/skip-ci ci_hpu
/skip-ci build_xpu
/skip-ci pre_ce_test
/skip-ci stable_test
/skip-ci base_test
/skip-ci logprob_test
/skip-ci gpu_4cards_test

@PaddlePaddle-bot PaddlePaddle-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.

🤖 Paddle-CI-Agent | pr_review | 2026-07-02 13:28:46

📋 Review 摘要

PR 概述:将 CacheController 析构器单测中的 _free_host_cache mock 从类级 patch 改为当前 controller 实例属性,降低跨测试析构调用对 mock 计数的干扰。
变更范围tests/cache_manager/v1/test_cache_controller.py
影响面 Tag[KVCache]

问题

未发现阻塞性问题。PR 规范问题在下面章节报,不在这里重复。

📝 PR 规范检查

标题包含 [CI][BugFix] 两个官方 Tag;FastDeploy 普通 PR 标题规则要求仅保留一个官方 Tag。建议使用更贴合本次测试稳定性变更的标题:

标题建议(可直接复制):

  • [CI] Isolate controller mock to instance level

PR 描述已包含 ## Motivation## Modifications## Usage or Command## Accuracy Tests## Checklist,结构符合模板。

总体评价

代码改动与 PR 描述一致:实例级 MagicMock 只影响当前 controller,对 __del__ 的调用与异常吞掉路径仍有断言覆盖。未发现需要阻塞合入的代码问题;本地尝试执行 TestDestructor 失败,原因是当前环境缺少 paddle 依赖,未能完成运行时验证。

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (develop@7a60f79). Learn more about missing BASE report.

Additional details and impacted files
@@            Coverage Diff             @@
##             develop    #8092   +/-   ##
==========================================
  Coverage           ?   77.53%           
==========================================
  Files              ?      409           
  Lines              ?    58174           
  Branches           ?     9157           
==========================================
  Hits               ?    45106           
  Misses             ?    10199           
  Partials           ?     2869           
Flag Coverage Δ
GPU 77.53% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@EmmonsCurse
EmmonsCurse merged commit 55edabb into PaddlePaddle:develop Jul 2, 2026
42 checks passed
@EmmonsCurse
EmmonsCurse deleted the fix_test_cache_controller branch July 2, 2026 13:59
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