Skip to content

Commit 71b8779

Browse files
committed
fix(ci): Execute newly added or modified scheduled_only tests in pre-submits
1 parent 7151573 commit 71b8779

6 files changed

Lines changed: 423 additions & 2 deletions

File tree

.github/workflows/run_pathways_tests.yml

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,11 @@ jobs:
113113
if [ "${{ inputs.is_scheduled_run }}" = "true" ]; then
114114
FINAL_PYTEST_MARKER="${{ inputs.pytest_marker }}"
115115
else
116-
FINAL_PYTEST_MARKER="${{ inputs.pytest_marker }} and not scheduled_only"
116+
if [ -z "${{ inputs.pytest_marker }}" ]; then
117+
FINAL_PYTEST_MARKER="not scheduled_only or newly_added"
118+
else
119+
FINAL_PYTEST_MARKER="${{ inputs.pytest_marker }} and (not scheduled_only or newly_added)"
120+
fi
117121
fi
118122
export MAXTEXT_REPO_ROOT=$(pwd)
119123
export MAXTEXT_ASSETS_ROOT=$(pwd)/src/maxtext/assets

.github/workflows/run_tests_against_package.yml

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ jobs:
100100
uses: actions/checkout@v5
101101
with:
102102
ref: ${{ inputs.maxtext_sha }}
103+
fetch-depth: 0
103104
- name: Download the maxtext wheel
104105
if: ${{ !inputs.maxtext_installed }}
105106
uses: actions/download-artifact@634f93cb2916e3fdff6788551b99b062d0335ce0 # v5.0.0
@@ -147,7 +148,11 @@ jobs:
147148
if [ "${INPUTS_IS_SCHEDULED_RUN}" == "true" ]; then
148149
FINAL_PYTEST_MARKER="${INPUTS_PYTEST_MARKER}"
149150
else
150-
FINAL_PYTEST_MARKER="${INPUTS_PYTEST_MARKER} and not scheduled_only"
151+
if [ -z "${INPUTS_PYTEST_MARKER}" ]; then
152+
FINAL_PYTEST_MARKER="not scheduled_only or newly_added"
153+
else
154+
FINAL_PYTEST_MARKER="${INPUTS_PYTEST_MARKER} and (not scheduled_only or newly_added)"
155+
fi
151156
fi
152157
# TODO: Use package data for testing and remove the env vars
153158
export MAXTEXT_REPO_ROOT=$(pwd)

pytest.ini

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ markers =
3838
NOTE: this marker is not to be used manually, it is auto-
3939
applied to tests with external_* or tpu_only marker.
4040
scheduled_only: marks tests to run only on scheduled CI runs
41+
newly_added: newly introduced or modified tests in PRs, executed even if scheduled_only
4142
integration_test: tests exercising larger portions of the system,
4243
including interactions with other systems like GCS,
4344
e.g., end_to_end tests

tests/conftest.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ def _custom_iter(self):
115115
import maxtext # pylint: disable=unused-import
116116

117117
from maxtext.common.gcloud_stub import is_decoupled
118+
from tests.utils.newly_added_detection import get_changed_tests
118119

119120
# Configure JAX to use unsafe_rbg PRNG implementation to match main scripts.
120121
if is_decoupled():
@@ -149,6 +150,15 @@ def pytest_collection_modifyitems(config, items):
149150
- Deselect tests marked as external_serving/training in decoupled mode.
150151
- Mark remaining tests with the `decoupled` marker when running decoupled.
151152
"""
153+
154+
changed_tests = get_changed_tests() # set[(file_path, test_name)]
155+
if changed_tests:
156+
for item in items:
157+
item_file = item.nodeid.split("::", 1)[0]
158+
base_name = getattr(item, "originalname", item.name)
159+
if (item_file, base_name) in changed_tests or (item_file, item.name) in changed_tests:
160+
item.add_marker(pytest.mark.newly_added)
161+
152162
decoupled = is_decoupled()
153163
remaining = []
154164
deselected = []
@@ -199,6 +209,7 @@ def pytest_configure(config):
199209
"external_training: goodput integrations",
200210
"decoupled: marked on tests that are not skipped due to GCP deps, when DECOUPLE_GCLOUD=TRUE",
201211
"skip_on_tpu7x: skip test if running on TPU7x platform",
212+
"newly_added: newly introduced or modified tests in PRs, executed even if scheduled_only",
202213
]:
203214
config.addinivalue_line("markers", m)
204215

Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
# Copyright 2026 Google LLC
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# https://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
"""Unit tests for the line-range test-change detector in ``newly_added_detection``."""
16+
17+
import textwrap
18+
19+
import pytest
20+
21+
from tests.utils.newly_added_detection import _build_diff_commands
22+
from tests.utils.newly_added_detection import _is_test_file
23+
from tests.utils.newly_added_detection import parse_changed_line_map
24+
from tests.utils.newly_added_detection import touched_test_names
25+
26+
pytestmark = pytest.mark.cpu_only
27+
28+
29+
# --- _is_test_file -----------------------------------------------------------
30+
31+
32+
def test_is_test_file_accepts_test_suffixes_under_tests():
33+
assert _is_test_file("tests/unit/a_test.py")
34+
assert _is_test_file("tests/integration/b_tests.py")
35+
36+
37+
def test_is_test_file_rejects_non_tests_and_non_suffix():
38+
assert not _is_test_file("tests/utils/test_helpers.py") # helper, not *_test.py
39+
assert not _is_test_file("src/maxtext/foo_test.py") # outside tests/
40+
assert not _is_test_file("tests/unit/helper.py") # not a test suffix
41+
42+
43+
# --- _build_diff_commands ----------------------------------------------------
44+
45+
46+
def test_build_diff_commands_uses_only_merge_base_threedot_ranges():
47+
# Only three-dot (merge-base) ranges: remote first (CI / local-with-origin),
48+
# then local (developer without an origin remote). Two-dot tip-vs-tip ranges are
49+
# excluded because they over-report every commit the base gained past the fork
50+
# point, dragging unrelated scheduled_only tests into pre-submit.
51+
cmds = _build_diff_commands("main")
52+
assert cmds == [
53+
["git", "diff", "--unified=0", "origin/main...HEAD"],
54+
["git", "diff", "--unified=0", "main...HEAD"],
55+
]
56+
# Guard against a two-dot form sneaking back in: every range arg is three-dot.
57+
range_args = [cmd[-1] for cmd in cmds]
58+
assert all("..." in arg for arg in range_args)
59+
60+
61+
def test_build_diff_commands_honours_non_main_base():
62+
cmds = _build_diff_commands("release/v2")
63+
assert cmds == [
64+
["git", "diff", "--unified=0", "origin/release/v2...HEAD"],
65+
["git", "diff", "--unified=0", "release/v2...HEAD"],
66+
]
67+
68+
69+
# --- parse_changed_line_map --------------------------------------------------
70+
71+
72+
def test_line_map_added_region_from_header_range():
73+
diff = (
74+
"diff --git a/tests/unit/a_test.py b/tests/unit/a_test.py\n"
75+
"--- a/tests/unit/a_test.py\n"
76+
"+++ b/tests/unit/a_test.py\n"
77+
"@@ -0,0 +1,3 @@\n"
78+
"+line1\n+line2\n+line3\n"
79+
)
80+
assert parse_changed_line_map(diff) == {"tests/unit/a_test.py": {1, 2, 3}}
81+
82+
83+
def test_line_map_single_line_default_length():
84+
diff = "+++ b/tests/unit/a_test.py\n" "@@ -10 +12 @@\n" "+changed\n"
85+
assert parse_changed_line_map(diff) == {"tests/unit/a_test.py": {12}}
86+
87+
88+
def test_line_map_pure_deletion_marks_boundary():
89+
# `+4,0` = pure deletion; the join sits between new-file lines 4 and 5.
90+
diff = "+++ b/tests/unit/a_test.py\n" "@@ -5,2 +4,0 @@\n" "-gone1\n-gone2\n"
91+
assert parse_changed_line_map(diff) == {"tests/unit/a_test.py": {4, 5}}
92+
93+
94+
def test_line_map_accumulates_multiple_hunks_and_files():
95+
diff = (
96+
"+++ b/tests/unit/a_test.py\n"
97+
"@@ -0,0 +1,1 @@\n"
98+
"+x\n"
99+
"@@ -8,0 +10,2 @@\n"
100+
"+y\n+z\n"
101+
"+++ b/tests/unit/b_test.py\n"
102+
"@@ -0,0 +3,1 @@\n"
103+
"+w\n"
104+
)
105+
assert parse_changed_line_map(diff) == {
106+
"tests/unit/a_test.py": {1, 10, 11},
107+
"tests/unit/b_test.py": {3},
108+
}
109+
110+
111+
def test_line_map_ignores_deleted_file():
112+
diff = "+++ /dev/null\n" "@@ -1,2 +0,0 @@\n" "-a\n-b\n"
113+
assert not parse_changed_line_map(diff)
114+
115+
116+
# --- tests_touching_lines ----------------------------------------------------
117+
118+
_SOURCE = textwrap.dedent(
119+
"""\
120+
import pytest
121+
122+
123+
class TestAlpha:
124+
125+
@pytest.mark.scheduled_only
126+
def test_existing(self):
127+
x = 1
128+
assert x == 1
129+
130+
def test_untouched(self):
131+
assert True
132+
133+
134+
def test_top_level_untouched():
135+
assert True
136+
137+
138+
async def test_async_new():
139+
assert True
140+
"""
141+
)
142+
# Line numbers in _SOURCE:
143+
# 6 @pytest.mark.scheduled_only
144+
# 7 def test_existing (span 6-9, decorator included)
145+
# 11 def test_untouched (span 11-12)
146+
# 15 def test_top_level_untouched (span 15-16)
147+
# 19 async def test_async_new (span 19-20)
148+
149+
150+
def test_touching_body_line_flags_that_test():
151+
assert touched_test_names(_SOURCE, {9}) == {"test_existing"}
152+
153+
154+
def test_touching_decorator_line_flags_that_test():
155+
assert touched_test_names(_SOURCE, {6}) == {"test_existing"}
156+
157+
158+
def test_touching_async_test_is_detected():
159+
assert touched_test_names(_SOURCE, {20}) == {"test_async_new"}
160+
161+
162+
def test_untouched_tests_are_not_flagged():
163+
# Editing test_existing must not drag in the neighbouring untouched tests.
164+
assert touched_test_names(_SOURCE, {8, 9}) == {"test_existing"}
165+
166+
167+
def test_lines_outside_any_test_flag_nothing():
168+
assert touched_test_names(_SOURCE, {1, 2}) == set() # imports / blank lines
169+
170+
171+
def test_empty_touched_set_returns_empty():
172+
assert touched_test_names(_SOURCE, set()) == set()
173+
174+
175+
def test_unparseable_source_returns_empty_gracefully():
176+
# pytest collection surfaces the syntax error itself; the parser must not raise.
177+
assert touched_test_names("def broken(:\n", {1}) == set()
178+
179+
180+
def test_insertion_after_untouched_test_does_not_flag_it():
181+
# The regression that git's hunk-header heuristic caused: a new test added
182+
# right after an untouched test must flag ONLY the new test.
183+
source = textwrap.dedent(
184+
"""\
185+
def test_old():
186+
assert True
187+
188+
189+
def test_new():
190+
assert True
191+
"""
192+
)
193+
# test_old spans lines 1-2; test_new spans lines 5-6. The added lines are 5-6.
194+
assert touched_test_names(source, {5, 6}) == {"test_new"}

0 commit comments

Comments
 (0)