Skip to content

Fix TrainDistillTest failure by removing obsolete OfflineArrayRecordIterator mock#4036

Merged
copybara-service[bot] merged 1 commit into
mainfrom
bvandermoon-refactor-temp-dirs
Jun 2, 2026
Merged

Fix TrainDistillTest failure by removing obsolete OfflineArrayRecordIterator mock#4036
copybara-service[bot] merged 1 commit into
mainfrom
bvandermoon-refactor-temp-dirs

Conversation

@bvandermoon
Copy link
Copy Markdown
Collaborator

@bvandermoon bvandermoon commented Jun 1, 2026

Description

Remove the outdated mock of distillation_utils.OfflineArrayRecordIterator in test_main_offline_mode_skips_teacher_loading.

In commit 7329370, offline distillation was refactored to use MaxText's standard input pipeline (Grain) instead of the custom array record reader. This removed OfflineArrayRecordIterator from distillation_utils.py. However, the mock in train_distill_test.py was left in place, resulting in AttributeError failures during test setup. Removing the obsolete mock restores the test suite to a passing state.

FIXES: b/518933738

Tests

CI tests passing now

Checklist

  • I have performed a self-review of my code. For an optional AI review, add the gemini-review label.
  • I have necessary comments in my code, particularly in hard-to-understand areas.
  • I have run end-to-end tests and provided workload links above if applicable.
  • I have made or will make corresponding changes to the doc if needed.

Copy link
Copy Markdown
Collaborator

@ajkv-google ajkv-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 1, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@copybara-service copybara-service Bot merged commit 785e963 into main Jun 2, 2026
55 checks passed
@copybara-service copybara-service Bot deleted the bvandermoon-refactor-temp-dirs branch June 2, 2026 01:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants