[ci]: add transient pytest reruns for modal tests
This commit is contained in:
@@ -15,9 +15,9 @@
|
||||
- Branch: `issue/1571-flaky-gpu-test-retry`
|
||||
- Base: `origin/main` at `9d909f5f0 [test]: remove dead and duplicate tests (-489 lines) (#1556)`
|
||||
- Handoff path: `.agents/handoffs/issue-1571-handoff.md`
|
||||
- Current stage: Stage 1 - Deep Dive And Plan
|
||||
- Implementation begun: no. Stage 1 is analysis/planning only.
|
||||
- Last updated: 2026-07-10T08:40:26Z
|
||||
- Current stage: Stage 2 - Implement The User-Directed Fix
|
||||
- Implementation begun: yes. User approved the recommended central Modal pytest rerun approach.
|
||||
- Last updated: 2026-07-10T10:23:10Z
|
||||
|
||||
## Stage 0 Notes
|
||||
|
||||
@@ -52,6 +52,11 @@
|
||||
- #1389 `[ci] Modal: enable Flash Attention 3 on H100 perf tests with build cache`, ready-for-review, not draft. Modal/perf infrastructure, not pytest rerun support.
|
||||
- #1494 and #1183 are attention-related but not focused on CI retry behavior.
|
||||
- No PR draft status was changed.
|
||||
- Stage 2 re-check on 2026-07-10T10:10:42Z:
|
||||
- Verified `gh` identity as `macthecadillac`.
|
||||
- Issue #1571 remains OPEN with no comments and no assignees.
|
||||
- Focused open PR search for issue number/title/`pytest-rerunfailures`/`--only-rerun`/reruns found no direct implementation of this issue.
|
||||
- No PR draft status was changed.
|
||||
|
||||
## Investigation Log
|
||||
|
||||
@@ -159,7 +164,52 @@
|
||||
- Recommended approach: central pytest-rerunfailures support in Modal orchestrators with a transient-only regex, plus docs.
|
||||
- Awaiting user guidance before Stage 2 implementation.
|
||||
|
||||
## Selected Approach
|
||||
|
||||
- User approved the Stage 1 recommendation on 2026-07-10.
|
||||
- Implement Approach B:
|
||||
- Add `pytest-rerunfailures` to the test extra.
|
||||
- Add one Modal-local helper for retry args and transient regex.
|
||||
- Apply it centrally to `fastvideo/tests/modal/pr_test.py` and `fastvideo/tests/modal/ssim_test.py`.
|
||||
- Add focused static contract coverage.
|
||||
- Update CI/testing docs.
|
||||
|
||||
## Stage 2 Implementation Log
|
||||
|
||||
- Implemented central Modal pytest retry wiring:
|
||||
- Added `fastvideo/tests/modal/pytest_retry.py` with shared `pytest-rerunfailures` args, retry count `2`, delay `10s`, and a conservative transient-only regex.
|
||||
- Updated `fastvideo/tests/modal/pr_test.py` to compute `PYTEST_ADDOPTS` from the cloned repo's helper after checkout, so Modal does not need to mount helper files beside the entrypoint.
|
||||
- Updated `fastvideo/tests/modal/ssim_test.py` to append the same retry args to each SSIM subprocess pytest command.
|
||||
- Added `pytest-rerunfailures` to the `test` extra in `pyproject.toml`.
|
||||
- Added static contract coverage in `fastvideo/tests/contract/test_ci_pytest_reruns.py`.
|
||||
- Updated `docs/contributing/ci_architecture.md` and `docs/contributing/testing.md` with durable retry policy documentation.
|
||||
- Local cleanup:
|
||||
- `uv run --with modal ... --help` was accidentally started from the repo without `--no-project`; it began resolving FastVideo dependencies and created a small `.venv`.
|
||||
- Stopped that process and removed the generated `.venv`.
|
||||
|
||||
## Validation Log
|
||||
|
||||
- Do not run project tests locally rule followed; validation was run on Modal L40S through `/tmp/fastvideo-worktrees/interleavethinker-launcher/fastvideo/tests/modal/launch_l40s_job.py`.
|
||||
- Failed validation attempt 1:
|
||||
- Command:
|
||||
`uv run --no-project --with modal python -m modal run /tmp/fastvideo-worktrees/interleavethinker-launcher/fastvideo/tests/modal/launch_l40s_job.py --install-extra test --no-build-kernel --apply-local-patch --patch-paths "pyproject.toml,fastvideo/tests/modal/pr_test.py,fastvideo/tests/modal/ssim_test.py,fastvideo/tests/modal/pytest_retry.py,fastvideo/tests/contract/test_ci_pytest_reruns.py,docs/contributing/ci_architecture.md,docs/contributing/testing.md" --command "pytest fastvideo/tests/contract/test_ci_test_collection.py fastvideo/tests/contract/test_ci_pytest_reruns.py -q"`
|
||||
- Modal URL: https://modal.com/apps/hao-ai-lab/main/ap-r6oqddOSxQlgIduaTgx2Jq
|
||||
- Result: failed before tests. `uv pip install -e '.[test]'` attempted to build `fastvideo-kernel==0.3.2` from sdist and failed because `cutlass/cutlass.h` was missing from the package build context.
|
||||
- Assessment: unrelated to this CI retry patch; the targeted static tests do not require installing FastVideo or building kernels.
|
||||
- Failed validation attempt 2:
|
||||
- Command used `--install-extra none` and `uv pip install pytest` in the remote command.
|
||||
- Modal URL: https://modal.com/apps/hao-ai-lab/main/ap-njpA3gOITr1UJ4QO3DdqD6
|
||||
- Result: failed before tests with `/bin/bash: line 1: uv: command not found` because `--install-extra none` skips environment sourcing that places `uv` on PATH.
|
||||
- Assessment: command setup issue only.
|
||||
- Passing validation:
|
||||
- Command:
|
||||
`uv run --no-project --with modal python -m modal run /tmp/fastvideo-worktrees/interleavethinker-launcher/fastvideo/tests/modal/launch_l40s_job.py --install-extra none --no-build-kernel --apply-local-patch --patch-paths "pyproject.toml,fastvideo/tests/modal/pr_test.py,fastvideo/tests/modal/ssim_test.py,fastvideo/tests/modal/pytest_retry.py,fastvideo/tests/contract/test_ci_pytest_reruns.py,docs/contributing/ci_architecture.md,docs/contributing/testing.md" --command "source $HOME/.local/bin/env 2>/dev/null || true; source /opt/venv/bin/activate 2>/dev/null || true; uv pip install pytest && pytest fastvideo/tests/contract/test_ci_test_collection.py fastvideo/tests/contract/test_ci_pytest_reruns.py -q"`
|
||||
- Modal URL: https://modal.com/apps/hao-ai-lab/main/ap-Z7mqFKXntYOTP64dNof6m8
|
||||
- Result: passed, `6 passed in 0.04s`.
|
||||
- Modal return summary: local patch applied, `install_extra=none`, `build_kernel=False`, commit `732f9df86c5c67b6f55a7dd1e7400e1b78d9e69a`.
|
||||
|
||||
## Next Steps
|
||||
|
||||
- Commit and push this handoff-only Stage 1 state.
|
||||
- Present Stage 1 report to the user and ask whether to implement the recommended approach.
|
||||
- Stage and commit signed implementation changes.
|
||||
- Push branch.
|
||||
- Proceed to Stage 3 review/adjudication loop before presenting the draft PR message.
|
||||
|
||||
@@ -125,6 +125,18 @@ it), while a GitHub outage or a >25 min wait lets it run anyway (fail open).
|
||||
See [Performance Benchmarks](performance_benchmarks.md) for the performance
|
||||
lane's thresholds, rolling baseline, artifacts, and reseeding process.
|
||||
|
||||
### Modal Pytest Reruns
|
||||
|
||||
Modal pytest lanes install `pytest-rerunfailures` through the `test` extra.
|
||||
`fastvideo/tests/modal/pr_test.py` applies the shared retry policy through
|
||||
`PYTEST_ADDOPTS`, and `fastvideo/tests/modal/ssim_test.py` appends the same
|
||||
arguments to each SSIM subprocess.
|
||||
|
||||
The policy reruns an individual pytest failure twice with a short delay, but
|
||||
only when the failure text matches the transient infrastructure regex in
|
||||
`fastvideo/tests/modal/pytest_retry.py`. Plain assertion and numerical parity
|
||||
failures are not matched by that policy and should fail without rerun.
|
||||
|
||||
## Slash Commands
|
||||
|
||||
Slash commands are handled by `.github/workflows/ci-slash-commands.yml`.
|
||||
@@ -239,7 +251,9 @@ All Buildkite test jobs go through `.buildkite/scripts/pr_test.sh`, which:
|
||||
3. Passes Buildkite metadata into the Modal container.
|
||||
4. Runs the selected test command from `fastvideo/tests/modal/pr_test.py` or
|
||||
`fastvideo/tests/modal/ssim_test.py`.
|
||||
5. Uploads performance artifacts for `TEST_TYPE=performance`.
|
||||
5. Applies the shared transient-only pytest rerun policy to Modal pytest
|
||||
commands.
|
||||
6. Uploads performance artifacts for `TEST_TYPE=performance`.
|
||||
|
||||
If you add a new CI test category:
|
||||
|
||||
|
||||
@@ -219,6 +219,12 @@ instances. The main files are:
|
||||
For exact tier membership, path filters, slash commands, and aggregate statuses,
|
||||
see [CI/CD Architecture](ci_architecture.md).
|
||||
|
||||
Modal pytest commands use `pytest-rerunfailures` with the shared transient-only
|
||||
policy in `fastvideo/tests/modal/pytest_retry.py`. The retry layer reruns
|
||||
individual pytest failures for infrastructure-style errors such as connection,
|
||||
Modal worker, NCCL, or CUDA initialization failures. Deterministic assertion
|
||||
and numerical parity failures should still fail immediately.
|
||||
|
||||
### Adding A New CI Test Category
|
||||
|
||||
If a new test does not fit an existing lane:
|
||||
|
||||
@@ -0,0 +1,36 @@
|
||||
# SPDX-License-Identifier: Apache-2.0
|
||||
"""Static checks for Modal pytest rerun wiring.
|
||||
|
||||
Pure text analysis: no fastvideo imports, no GPU, no torch.
|
||||
"""
|
||||
from pathlib import Path
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parents[3]
|
||||
PYPROJECT = REPO_ROOT / "pyproject.toml"
|
||||
MODAL_ROOT = REPO_ROOT / "fastvideo" / "tests" / "modal"
|
||||
PYTEST_RETRY = MODAL_ROOT / "pytest_retry.py"
|
||||
PR_TEST = MODAL_ROOT / "pr_test.py"
|
||||
SSIM_TEST = MODAL_ROOT / "ssim_test.py"
|
||||
|
||||
|
||||
def test_pytest_rerunfailures_is_in_test_extra():
|
||||
assert '"pytest-rerunfailures"' in PYPROJECT.read_text()
|
||||
|
||||
|
||||
def test_modal_entrypoints_share_pytest_retry_policy():
|
||||
retry_text = PYTEST_RETRY.read_text()
|
||||
pr_text = PR_TEST.read_text()
|
||||
ssim_text = SSIM_TEST.read_text()
|
||||
|
||||
assert "TRANSIENT_FAILURE_REGEX" in retry_text
|
||||
assert "--only-rerun" in retry_text
|
||||
assert "build_pytest_addopts" in pr_text
|
||||
assert "PYTEST_ADDOPTS" in pr_text
|
||||
assert "_build_pytest_rerun_args()" in ssim_text
|
||||
|
||||
|
||||
def test_retry_policy_excludes_plain_assertion_failures():
|
||||
retry_text = PYTEST_RETRY.read_text()
|
||||
|
||||
assert "AssertionError" not in retry_text
|
||||
assert "assert " not in retry_text
|
||||
@@ -1,5 +1,6 @@
|
||||
import os
|
||||
import re
|
||||
import shlex
|
||||
import shutil
|
||||
import subprocess
|
||||
import sys
|
||||
@@ -194,6 +195,24 @@ def run_test(pytest_command: str):
|
||||
run_test_command(pytest_command, build_kernel=True)
|
||||
|
||||
|
||||
def _pytest_retry_setup_command() -> str:
|
||||
addopts_statement = (
|
||||
"import os, sys; "
|
||||
"sys.path.insert(0, 'fastvideo/tests/modal'); "
|
||||
"from pytest_retry import build_pytest_addopts; "
|
||||
"print(build_pytest_addopts(os.environ.get('PYTEST_ADDOPTS', '')))"
|
||||
)
|
||||
describe_statement = (
|
||||
"import sys; "
|
||||
"sys.path.insert(0, 'fastvideo/tests/modal'); "
|
||||
"from pytest_retry import describe_pytest_reruns; "
|
||||
"print(describe_pytest_reruns())"
|
||||
)
|
||||
addopts_command = f"python -c {shlex.quote(addopts_statement)}"
|
||||
describe_command = f"python -c {shlex.quote(describe_statement)}"
|
||||
return f'export PYTEST_ADDOPTS="$({addopts_command})" &&\n {describe_command} &&'
|
||||
|
||||
|
||||
def run_test_command(test_command: str,
|
||||
build_kernel: bool,
|
||||
install_command: str = 'uv pip install -e ".[test]"'):
|
||||
@@ -232,6 +251,7 @@ def run_test_command(test_command: str,
|
||||
source /opt/venv/bin/activate &&
|
||||
cd /FastVideo &&
|
||||
{install_clause}
|
||||
{_pytest_retry_setup_command()}
|
||||
{build_kernel_command}
|
||||
{test_command}
|
||||
"""
|
||||
|
||||
@@ -0,0 +1,51 @@
|
||||
# SPDX-License-Identifier: Apache-2.0
|
||||
"""Shared pytest rerun policy for Modal CI entrypoints."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import shlex
|
||||
|
||||
PYTEST_RERUNS = 2
|
||||
PYTEST_RERUNS_DELAY_SECONDS = 10
|
||||
|
||||
TRANSIENT_FAILURE_PATTERNS = (
|
||||
r"connection (?:reset|aborted|refused|timed out)",
|
||||
r"read timed out",
|
||||
r"remote end closed connection",
|
||||
r"temporarily unavailable",
|
||||
r"(?:HTTP|status code) 5\d\d",
|
||||
r"502 Bad Gateway",
|
||||
r"503 Service Unavailable",
|
||||
r"504 Gateway Timeout",
|
||||
r"NCCL.*(?:timeout|connection|remote error|unhandled system error|system error)",
|
||||
r"CUDA error: (?:unknown error|initialization error|operation not permitted|operation not supported)",
|
||||
r"Modal.*(?:preempt|interrupt|worker|container|timeout)",
|
||||
)
|
||||
TRANSIENT_FAILURE_REGEX = "(?i)(" + "|".join(TRANSIENT_FAILURE_PATTERNS) + ")"
|
||||
|
||||
|
||||
def build_pytest_rerun_args() -> list[str]:
|
||||
return [
|
||||
"--reruns",
|
||||
str(PYTEST_RERUNS),
|
||||
"--reruns-delay",
|
||||
str(PYTEST_RERUNS_DELAY_SECONDS),
|
||||
"--only-rerun",
|
||||
TRANSIENT_FAILURE_REGEX,
|
||||
]
|
||||
|
||||
|
||||
def build_pytest_addopts(existing_addopts: str = "") -> str:
|
||||
retry_addopts = shlex.join(build_pytest_rerun_args())
|
||||
existing_addopts = existing_addopts.strip()
|
||||
if existing_addopts:
|
||||
return f"{existing_addopts} {retry_addopts}"
|
||||
return retry_addopts
|
||||
|
||||
|
||||
def describe_pytest_reruns() -> str:
|
||||
return (
|
||||
"Pytest transient reruns enabled: "
|
||||
f"{PYTEST_RERUNS} reruns, {PYTEST_RERUNS_DELAY_SECONDS}s delay, "
|
||||
f"only-rerun={TRANSIENT_FAILURE_REGEX!r}"
|
||||
)
|
||||
@@ -680,7 +680,7 @@ def _build_pytest_extra_args(
|
||||
ssim_bootstrap_mode: bool,
|
||||
pytest_k: str,
|
||||
) -> list[str]:
|
||||
args = []
|
||||
args = _build_pytest_rerun_args()
|
||||
if ssim_full_quality:
|
||||
args.append("--ssim-full-quality")
|
||||
if ssim_reference_repo.strip():
|
||||
@@ -694,6 +694,21 @@ def _build_pytest_extra_args(
|
||||
return args
|
||||
|
||||
|
||||
def _build_pytest_rerun_args() -> list[str]:
|
||||
helper_paths = [
|
||||
os.path.dirname(os.path.abspath(__file__)),
|
||||
os.path.join("/FastVideo", "fastvideo", "tests", "modal"),
|
||||
]
|
||||
for helper_path in helper_paths:
|
||||
if helper_path not in sys.path:
|
||||
sys.path.insert(0, helper_path)
|
||||
|
||||
from pytest_retry import build_pytest_rerun_args, describe_pytest_reruns # noqa: E402
|
||||
|
||||
print(describe_pytest_reruns())
|
||||
return build_pytest_rerun_args()
|
||||
|
||||
|
||||
def _schedule_ssim_tasks(
|
||||
repo_root: str,
|
||||
tasks: list[SSIMTask],
|
||||
|
||||
@@ -136,6 +136,7 @@ test = [
|
||||
"av",
|
||||
"pytorch-msssim>=1.0.0",
|
||||
"pytest",
|
||||
"pytest-rerunfailures",
|
||||
"pandas",
|
||||
"plotly",
|
||||
]
|
||||
|
||||
Reference in New Issue
Block a user