Prhub

#39053 [ROCm][CI] Fix test repo-root assumptions

原始 PR 作者 AndreasKaratzas 合并时间 2026-04-07 13:36 文件变更 7 提交数 2 评论 0 代码增减 +21 / -11

rocm v1 kv-connector ci test

执行摘要

修复 ROCm CI 环境中集成测试脚本因缺少 Git 元数据导致的仓库根目录查找失败问题。

根据PR body描述，ROCm CI镜像将工作空间复制到/vllm-workspace，但运行时不需要活的.git检出，导致多个集成测试脚本使用git rev-parse --show-toplevel查找仓库根目录时失败。需要移除这种运行时假设，确保测试在CI环境中可靠运行。

该PR变更直接且必要，值得快速合并。对于关注CI基础设施和跨平台测试兼容性的工程师，可关注这种基于脚本位置解析仓库根目录的模式，作为处理无Git元数据环境的参考方案。

讨论亮点

review中仅有少量正面反馈，无争议讨论。Bortlesboat指出变更模式在所有7个脚本中保持一致，${GIT_ROOT:-...}回退机制设计巧妙，既允许CI覆盖又支持本地开箱即用，且cd -- && pwd -P方法避免了符号链接问题。tjtanaa和gemini-code-assist[bot]均表示认可。

实现拆解

修改了7个集成测试shell脚本，统一采用以下模式：1. 使用SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd -P)"获取脚本绝对路径；2. 通过GIT_ROOT="${GIT_ROOT:-$(cd -- "${SCRIPT_DIR}/../../../.." && pwd -P)}"设置仓库根目录，其中${GIT_ROOT:-...}允许环境变量覆盖；3. 移除原有的GIT_ROOT=$(git rev-parse --show-toplevel)调用。所有脚本都位于tests/v1/kv_connector/nixl_integration/或tests/v1/ec_connector/integration/目录下，相对路径../../../..能正确指向仓库根目录。

文件 模块 状态 重要度
tests/v1/kv_connector/nixl_integration/run_accuracy_test.sh kv-connector modified 4.0
tests/v1/ec_connector/integration/run_epd_correctness_test.sh ec_connector modified 4.0
tests/v1/kv_connector/nixl_integration/run_tpu_disagg_accuracy_test.sh kv-connector modified 3.0

分析完成后，这里会展示 LLM 生成的相对完整源码片段和详细注释。

评论区精华

路径解析模式的一致性与设计优点 设计

Bortlesboat 评论指出变更模式在所有 7 个脚本中保持一致，${GIT_ROOT:-...} 回退机制允许 CI 覆盖同时支持本地运行，cd -- && pwd -P 避免符号链接问题。

结论：设计得到认可，无修改建议。 · 已解决

风险与影响

技术风险较低：1. 路径解析逻辑简单，回归风险小；2. 使用pwd -P解析物理路径，避免符号链接问题；3. 保留${GIT_ROOT:-...}允许环境变量覆盖，不影响现有CI配置。潜在风险：相对路径../../../..依赖于脚本位置固定，若未来目录结构调整可能失效，但当前所有脚本都在相同深度目录下，风险可控。

影响范围限于ROCm CI环境中的集成测试执行可靠性，对用户功能无直接影响。确保kv-connector和ec_connector相关集成测试在ROCm CI中能正常运行，提升CI稳定性。对团队而言，减少了因环境差异导致的测试失败，维护成本降低。

路径依赖风险

关联 Issue

未识别关联 Issue

当前没有检测到明确关联的 Issue 链接，后续同步到相关引用后会出现在这里。

完整报告

Markdown PDF

执行摘要

  • 一句话：修复ROCm CI环境中集成测试脚本因缺少Git元数据导致的仓库根目录查找失败问题。
  • 推荐动作：该PR变更直接且必要，值得快速合并。对于关注CI基础设施和跨平台测试兼容性的工程师，可关注这种基于脚本位置解析仓库根目录的模式，作为处理无Git元数据环境的参考方案。

功能与动机

根据PR body描述，ROCm CI镜像将工作空间复制到/vllm-workspace，但运行时不需要活的.git检出，导致多个集成测试脚本使用git rev-parse --show-toplevel查找仓库根目录时失败。需要移除这种运行时假设，确保测试在CI环境中可靠运行。

实现拆解

修改了7个集成测试shell脚本，统一采用以下模式：1. 使用SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd -P)"获取脚本绝对路径；2. 通过GIT_ROOT="${GIT_ROOT:-$(cd -- "${SCRIPT_DIR}/../../../.." && pwd -P)}"设置仓库根目录，其中${GIT_ROOT:-...}允许环境变量覆盖；3. 移除原有的GIT_ROOT=$(git rev-parse --show-toplevel)调用。所有脚本都位于tests/v1/kv_connector/nixl_integration/或tests/v1/ec_connector/integration/目录下，相对路径../../../..能正确指向仓库根目录。

关键文件：

  • tests/v1/kv_connector/nixl_integration/run_accuracy_test.sh（模块 kv-connector）: 修改了kv-connector准确性测试脚本，是核心集成测试之一，且添加了详细的注释说明ROCm CI环境问题。
  • tests/v1/ec_connector/integration/run_epd_correctness_test.sh（模块 ec_connector）: 修改了ec_connector正确性测试脚本，覆盖另一个关键集成测试模块。
  • tests/v1/kv_connector/nixl_integration/run_tpu_disagg_accuracy_test.sh（模块 kv-connector）: 修改了TPU解聚准确性测试脚本，涉及多平台测试支持。

关键符号：未识别

评论区精华

review中仅有少量正面反馈，无争议讨论。Bortlesboat指出变更模式在所有7个脚本中保持一致，${GIT_ROOT:-...}回退机制设计巧妙，既允许CI覆盖又支持本地开箱即用，且cd -- && pwd -P方法避免了符号链接问题。tjtanaa和gemini-code-assist[bot]均表示认可。

  • 路径解析模式的一致性与设计优点 (design): 设计得到认可，无修改建议。

风险与影响

  • 风险：技术风险较低：1. 路径解析逻辑简单，回归风险小；2. 使用pwd -P解析物理路径，避免符号链接问题；3. 保留${GIT_ROOT:-...}允许环境变量覆盖，不影响现有CI配置。潜在风险：相对路径../../../..依赖于脚本位置固定，若未来目录结构调整可能失效，但当前所有脚本都在相同深度目录下，风险可控。
  • 影响：影响范围限于ROCm CI环境中的集成测试执行可靠性，对用户功能无直接影响。确保kv-connector和ec_connector相关集成测试在ROCm CI中能正常运行，提升CI稳定性。对团队而言，减少了因环境差异导致的测试失败，维护成本降低。
  • 风险标记：路径依赖风险

关联脉络

  • PR #37636 [KVConnector] Support 3FS KVConnector: 同属kv-connector模块，该PR引入了3FS KVConnector支持，而当前PR修复其集成测试的CI环境问题。
  • PR #38301 [KVConnector]: prioritize external connector over internal registry: 同属kv-connector模块的bugfix，当前PR修复的测试脚本可能用于验证此类功能。

参与讨论