[Refactor] Split large test files into per-concept files#3747
Merged
Conversation
Splits four monolithic test files into per-concept modules under
test/modules/ and test/rb/, mirroring the existing test/objectives/
and test/transforms/ layout. Pure file reorganization — no test logic
changes; pytest collection counts match pre-split baselines exactly
(modules: 3025; rb: 4992).
- test/test_modules.py, test/test_tensordictmodules.py, test/test_actors.py
-> test/modules/test_{mlp_conv,rnn,planners,dreamer,multiagent,
decision_transformer,td_module,actor,qvalue_actor}.py
plus test/modules/_modules_common.py and conftest.py.
- test/test_rb.py, test/test_rb_distributed.py, test/test_storage_map.py
-> test/rb/test_{rb_core,storages,samplers,prioritized,writers,
composable,rb_transforms,ensemble,distributed,storage_map}.py
plus test/rb/_rb_common.py and conftest.py.
- Update .github/labeler.yml globs (Modules -> test/modules/**,
ReplayBuffers -> test/rb/**) and adjust setup.cfg per-file-ignores.
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/rl/3747
Note: Links to docs will display an error until the docs builds have been completed. ❗ 1 Active SEVsThere are 1 currently active SEVs. If your PR is affected, please view them below: This comment was automatically generated by Dr. CI and updates every 15 minutes. |
meta-cla
Bot
added
the
CLA Signed
github-actions
Bot
added
CI
- test/modules/test_dreamer.py -> test_dreamer_components.py (basename clashed with test/objectives/test_dreamer.py — pytest could not import both modules with the same name in the absence of __init__.py) - test/modules/test_multiagent.py -> test_multiagent_models.py (clashed with test/libs/test_multiagent.py) - Run ufmt across the new files to satisfy the lint hook.
Same basename-collision issue as the previous commit: pytest cannot import both test/test_distributed.py and test/rb/test_distributed.py with the same module name. Preserve the original full basename (test_rb_distributed.py) to keep it unique, and update run_all.sh accordingly.
…rb/test_samplers.py PR pytorch#3749 added 6 new tests for RandomSampler/SliceSampler replacement-kwarg dispatch to test/test_rb.py — which this branch deleted as part of the split into test/rb/. Ported those 6 TestSamplers methods (plus the new Sampler/RandomSampler imports) into test/rb/test_samplers.py. All 6 ported tests pass locally; collection count rises from 225 to 231.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Splits four monolithic test files into per-concept modules under
test/modules/andtest/rb/, mirroring the existingtest/objectives/andtest/transforms/layout. Pure file reorganization — no test logic changes.test/test_modules.pytest/modules/test_{mlp_conv,rnn,planners,dreamer,multiagent,decision_transformer}.py(split across)test/test_tensordictmodules.pytest/modules/test_{td_module,rnn,decision_transformer,actor}.pytest/test_actors.pytest/modules/test_{actor,qvalue_actor}.pytest/test_rb.pytest/rb/test_{rb_core,storages,samplers,prioritized,writers,composable,rb_transforms,ensemble,distributed}.pytest/test_rb_distributed.pytest/rb/test_distributed.py(moved; absorbsTestRayRB)test/test_storage_map.pytest/rb/test_storage_map.py(verbatim move)Plus
test/modules/_modules_common.py+conftest.pyandtest/rb/_rb_common.py+conftest.pyfor shared flags/helpers (pattern mirrored fromtest/objectives/_objectives_common.py)..github/labeler.ymlglobs andsetup.cfgper-file-ignores updated accordingly.Test plan
pytest --collect-only test/modules/returns 3025 tests (matches pre-split baseline exactly)pytest --collect-only test/rb/returns 4992 tests (matches pre-split baseline exactly)ruff check test/modules/ test/rb/— 6 findings, all pre-existing in source (same count asruff checkagainst the pre-split files)pytest test/modules/test_mlp_conv.py::TestMLP— 1729 passedpytest test/rb/test_rb_core.py::test_replay_buffer_read_write_all_in_order*— 2 passed