执行摘要
- 一句话:将 fanout 测试辅助模块迁至 tests/ 并改用 PYTHONPATH 注入
- 推荐动作:值得花两三分钟快速阅读,作为测试基础设施组织的样例,不建议精读。核心关注点是两点:一是 importlib.import_module 无法解析文件名带点的模块路径,导致测试辅助代码被迫寻找「无点模块名 + 可导入」的位置;二是通过 extra_env_vars 注入 PYTHONPATH 让 Ray driver 与 worker 都能看到 tests/ 目录的做法,这是 slime 仓库 E2E 测试中一个可复用的模式。该 PR 展示了作者在包边界上的审慎:避免测试代码渗入运行库。
功能与动机
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.」。
实现拆解
- 重命名与文档更新:将 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 逻辑的细节。
- 测试调用方配套修改(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/ 下的辅助模块。
- 无配置、schema、部署或 CI workflow 配套改动;本次变更本身就是测试与 CI 相关的整理,不引入新测试文件。
关键文件:
tests/fanout_test_helpers.py(模块 测试工具;类别 test;类型 rename-or-move;符号 compact_generate, grpo_normalize_by_group_index): 核心变更对象:从 slime/rollout/_fanout_test_helpers.py 重命名迁移至 tests/,去除下划线前缀与包内路径,docstring 同步更新为「测试专用 PYTHONPATH 导入」的新契约描述,明确定位为测试夹具而非运行库 API。
tests/test_qwen2.5_0.5B_fanout_short.py(模块 端到端测试;类别 test;类型 test-coverage;符号 execute, prepare): E2E 测试的调用方更新:新增 TESTS_DIR 并注入 PYTHONPATH,将两个自定义函数路径从 slime.rollout._fanout_test_helpers. 改为 fanout_test_helpers.,是保证迁移后 Ray 作业仍能解析 helper 的关键配套。
关键符号:compact_generate, grpo_normalize_by_group_index, execute, prepare
关键源码片段
tests/test_qwen2.5_0.5B_fanout_short.py
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,
}
评论区精华
本 PR 无 review 评论、无关联 Issue、无讨论线程,由作者 zhuzilin 自行合并。唯一可提炼的决策来自提交信息 [ci] move fanout_test_helpers to tests/ 与代码注释:测试夹具不应进入 slime 发布包,且必须依靠显式 PYTHONPATH 注入来保证 Ray driver 与 worker 的可导入性。
风险与影响
- 风险:风险整体很低,但有两处值得注意:
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 作业
关联脉络
- PR #2298 [NFC] Add observability subfolder: 同为包内代码重新组织的重构,将日志指标相关工具迁入子目录;与本 PR 一起体现 slime 仓库持续把辅助/可观测性代码从核心包剥离的演进方向。
- PR #2294 cleanup: 该 PR 整理了 tests/ 下的 GLM 逐层比较脚本并移除冗余依赖,与本 PR 都聚焦测试资产的组织与清理,说明 tests 目录结构正在被主动维护。
- PR #2242 fix: honor every eval.defaults key and restore per-dataset stop / min_new_tokens: 该 PR 也修改了 CI workflow 并涉及 e2e 测试配置,与本 PR 同属 CI/测试配置链路,体现仓库对 e2e 测试与 CI 配置的高频维护。
参与讨论