Skip to content

Commit 6811f1d

Browse files
authored
fix(ci): run the full dashboard suite when a change reaches outside src/ (#37563)
The UI unit test job narrows a pull request to `vitest related <changed files>`. `related` maps a file to the tests that import it, so a file no test imports maps to nothing, and `--passWithNoTests` turns that empty selection into a green job. package.json, package-lock.json, the Vitest, Tailwind and TypeScript configs and tests/setupTests.ts are all in that category even though each of them can change the behaviour of every test in the suite, so a dashboard dependency bump merged having run no unit tests at all and only got real coverage later, from the full run on the push to litellm_internal_staging. Keep `related` for the common case where a pull request only touches files under src/, and fall back to the full suite as soon as one changed file sits outside it. The decision lives in .github/scripts/select_ui_test_scope.sh so it can be tested on its own, next to the existing classify_changes.sh gate.
1 parent 93c1461 commit 6811f1d

3 files changed

Lines changed: 209 additions & 17 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
#!/usr/bin/env bash
2+
set -uo pipefail
3+
4+
has_file=false
5+
has_file_outside_src=false
6+
while IFS= read -r file || [ -n "$file" ]; do
7+
[ -n "$file" ] || continue
8+
has_file=true
9+
case "$file" in
10+
src/*) ;;
11+
*) has_file_outside_src=true ;;
12+
esac
13+
done
14+
15+
{ [ "$has_file" = true ] && [ "$has_file_outside_src" = false ]; } && echo related || echo full

.github/workflows/test-litellm-ui-unit.yml

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -65,22 +65,33 @@ jobs:
6565
BASE_SHA: ${{ github.event.pull_request.base.sha }}
6666
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
6767
run: |
68-
if [ -n "$BASE_SHA" ]; then
69-
merge_base=$(gh api "repos/${{ github.repository }}/compare/${BASE_SHA}...${HEAD_SHA}?per_page=1" --jq '.merge_base_commit.sha')
70-
test -n "$merge_base"
71-
git fetch --no-tags --depth=1 origin "$merge_base" "$HEAD_SHA"
72-
changed_files=()
73-
while IFS= read -r f; do
74-
changed_files+=("$f")
75-
done < <(git diff --name-only --relative "$merge_base" "$HEAD_SHA" -- .)
76-
if [ ${#changed_files[@]} -eq 0 ]; then
77-
echo "No UI files changed in this PR; skipping unit tests."
78-
exit 0
79-
fi
80-
echo "Pull request: running tests related to ${#changed_files[@]} changed UI files"
81-
npm run test -- related "${changed_files[@]}" --run --passWithNoTests \
82-
--pool forks --poolOptions.forks.maxForks=14
83-
else
68+
full_suite() { npm run test -- --run --pool forks --poolOptions.forks.maxForks=14; }
69+
70+
if [ -z "$BASE_SHA" ]; then
8471
echo "Push to $GITHUB_REF_NAME: running the full suite"
85-
npm run test -- --run --pool forks --poolOptions.forks.maxForks=14
72+
full_suite
73+
exit 0
74+
fi
75+
76+
merge_base=$(gh api "repos/${{ github.repository }}/compare/${BASE_SHA}...${HEAD_SHA}?per_page=1" --jq '.merge_base_commit.sha')
77+
test -n "$merge_base"
78+
git fetch --no-tags --depth=1 origin "$merge_base" "$HEAD_SHA"
79+
changed_files=()
80+
while IFS= read -r f; do
81+
changed_files+=("$f")
82+
done < <(git diff --name-only --relative "$merge_base" "$HEAD_SHA" -- .)
83+
if [ ${#changed_files[@]} -eq 0 ]; then
84+
echo "No UI files changed in this PR; skipping unit tests."
85+
exit 0
8686
fi
87+
88+
scope=$(printf '%s\n' "${changed_files[@]}" | bash "$GITHUB_WORKSPACE/.github/scripts/select_ui_test_scope.sh")
89+
if [ "$scope" != related ]; then
90+
echo "Pull request: ${#changed_files[@]} changed UI files reach outside src/, so related would miss their dependents; running the full suite"
91+
full_suite
92+
exit 0
93+
fi
94+
95+
echo "Pull request: running tests related to ${#changed_files[@]} changed UI files"
96+
npm run test -- related "${changed_files[@]}" --run --passWithNoTests \
97+
--pool forks --poolOptions.forks.maxForks=14
Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
"""Regression tests for the UI unit-test scope decision.
2+
3+
`.github/workflows/test-litellm-ui-unit.yml` narrows the dashboard's Vitest run to
4+
`vitest related <changed files>` so a pull request only pays for the tests it can
5+
affect. `related` resolves a file to the tests that import it, so a file no test
6+
imports resolves to nothing, and with `--passWithNoTests` the job then goes green
7+
without running a single test. `package.json`, `package-lock.json`, the Vitest and
8+
TypeScript configs and `tests/setupTests.ts` are all such files, so a dependency
9+
bump used to merge untested.
10+
11+
`.github/scripts/select_ui_test_scope.sh` is the decision function that closes
12+
that hole: it prints `related` only when every changed file lives under `src/`,
13+
and `full` otherwise. These tests lock both the decision and the workflow step
14+
that consumes it, running the step's real shell against stubbed `gh`, `git` and
15+
`npm` so a regression shows up as the wrong Vitest command.
16+
"""
17+
18+
from __future__ import annotations
19+
20+
import os
21+
import subprocess
22+
from pathlib import Path
23+
24+
import pytest
25+
import yaml
26+
27+
REPO_ROOT = Path(__file__).resolve().parents[2]
28+
SCOPE_SCRIPT = REPO_ROOT / ".github" / "scripts" / "select_ui_test_scope.sh"
29+
WORKFLOW = REPO_ROOT / ".github" / "workflows" / "test-litellm-ui-unit.yml"
30+
STEP_NAME = "Run UI unit tests (Vitest)"
31+
32+
FULL_SUITE_ARGV = ["run", "test", "--", "--run", "--pool", "forks", "--poolOptions.forks.maxForks=14"]
33+
34+
NON_SRC_FILES = [
35+
"package.json",
36+
"package-lock.json",
37+
"vitest.config.ts",
38+
"tsconfig.json",
39+
"tests/setupTests.ts",
40+
"next.config.mjs",
41+
]
42+
43+
44+
def scope(changed: list[str]) -> str:
45+
result = subprocess.run(
46+
["bash", str(SCOPE_SCRIPT)],
47+
input="\n".join(changed),
48+
capture_output=True,
49+
text=True,
50+
check=True,
51+
)
52+
return result.stdout.strip()
53+
54+
55+
@pytest.mark.parametrize("changed_file", NON_SRC_FILES)
56+
def test_a_file_no_test_imports_selects_the_full_suite(changed_file: str) -> None:
57+
assert scope([changed_file]) == "full"
58+
59+
60+
def test_src_only_changes_stay_on_related() -> None:
61+
assert scope(["src/app/page.tsx", "src/lib/http/client.ts"]) == "related"
62+
63+
64+
def test_one_non_src_file_pulls_a_src_only_set_up_to_full() -> None:
65+
assert scope(["src/app/page.tsx", "package-lock.json"]) == "full"
66+
67+
68+
def test_a_path_merely_prefixed_with_src_is_not_under_src() -> None:
69+
assert scope(["srcipts/build.mjs"]) == "full"
70+
71+
72+
def test_an_empty_change_set_fails_open_to_the_full_suite() -> None:
73+
assert scope([]) == "full"
74+
75+
76+
def _step_script() -> str:
77+
workflow = yaml.safe_load(WORKFLOW.read_text())
78+
steps = workflow["jobs"]["ui-unit-tests"]["steps"]
79+
script = next(step["run"] for step in steps if step.get("name") == STEP_NAME)
80+
resolved = script.replace("${{ github.repository }}", "BerriAI/litellm")
81+
assert "${{" not in resolved, "the step uses an Actions expression this harness does not resolve"
82+
return resolved
83+
84+
85+
def _stub(bin_dir: Path, name: str, body: str) -> None:
86+
stub = bin_dir / name
87+
stub.write_text(f"#!/usr/bin/env bash\n{body}\n")
88+
stub.chmod(0o755)
89+
90+
91+
def _run_step(tmp_path: Path, changed: list[str], base_sha: str = "basesha") -> tuple[int, str, list[list[str]]]:
92+
"""Run the workflow step's real shell; return its status, stdout and every npm argv."""
93+
bin_dir = tmp_path / "bin"
94+
bin_dir.mkdir()
95+
changed_file = tmp_path / "changed.txt"
96+
changed_file.write_text("".join(f"{name}\n" for name in changed))
97+
npm_log = tmp_path / "npm.log"
98+
99+
_stub(bin_dir, "gh", 'echo "mergebasesha"')
100+
_stub(bin_dir, "git", 'if [ "$1" = diff ]; then cat "$CHANGED_FILES"; fi')
101+
_stub(bin_dir, "npm", 'printf "%s\\n" "$@" >>"$NPM_LOG"; printf "\\0" >>"$NPM_LOG"')
102+
103+
step = tmp_path / "step.sh"
104+
step.write_text(_step_script())
105+
106+
env = dict(os.environ)
107+
env["PATH"] = f"{bin_dir}{os.pathsep}{env['PATH']}"
108+
env["BASE_SHA"] = base_sha
109+
env["HEAD_SHA"] = "headsha"
110+
env["GITHUB_WORKSPACE"] = str(REPO_ROOT)
111+
env["GITHUB_REF_NAME"] = "litellm_internal_staging"
112+
env["CHANGED_FILES"] = str(changed_file)
113+
env["NPM_LOG"] = str(npm_log)
114+
115+
result = subprocess.run(
116+
["bash", "--noprofile", "--norc", "-eo", "pipefail", str(step)],
117+
cwd=tmp_path,
118+
capture_output=True,
119+
text=True,
120+
env=env,
121+
)
122+
raw = npm_log.read_text() if npm_log.exists() else ""
123+
invocations = [call.splitlines() for call in raw.split("\0") if call]
124+
return result.returncode, result.stdout + result.stderr, invocations
125+
126+
127+
def _related_argv(changed: list[str]) -> list[str]:
128+
return [
129+
"run",
130+
"test",
131+
"--",
132+
"related",
133+
*changed,
134+
"--run",
135+
"--passWithNoTests",
136+
"--pool",
137+
"forks",
138+
"--poolOptions.forks.maxForks=14",
139+
]
140+
141+
142+
def test_step_runs_related_for_a_src_only_pull_request(tmp_path: Path) -> None:
143+
changed = ["src/app/page.tsx", "src/lib/http/client.ts"]
144+
returncode, output, invocations = _run_step(tmp_path, changed)
145+
assert returncode == 0, output
146+
assert invocations == [_related_argv(changed)]
147+
148+
149+
@pytest.mark.parametrize("changed_file", NON_SRC_FILES)
150+
def test_step_runs_the_full_suite_when_a_changed_file_is_outside_src(tmp_path: Path, changed_file: str) -> None:
151+
returncode, output, invocations = _run_step(tmp_path, ["src/app/page.tsx", changed_file])
152+
assert returncode == 0, output
153+
assert invocations == [FULL_SUITE_ARGV]
154+
assert "related" not in " ".join(invocations[0])
155+
156+
157+
def test_step_runs_the_full_suite_on_a_push(tmp_path: Path) -> None:
158+
returncode, output, invocations = _run_step(tmp_path, ["src/app/page.tsx"], base_sha="")
159+
assert returncode == 0, output
160+
assert invocations == [FULL_SUITE_ARGV]
161+
162+
163+
def test_step_runs_nothing_when_the_pull_request_touches_no_dashboard_file(tmp_path: Path) -> None:
164+
returncode, output, invocations = _run_step(tmp_path, [])
165+
assert returncode == 0, output
166+
assert invocations == []

0 commit comments

Comments
 (0)