mirror of
https://github.com/meta-llama/llama-stack.git
synced 2025-12-21 16:08:40 +00:00
fix: run all clients on stainless SDK, fix workflow, properly commit recordings (#4410)
# What does this PR do?
Various fixes to integration test recording + stainless calling of
integration tests:
1. only the library client was being run, they all should be
2. the git check grabs diffs like:
M tests/integration/client-typescript/package-lock.json
M tests/integration/client-typescript/package.json
it should not
additionally:
Fixes rebase conflicts when stainless workflow runs integration tests
with
record-if-missing mode on PRs. Previously, the workflow would:
1. Commit all files in tests/integration/ (including non-recordings)
2. Try to rebase and push to 'main' instead of the PR branch
3. Fail with merge conflicts on PR-specific changes
Changes:
- Add pr_head_ref and is_fork_pr parameters flowing through workflow
chain
- Use target-branch input instead of github.ref_name in recording
commits
- Detect and handle fork PRs by skipping push and uploading recordings
as artifacts
- Add 7-day artifact retention for fork PR recordings
- Support both workflow_call and direct pull_request trigger contexts
For same-repo PRs: recordings now commit/push to the PR branch correctly
For fork PRs: recordings upload as downloadable artifacts with
instructions
you can see a failing workflow:
5846590613
with the rebase issues.
---------
Signed-off-by: Charlie Doern <cdoern@redhat.com>
This commit is contained in:
parent
5d52cb28c2
commit
e710622d4c
4 changed files with 62 additions and 11 deletions
54
.github/actions/run-and-record-tests/action.yml
vendored
54
.github/actions/run-and-record-tests/action.yml
vendored
|
|
@ -24,6 +24,14 @@ inputs:
|
||||||
description: 'Regex pattern to pass to pytest -k'
|
description: 'Regex pattern to pass to pytest -k'
|
||||||
required: false
|
required: false
|
||||||
default: ''
|
default: ''
|
||||||
|
target-branch:
|
||||||
|
description: 'Target branch for recording commits (for PRs, use the PR head branch)'
|
||||||
|
required: false
|
||||||
|
default: ''
|
||||||
|
is-fork-pr:
|
||||||
|
description: 'Whether this is a fork PR (recordings cannot be pushed to forks)'
|
||||||
|
required: false
|
||||||
|
default: 'false'
|
||||||
|
|
||||||
runs:
|
runs:
|
||||||
using: 'composite'
|
using: 'composite'
|
||||||
|
|
@ -66,23 +74,49 @@ runs:
|
||||||
shell: bash
|
shell: bash
|
||||||
run: |
|
run: |
|
||||||
echo "Checking for recording changes"
|
echo "Checking for recording changes"
|
||||||
git status --porcelain tests/integration/
|
git status --porcelain tests/integration/recordings/ tests/integration/*/recordings/
|
||||||
|
|
||||||
if [[ -n $(git status --porcelain tests/integration/) ]]; then
|
if [[ -n $(git status --porcelain tests/integration/recordings/ tests/integration/*/recordings/) ]]; then
|
||||||
echo "New recordings detected, committing and pushing"
|
echo "New recordings detected"
|
||||||
git add tests/integration/
|
|
||||||
|
|
||||||
git commit -m "Recordings update from CI (setup: ${{ inputs.setup }}, suite: ${{ inputs.suite }})"
|
# Determine target branch: use target-branch input if provided, otherwise use current branch
|
||||||
|
TARGET_BRANCH="${{ inputs.target-branch }}"
|
||||||
|
if [ -z "$TARGET_BRANCH" ]; then
|
||||||
|
TARGET_BRANCH="${{ github.ref_name }}"
|
||||||
|
fi
|
||||||
|
echo "Target branch: $TARGET_BRANCH"
|
||||||
|
|
||||||
git fetch origin ${{ github.ref_name }}
|
# Check if this is a fork PR
|
||||||
git rebase origin/${{ github.ref_name }}
|
if [ "${{ inputs.is-fork-pr }}" = "true" ]; then
|
||||||
echo "Rebased successfully"
|
echo "::warning::This is a fork PR. Recordings were updated locally but cannot be pushed to the fork."
|
||||||
git push origin HEAD:${{ github.ref_name }}
|
echo "::warning::Please download the workflow artifacts and commit the recordings manually."
|
||||||
echo "Pushed successfully"
|
else
|
||||||
|
echo "Committing and pushing recordings to branch: $TARGET_BRANCH"
|
||||||
|
git add tests/integration/recordings/ tests/integration/*/recordings/
|
||||||
|
|
||||||
|
git commit -m "Recordings update from CI (setup: ${{ inputs.setup }}, suite: ${{ inputs.suite }})"
|
||||||
|
|
||||||
|
git fetch origin "$TARGET_BRANCH"
|
||||||
|
git rebase "origin/$TARGET_BRANCH"
|
||||||
|
echo "Rebased successfully"
|
||||||
|
git push origin "HEAD:$TARGET_BRANCH"
|
||||||
|
echo "Pushed successfully to $TARGET_BRANCH"
|
||||||
|
fi
|
||||||
else
|
else
|
||||||
echo "No recording changes"
|
echo "No recording changes"
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
- name: Upload recordings (for fork PRs)
|
||||||
|
if: ${{ inputs.is-fork-pr == 'true' && (inputs.inference-mode == 'record' || inputs.inference-mode == 'record-if-missing') }}
|
||||||
|
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
|
||||||
|
with:
|
||||||
|
name: recordings-${{ github.run_id }}-${{ github.run_attempt || '1' }}-${{ strategy.job-index || github.job }}
|
||||||
|
path: |
|
||||||
|
tests/integration/recordings/
|
||||||
|
tests/integration/*/recordings/
|
||||||
|
retention-days: 7
|
||||||
|
if-no-files-found: ignore
|
||||||
|
|
||||||
- name: Write docker logs to file
|
- name: Write docker logs to file
|
||||||
if: ${{ always() }}
|
if: ${{ always() }}
|
||||||
shell: bash
|
shell: bash
|
||||||
|
|
|
||||||
11
.github/workflows/integration-tests.yml
vendored
11
.github/workflows/integration-tests.yml
vendored
|
|
@ -52,6 +52,15 @@ on:
|
||||||
required: false
|
required: false
|
||||||
type: string
|
type: string
|
||||||
description: 'The SHA of the pull request head to checkout'
|
description: 'The SHA of the pull request head to checkout'
|
||||||
|
pr_head_ref:
|
||||||
|
required: false
|
||||||
|
type: string
|
||||||
|
description: 'The branch name of the pull request head (for recording commits)'
|
||||||
|
is_fork_pr:
|
||||||
|
required: false
|
||||||
|
type: boolean
|
||||||
|
default: false
|
||||||
|
description: 'Whether this is a fork PR (cannot push recordings to forks)'
|
||||||
test-all-client-versions:
|
test-all-client-versions:
|
||||||
required: false
|
required: false
|
||||||
type: boolean
|
type: boolean
|
||||||
|
|
@ -150,3 +159,5 @@ jobs:
|
||||||
setup: ${{ matrix.config.setup }}
|
setup: ${{ matrix.config.setup }}
|
||||||
inference-mode: ${{ matrix.config.inference_mode || 'replay' }}
|
inference-mode: ${{ matrix.config.inference_mode || 'replay' }}
|
||||||
suite: ${{ matrix.config.suite }}
|
suite: ${{ matrix.config.suite }}
|
||||||
|
target-branch: ${{ inputs.pr_head_ref || '' }}
|
||||||
|
is-fork-pr: ${{ inputs.is_fork_pr && 'true' || (github.event.pull_request.head.repo.full_name != github.repository && 'true' || 'false') }}
|
||||||
|
|
|
||||||
6
.github/workflows/stainless-builds.yml
vendored
6
.github/workflows/stainless-builds.yml
vendored
|
|
@ -66,6 +66,7 @@ jobs:
|
||||||
pr_base_sha: ${{ steps.compute.outputs.pr_base_sha }}
|
pr_base_sha: ${{ steps.compute.outputs.pr_base_sha }}
|
||||||
pr_base_ref: ${{ steps.compute.outputs.pr_base_ref }}
|
pr_base_ref: ${{ steps.compute.outputs.pr_base_ref }}
|
||||||
pr_title: ${{ steps.compute.outputs.pr_title }}
|
pr_title: ${{ steps.compute.outputs.pr_title }}
|
||||||
|
is_fork_pr: ${{ steps.compute.outputs.is_fork_pr }}
|
||||||
steps:
|
steps:
|
||||||
- name: Fetch PR details for workflow_dispatch
|
- name: Fetch PR details for workflow_dispatch
|
||||||
if: github.event_name == 'workflow_dispatch'
|
if: github.event_name == 'workflow_dispatch'
|
||||||
|
|
@ -111,10 +112,12 @@ jobs:
|
||||||
fi
|
fi
|
||||||
PREVIEW_BRANCH="preview/${FORK_OWNER}/${BRANCH_NAME}"
|
PREVIEW_BRANCH="preview/${FORK_OWNER}/${BRANCH_NAME}"
|
||||||
BASE_BRANCH="preview/base/${FORK_OWNER}/${BRANCH_NAME}"
|
BASE_BRANCH="preview/base/${FORK_OWNER}/${BRANCH_NAME}"
|
||||||
|
IS_FORK_PR="true"
|
||||||
else
|
else
|
||||||
# Same-repo PR
|
# Same-repo PR
|
||||||
PREVIEW_BRANCH="preview/${BRANCH_NAME}"
|
PREVIEW_BRANCH="preview/${BRANCH_NAME}"
|
||||||
BASE_BRANCH="preview/base/${BRANCH_NAME}"
|
BASE_BRANCH="preview/base/${BRANCH_NAME}"
|
||||||
|
IS_FORK_PR="false"
|
||||||
fi
|
fi
|
||||||
|
|
||||||
echo "preview_branch=${PREVIEW_BRANCH}" >> $GITHUB_OUTPUT
|
echo "preview_branch=${PREVIEW_BRANCH}" >> $GITHUB_OUTPUT
|
||||||
|
|
@ -126,6 +129,7 @@ jobs:
|
||||||
echo "pr_base_sha=${BASE_SHA}" >> $GITHUB_OUTPUT
|
echo "pr_base_sha=${BASE_SHA}" >> $GITHUB_OUTPUT
|
||||||
echo "pr_base_ref=${BASE_REF}" >> $GITHUB_OUTPUT
|
echo "pr_base_ref=${BASE_REF}" >> $GITHUB_OUTPUT
|
||||||
echo "pr_title=${PR_TITLE}" >> $GITHUB_OUTPUT
|
echo "pr_title=${PR_TITLE}" >> $GITHUB_OUTPUT
|
||||||
|
echo "is_fork_pr=${IS_FORK_PR}" >> $GITHUB_OUTPUT
|
||||||
|
|
||||||
preview:
|
preview:
|
||||||
needs: compute-branch
|
needs: compute-branch
|
||||||
|
|
@ -182,6 +186,8 @@ jobs:
|
||||||
matrix_key: 'stainless'
|
matrix_key: 'stainless'
|
||||||
test-all-client-versions: false
|
test-all-client-versions: false
|
||||||
pr_head_sha: ${{ needs.compute-branch.outputs.pr_head_sha }}
|
pr_head_sha: ${{ needs.compute-branch.outputs.pr_head_sha }}
|
||||||
|
pr_head_ref: ${{ needs.compute-branch.outputs.pr_head_ref }}
|
||||||
|
is_fork_pr: ${{ needs.compute-branch.outputs.is_fork_pr == 'true' }}
|
||||||
|
|
||||||
merge:
|
merge:
|
||||||
needs: compute-branch
|
needs: compute-branch
|
||||||
|
|
|
||||||
|
|
@ -7,7 +7,7 @@
|
||||||
{"suite": "base-vllm-subset", "setup": "vllm"}
|
{"suite": "base-vllm-subset", "setup": "vllm"}
|
||||||
],
|
],
|
||||||
"stainless": [
|
"stainless": [
|
||||||
{"suite": "base", "setup": "ollama", "allowed_clients": ["library"], "inference_mode": "record-if-missing"}
|
{"suite": "base", "setup": "ollama", "inference_mode": "record-if-missing"}
|
||||||
],
|
],
|
||||||
"schedules": {
|
"schedules": {
|
||||||
"1 0 * * 0": [
|
"1 0 * * 0": [
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue