Skip to content

fix(ci): enable auto-approve for infrastructure-only PRs #222

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
eric-wang-1990 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(ci): enable auto-approve for infrastructure-only PRs #222

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fc70946
fix(ci): enable auto-approve for infrastructure-only PRs
eric-wang-1990 Feb 5, 2026
6adaacb
Address PR review feedback
eric-wang-1990 Feb 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 48 additions & 17 deletions .github/workflows/trigger-integration-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
name: Trigger Integration Tests
on:
pull_request:
types: [labeled, synchronize] # Trigger on label add or new commits
types: [opened, synchronize, labeled] # Trigger on PR open, new commits, or label add
merge_group: # Trigger when added to merge queue
jobs:
# =============================================================================
Expand Down Expand Up @@ -140,13 +140,14 @@ jobs:
body: '🚀 Integration tests triggered! [View workflow run](https://github.com/databricks/databricks-driver-test/actions)'
});
# =============================================================================
# For Merge Queue: Check if relevant files changed
# Check if relevant files changed (for both PR and Merge Queue)
# =============================================================================
check-merge-queue-paths:
if: github.event_name == 'merge_group'
check-paths:
if: github.event_name == 'pull_request' || github.event_name == 'merge_group'
runs-on: [self-hosted, Linux, X64, peco-driver]
outputs:
should-test: ${{ steps.check-paths.outputs.should_test }}
head-sha: ${{ steps.check-paths.outputs.head_sha }}
steps:
- name: Checkout code
uses: actions/checkout@v4
Expand All @@ -155,17 +156,28 @@ jobs:
- name: Check if relevant files changed
id: check-paths
run: |
# Get the base and head commits for the merge queue
BASE_SHA="${{ github.event.merge_group.base_sha }}"
HEAD_SHA="${{ github.event.merge_group.head_sha }}"
# Get the base and head commits based on event type
if [ "${{ github.event_name }}" = "merge_group" ]; then
BASE_SHA="${{ github.event.merge_group.base_sha }}"
HEAD_SHA="${{ github.event.merge_group.head_sha }}"
echo "Event: merge_group"
else
BASE_SHA="${{ github.event.pull_request.base.sha }}"
HEAD_SHA="${{ github.event.pull_request.head.sha }}"
echo "Event: pull_request"
fi

echo "Checking files changed between $BASE_SHA and $HEAD_SHA"
# Define paths to check
echo "head_sha=$HEAD_SHA" >> $GITHUB_OUTPUT

# Define paths to check - only driver code paths
PATHS=(
".github/workflows/trigger-integration-tests.yml"
"ci/scripts/"
"csharp/src/"
"csharp/test/"
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

The PR description states that "go/" and "rust/src/" and "rust/examples/" were added to the paths that trigger integration tests, but the PATHS array only includes ".github/workflows/trigger-integration-tests.yml", "ci/scripts/", "csharp/src/", and "csharp/test/".

The go/ and rust/ directories exist in the repository and contain driver code that should trigger integration tests. These paths need to be added to the PATHS array to match the documented behavior and ensure that changes to Go and Rust driver code trigger integration tests appropriately.

Suggested change
"csharp/test/"
"csharp/test/"
"go/"
"rust/src/"
"rust/examples/"

Copilot uses AI. Check for mistakes.
)
Comment on lines 174 to 179
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

The PR description claims "Added more driver paths: go/, rust/src/, rust/examples/" in the "Unified path checking" changes section, but these paths are not actually added to the PATHS array in the code. This is a significant discrepancy between the PR description and the actual implementation.

Either the paths need to be added to match the description, or the description needs to be updated to reflect that these paths were not added in this PR.

Copilot uses AI. Check for mistakes.

# Check if any relevant files changed
CHANGED=false
for path in "${PATHS[@]}"; do
Expand All @@ -175,19 +187,20 @@ jobs:
break
fi
done

if [ "$CHANGED" = true ]; then
echo "should_test=true" >> $GITHUB_OUTPUT
echo "✅ Relevant files changed - will run tests"
echo "✅ Relevant driver files changed - will run tests"
else
echo "should_test=false" >> $GITHUB_OUTPUT
echo "⏭️ No relevant files changed - skipping tests"
echo "⏭️ No driver files changed - skipping tests"
fi
# =============================================================================
# For Merge Queue: Check if tests already passed on PR
# =============================================================================
check-previous-run:
needs: check-merge-queue-paths
if: github.event_name == 'merge_group' && needs.check-merge-queue-paths.outputs.should-test == 'true'
needs: check-paths
if: github.event_name == 'merge_group' && needs.check-paths.outputs.should-test == 'true'
runs-on: [self-hosted, Linux, X64, peco-driver]
outputs:
already-passed: ${{ steps.check-pr.outputs.passed }}
Expand Down Expand Up @@ -289,8 +302,10 @@ jobs:
"pr_author": "merge-queue"
}
auto-approve-no-relevant-changes:
needs: check-merge-queue-paths
if: github.event_name == 'merge_group' && needs.check-merge-queue-paths.outputs.should-test == 'false'
needs: check-paths
if: |
(github.event_name == 'pull_request' || github.event_name == 'merge_group') &&
needs.check-paths.outputs.should-test == 'false'
runs-on: [self-hosted, Linux, X64, peco-driver]
steps:
- name: Generate GitHub App Token
Expand All @@ -306,19 +321,35 @@ jobs:
with:
github-token: ${{ steps.app-token.outputs.token }}
script: |
const eventType = '${{ github.event_name }}';
const isPR = eventType === 'pull_request';

await github.rest.checks.create({
owner: context.repo.owner,
repo: context.repo.repo,
name: 'Integration Tests',
head_sha: '${{ github.event.merge_group.head_sha }}',
head_sha: '${{ needs.check-paths.outputs.head-sha }}',
status: 'completed',
conclusion: 'success',
completed_at: new Date().toISOString(),
output: {
title: 'Auto-approved - no relevant changes',
summary: '✅ No integration test files changed. Skipping tests.'
title: 'Auto-approved - no driver code changes',
summary: `✅ No driver code changed. Skipping integration tests.\n\n` +
`**Changed files**: Infrastructure/documentation only\n` +
`**Event**: ${isPR ? 'Pull Request' : 'Merge Queue'}`
}
});

// Add a comment on PR for visibility
if (isPR) {
await github.rest.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: context.issue.number,
body: '✅ **Integration tests auto-approved**\n\n' +
'No driver code changes detected. Your changes only affect infrastructure/documentation, so tests are skipped.'
});
Comment on lines +343 to +351
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

The auto-approve comment on line 345-351 could be created multiple times if the workflow is re-run for the same PR. This could spam the PR with duplicate "Integration tests auto-approved" comments.

Consider adding logic to check if a comment with this message already exists before creating a new one. You can search for existing comments and either skip creation or update the existing comment instead of creating duplicates.

Suggested change
// Add a comment on PR for visibility
if (isPR) {
await github.rest.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: context.issue.number,
body: '✅ **Integration tests auto-approved**\n\n' +
'No driver code changes detected. Your changes only affect infrastructure/documentation, so tests are skipped.'
});
// Add a comment on PR for visibility (avoid duplicates)
if (isPR) {
const commentBody =
'✅ **Integration tests auto-approved**\n\n' +
'No driver code changes detected. Your changes only affect infrastructure/documentation, so tests are skipped.';
// Check if an identical comment already exists to avoid spamming the PR
const { data: existingComments } = await github.rest.issues.listComments({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: context.issue.number,
per_page: 100,
});
const alreadyCommented = existingComments.some(
(comment) => comment.body === commentBody
);
if (!alreadyCommented) {
await github.rest.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: context.issue.number,
body: commentBody,
});
}

Copilot uses AI. Check for mistakes.
}
auto-approve-already-passed:
needs: check-previous-run
if: needs.check-previous-run.outputs.already-passed == 'true'
Expand Down
Loading