Prhub

#2318 [ci] move fanout_test_helpers to tests/

原始 PR 作者 zhuzilin 合并时间 2026-08-24 16:03 文件变更 2 提交数 1 评论 0 代码增减 +18 / -17

执行摘要

将 fanout 测试辅助模块迁至 tests/ 并改用 PYTHONPATH 注入

PR body 为空,动机主要来自代码注释本身:原模块 docstring 说明辅助函数必须放在带下划线前缀的包内路径,是因为 --custom-generate-function-path / --custom-reward-post-process-path 通过 importlib.import_module 解析字符串,而 E2E 测试文件名自身含点,无法作为模块路径;这个位置约束导致测试专用代码混入了 slime 运行库。本次迁移即消除这一混淆:把夹具放到 tests/ 下,并让测试通过显式 PYTHONPATH 让 Ray 作业导入它。模块头部注释明确写道「These helpers are imported by module path from the Ray job started by the E2E test. They live on the test-only portion of PYTHONPATH because they are test fixtures, not part of slime's public or internal runtime API.」。

值得花两三分钟快速阅读,作为测试基础设施组织的样例,不建议精读。核心关注点是两点:一是 importlib.import_module 无法解析文件名带点的模块路径,导致测试辅助代码被迫寻找「无点模块名 + 可导入」的位置;二是通过 extra_env_vars 注入 PYTHONPATH 让 Ray driver 与 worker 都能看到 tests/ 目录的做法,这是 slime 仓库 E2E 测试中一个可复用的模式。该 PR 展示了作者在包边界上的审慎:避免测试代码渗入运行库。

讨论亮点

本 PR 无 review 评论、无关联 Issue、无讨论线程,由作者 zhuzilin 自行合并。唯一可提炼的决策来自提交信息 [ci] move fanout_test_helpers to tests/ 与代码注释:测试夹具不应进入 slime 发布包,且必须依靠显式 PYTHONPATH 注入来保证 Ray driver 与 worker 的可导入性。

实现拆解

  1. 重命名与文档更新:将 slime/rollout/_fanout_test_helpers.py 移动为 tests/fanout_test_helpers.py(status=renamed,previous_filename 记录为 slime/rollout/_fanout_test_helpers.py)。模块 docstring 从解释「下划线前缀标记测试基础设施、位于 slime/ 内以便 importlib 解析」改写为「这些是测试夹具,通过测试专用 PYTHONPATH 导入」;保留了 compact_generate 与 grpo_normalize_by_group_index 两个 helper 的行为契约说明,包括 roll-out_id 共享语义、group_index 归一化替换默认 reshape 逻辑的细节。
  2. 测试调用方配套修改(tests/test_qwen2.5_0.5B_fanout_short.py):新增 TESTS_DIR = os.path.dirname(os.path.abspath(file)) 用于动态定位 tests 目录;--custom-generate-function-path 与 --custom-reward-post-process-path 的参数值由 slime.rollout._fanout_test_helpers.compact_generate / grpo_normalize_by_group_index 改为 fanout_test_helpers.compact_generate / fanout_test_helpers.grpo_normalize_by_group_index;extra_env_vars 由仅传 SLIME_FANOUT_TEST_COUNTER_FILE 扩展为同时注入 PYTHONPATH=f"{TESTS_DIR}:{U.repo_base_dir}:/root/Megatron-LM/",确保 Ray 提交的作业进程能解析到 tests/ 下的辅助模块。
  3. 无配置、schema、部署或 CI workflow 配套改动;本次变更本身就是测试与 CI 相关的整理,不引入新测试文件。
文件 模块 状态 重要度
tests/fanout_test_helpers.py 测试工具 renamed 4.59
tests/test_qwen2.5_0.5B_fanout_short.py 端到端测试 modified 4.93

关键符号

compact_generate grpo_normalize_by_group_index execute prepare

关键源码片段

tests/test_qwen2.5_0.5B_fanout_short.py test-coverage

E2E 测试的调用方更新:新增 TESTS_DIR 并注入 PYTHONPATH,将两个自定义函数路径从 slime.rollout._fanout_test_helpers.* 改为 fanout_test_helpers.*,是保证迁移后 Ray 作业仍能解析 helper 的关键配套。

# 把 tests 目录、仓库根目录和 Megatron-LM 一起注入 Ray 作业的 PYTHONPATH,
# 让辅助模块在 driver 与所有 worker 中都能按模块名导入,同时避免把测试模块
# 安装进 slime 包内部,保持运行库与测试夹具的边界清晰。
TESTS_DIR = os.path.dirname(os.path.abspath(__file__))extra_env_vars = {
    # tests 目录放在最前,优先解析仓库内测试资产,再继承仓库根与 Megatron 依赖
    "PYTHONPATH": f"{TESTS_DIR}:{U.repo_base_dir}:/root/Megatron-LM/",
    # 计数器文件路径通过环境变量分享给所有 worker 进程,供训练后断言使用
    "SLIME_FANOUT_TEST_COUNTER_FILE": FANOUT_COUNTER_FILE,
}

评论区精华

没有提炼出高价值讨论线程

当前评论区没有形成足够清晰的争议点或结论,后续有更多讨论时会体现在这里。

风险与影响

风险整体很低,但有两处值得注意:

1) 模块命名的顶层冲突——tests/fanout_test_helpers.py 现在成为顶级模块 fanout_test_helpers,若未来 tests/ 下出现同名文件或第三方包撞名,PYTHONPATH 顺序会决定谁被导入;当前注入顺序 {TESTS_DIR}:{U.repo_base_dir}:/root/Megatron-LM/ 把 tests 目录放在最前,已把这一不确定性显式化。
2) 隐式契约——测试现在依赖 extra_env_vars 中的 PYTHONPATH 才能导入 helper,若其他脚本绕过 execute_train 直接复用该 E2E 测试,或 CI 环境变更导致 PYTHONPATH 未按预期传递到 Ray 作业,custom generate 路径会静默解析失败;不过测试后置的计数器断言(expected_calls = 2 * 4)恰好能捕获这种静默绕过。
3) 对外兼容性——旧路径 slime.rollout._fanout_test_helpers 是带下划线前缀的内部符号,外部脚本若直接引用会受影响,但这属于非公开 API。

影响范围为测试与 CI 资产组织:slime 包目录不再包含仅供 E2E 测试使用的辅助模块,发布边界更清晰;tests/ 目录成为可被 Ray 作业 import 的测试资产目录。对运行时用户无影响,对仓库其他 E2E 测试(尤其是通过 importlib 引用自定义 generate/reward 函数的场景)提供了一个可复用的「把夹具放 tests/ + PYTHONPATH 注入」模式。团队后续写类似自定义函数型 E2E 测试时可参考该结构。

测试辅助模块升级为顶级模块,存在命名冲突可能 PYTHONPATH 注入成为隐式导入契约 旧路径 slime.rollout._fanout_test_helpers 可能被外部脚本引用 e2e 测试依赖 extra_env_vars 正确传递到 Ray 作业

关联 Issue

未识别关联 Issue

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

完整报告

参与讨论