Skip to content

[CI/Build] Move test scripts and assets from .github/ to tests/ (part of #531) - #1024

Merged
ruizhang0101 merged 1 commit into
vllm-project:mainfrom
ighutake-debug:ci/move-test-assets-to-tests
Jul 30, 2026
Merged

ruizhang0101 merged 1 commit into
vllm-project:mainfrom
ighutake-debug:ci/move-test-assets-to-tests

Conversation

@ighutake-debug

@ighutake-debug ighutake-debug commented Jul 28, 2026 •

Copy link
Copy Markdown
Contributor

Implements item 1 ("Move the scripts and assets from .github/ to `tests/") of the CI refactor plan agreed in #531 (comment).

Part of #531 — this PR covers only item 1 of 4; follow-up PRs will address the fake-server, job-consolidation, and Helm de-duplication items, so merging this should NOT close the issue.

What changed

Moved 15 loose CI files out of .github/:

  • tests/scripts/: curl-02-two-pods.sh, curl-04-multiple-models.sh, curl-05-secure-vllm.sh, curl-06-monitoring.sh, port-forward.sh
  • tests/assets/: values-01/04/05/06/07/08/09/10/11-*.yaml, template-chatml.jinja

Updated every referencing call site:

  • .github/workflows/functionality-helm-chart.yml — helm install -f values paths + port-forward.sh invocations
  • .github/workflows/router-e2e-test.yml — --chat-template path for the static-discovery backends
  • tests/e2e/run-k8s-routing-test.sh — HELM_VALUES_FILE paths (10 references)
  • tests/scripts/port-forward.sh — path to the curl scripts it dispatches

Also added tests/assets/** and tests/scripts/** to the paths filters of both workflows so changes to the moved files keep triggering CI.

No functional changes — pure move + reference updates (all 14 content files are 100% renames; only port-forward.sh has a 1-line path edit).

Validation done locally

  • grep sweep: zero remaining references to the old .github/ paths
  • bash -n passes on all moved/edited shell scripts
  • YAML parse passes on both workflows and all moved values files
  • pre-commit run on all changed files: actionlint, check-yaml, end-of-file, trailing-whitespace, codespell all pass

The self-hosted runner jobs (functionality-helm-chart.yml, router-e2e-test.yml) will exercise the moved scripts end-to-end on this PR's CI run.

BEFORE SUBMITTING, PLEASE READ THE CHECKLIST BELOW AND FILL IN THE DESCRIPTION ABOVE


  • Make sure the code changes pass the pre-commit checks.
  • Sign-off your commit by using -s when doing git commit
  • Try to classify PRs for easy understanding of the type of changes, such as [Bugfix], [Feat], and [CI].

… of vllm-project#531)

Moves the 15 loose CI files (curl-*.sh, values-*.yaml, port-forward.sh,
template-chatml.jinja) out of .github/ into tests/scripts/ and
tests/assets/, and updates all referencing call sites:

- .github/workflows/functionality-helm-chart.yml (helm -f, port-forward.sh)
- .github/workflows/router-e2e-test.yml (chat-template path)
- tests/e2e/run-k8s-routing-test.sh (HELM_VALUES_FILE paths)
- tests/scripts/port-forward.sh (curl script path)

Also adds tests/assets/** and tests/scripts/** to the path filters of
both workflows so changes to the moved files keep triggering CI.

No functional changes; pure move + reference updates.

Signed-off-by: ighutake-debug <253169138+ighutake-debug@users.noreply.github.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates the paths in the end-to-end test and port-forwarding scripts to reflect the relocation of test assets and scripts from the .github/ directory to tests/assets/ and tests/scripts/. The review feedback suggests using $(dirname "$0") in port-forward.sh to make the script execution location-independent instead of hardcoding the relative path.

sleep 5

bash ".github/$1.sh" "$ip" "$port"
bash "tests/scripts/$1.sh" "$ip" "$port"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Since port-forward.sh and the target scripts (e.g., curl-*.sh) are now located in the same directory (tests/scripts/), we can avoid hardcoding the relative path tests/scripts/ by using $(dirname "$0"). This makes the script location-independent and allows it to be run successfully from any working directory (including from within tests/scripts/ itself).

Suggested change
bash "tests/scripts/$1.sh" "$ip" "$port"
bash "$(dirname "$0")/$1.sh" "$ip" "$port"

@ighutake-debug

Copy link
Copy Markdown
Contributor Author

Hey @ruizhang0101 — PR 1 is up as discussed in #531. Kept it strictly mechanical (pure moves + path updates; every content file is a 100% rename), so review should be quick. The self-hosted e2e jobs will exercise the moved scripts end-to-end on this run — I'll keep an eye on CI and fix anything that shakes out.

@ighutake-debug

Copy link
Copy Markdown
Contributor Author

Follow-up for the record: the PR-time pre-commit.ci failure appears to have been a service-side runner error — on main, the repo's own pre-commit workflow passes with these changes included, and all four functionality-helm-chart jobs (which exercise the moved values files and port-forward.sh) are green. Nothing outstanding from this PR; @zerofishnoodles happy to adjust anything if you'd like changes in a follow-up.

@ruizhang0101 ruizhang0101 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@ruizhang0101
ruizhang0101 enabled auto-merge (squash) July 29, 2026 19:30
@ighutake-debug

Copy link
Copy Markdown
Contributor Author

Status update and a correction:

  1. Correction of my earlier note — I previously said the change was green on main; that run actually predated/excluded this change (main was later reset), so it did not validate these files. Apologies for the confusion.

  2. Current CI on the reopened PR is almost fully green: pre-commit, shellcheck, checkov, helmlint, Python tests, CRD validation, both router discovery e2e suites, Secure-Minimal-Example, Minimal-Example-With-Monitoring, DCO, and the RTD build all pass.

  3. The single failure (Two-Pods-Minimal-Example) does not look like a defect in this change. The step fails with helm install vllm ./helm -f tests/assets/values-01-2pods-minimal-example.yaml: no such file or directory. However, the sibling jobs in the same workflow run (Secure-Minimal-Example, Minimal-Example-With-Monitoring) opened their moved files at identical tests/assets/ paths successfully, so the files are present in the checkout — this looks like a stale workspace on the shared self-hosted runner for that one job.

@zerofishnoodles @ruizhang0101 — could you re-run just the failed Two-Pods job when convenient? Once it goes green, I'd appreciate a review whenever you have a moment. Thanks!

@ruizhang0101
ruizhang0101 merged commit 3314ee6 into vllm-project:main Jul 30, 2026
23 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants