test: fix stale fixtures in test_loop_cleanup#139
Merged
Conversation
Four tests failed on main because the fixtures had fallen behind the
production code they exercise:
- test_run_training_loop_finally_runs_cleanup_on_{success,exception}
asserted the _safe_training_cleanup call without inference_engines,
but run_training_loop now forwards inference_engines to it. Add
inference_engines=None to both assert_called_once_with calls.
- test_setup_eval_dispatch_bs_{is_dp_size,caps_at_dataset_size} built
SimpleNamespace args lacking inference_batch_size, which setup_eval
now reads on the eval-cache-miss branch. Add inference_batch_size=4;
it satisfies both assertions while preserving each test's intent
(dp_size*2 binds in the first, dataset_size cap binds in the second).
Test-only change: the production code is correct; the tests were stale.
tests/test_loop_cleanup.py now passes 9/9.
torchspec-bot
approved these changes
Jul 1, 2026
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
2 participants
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.
Four tests failed on main because the fixtures had fallen behind the
production code they exercise:
test_run_training_loop_finally_runs_cleanup_on_{success,exception}
asserted the _safe_training_cleanup call without inference_engines,
but run_training_loop now forwards inference_engines to it. Add
inference_engines=None to both assert_called_once_with calls.
test_setup_eval_dispatch_bs_{is_dp_size,caps_at_dataset_size} built
SimpleNamespace args lacking inference_batch_size, which setup_eval
now reads on the eval-cache-miss branch. Add inference_batch_size=4;
it satisfies both assertions while preserving each test's intent
(dp_size*2 binds in the first, dataset_size cap binds in the second).
Test-only change: the production code is correct; the tests were stale.
tests/test_loop_cleanup.py now passes 9/9.