fix: address QA findings on detect-pipeline workflow
- Fix subdirectory matching: use prefix match for .github/* paths instead of exact dirname match (fixes .github/workflows/ not matching) - Upgrade tj-actions/changed-files from v44 to v47 (Node 24 support) - Extract detection logic into scripts/detect-pipeline.sh for testability - Add 22 automated tests in scripts/test-detect-pipeline.sh covering infra-only, plugin code, mixed, and edge cases - Add test-detection-logic CI job to run tests on every PR - Update README.md to reference v47 cc @cpfarhood Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
committed by
Countess von Containerheim [agent]
parent
2706245b03
commit
25fe4107e6
@@ -68,7 +68,7 @@ Use `tj-actions/changed-files`:
|
|||||||
```yaml
|
```yaml
|
||||||
- name: Get changed files
|
- name: Get changed files
|
||||||
id: changed-files
|
id: changed-files
|
||||||
uses: tj-actions/changed-files@v44
|
uses: tj-actions/changed-files@v47
|
||||||
with:
|
with:
|
||||||
files_separator: '\n'
|
files_separator: '\n'
|
||||||
```
|
```
|
||||||
|
|||||||
@@ -10,6 +10,16 @@ permissions:
|
|||||||
pull-requests: write
|
pull-requests: write
|
||||||
|
|
||||||
jobs:
|
jobs:
|
||||||
|
test-detection-logic:
|
||||||
|
runs-on: runners-privilegedescalation
|
||||||
|
timeout-minutes: 2
|
||||||
|
steps:
|
||||||
|
- name: Checkout
|
||||||
|
uses: actions/checkout@v6
|
||||||
|
|
||||||
|
- name: Run detection tests
|
||||||
|
run: bash scripts/test-detect-pipeline.sh
|
||||||
|
|
||||||
detect-pipeline:
|
detect-pipeline:
|
||||||
runs-on: runners-privilegedescalation
|
runs-on: runners-privilegedescalation
|
||||||
timeout-minutes: 5
|
timeout-minutes: 5
|
||||||
@@ -24,7 +34,7 @@ jobs:
|
|||||||
|
|
||||||
- name: Get changed files
|
- name: Get changed files
|
||||||
id: changed-files
|
id: changed-files
|
||||||
uses: tj-actions/changed-files@v44
|
uses: tj-actions/changed-files@v47
|
||||||
with:
|
with:
|
||||||
files_separator: '\n'
|
files_separator: '\n'
|
||||||
|
|
||||||
@@ -34,34 +44,7 @@ jobs:
|
|||||||
echo "Changed files:"
|
echo "Changed files:"
|
||||||
echo "${{ steps.changed-files.outputs.all_changed_files }}"
|
echo "${{ steps.changed-files.outputs.all_changed_files }}"
|
||||||
|
|
||||||
pipeline="pipeline-a"
|
pipeline=$(echo "${{ steps.changed-files.outputs.all_changed_files }}" | bash scripts/detect-pipeline.sh)
|
||||||
|
|
||||||
if [ -n "${{ steps.changed-files.outputs.all_changed_files }}" ]; then
|
|
||||||
all_infra=true
|
|
||||||
while IFS= read -r file; do
|
|
||||||
filename=$(basename "$file")
|
|
||||||
dirname=$(dirname "$file")
|
|
||||||
|
|
||||||
if [ "$dirname" = ".github" ] || \
|
|
||||||
[[ "$filename" == *.md ]] || \
|
|
||||||
[[ "$filename" == .eslintrc* ]] || \
|
|
||||||
[[ "$filename" == .prettierrc* ]] || \
|
|
||||||
[[ "$filename" == renovate.json* ]] || \
|
|
||||||
[[ "$filename" == .gitignore ]] || \
|
|
||||||
[[ "$filename" == .editorconfig ]] || \
|
|
||||||
[[ "$filename" == LICENSE ]]; then
|
|
||||||
continue
|
|
||||||
else
|
|
||||||
all_infra=false
|
|
||||||
echo "Non-infra file found: $file"
|
|
||||||
break
|
|
||||||
fi
|
|
||||||
done <<< "${{ steps.changed-files.outputs.all_changed_files }}"
|
|
||||||
|
|
||||||
if [ "$all_infra" = true ]; then
|
|
||||||
pipeline="pipeline-b"
|
|
||||||
fi
|
|
||||||
fi
|
|
||||||
|
|
||||||
echo "pipeline-type=$pipeline" >> $GITHUB_OUTPUT
|
echo "pipeline-type=$pipeline" >> $GITHUB_OUTPUT
|
||||||
echo "Detected pipeline: $pipeline"
|
echo "Detected pipeline: $pipeline"
|
||||||
|
|||||||
Executable
+44
@@ -0,0 +1,44 @@
|
|||||||
|
#!/usr/bin/env bash
|
||||||
|
set -euo pipefail
|
||||||
|
|
||||||
|
# Reads a newline-separated list of changed files from stdin.
|
||||||
|
# Outputs "pipeline-a" or "pipeline-b" to stdout.
|
||||||
|
# Pipeline B: all files are infra-only (config, docs, workflows).
|
||||||
|
# Pipeline A: any non-infra file present.
|
||||||
|
|
||||||
|
detect_pipeline() {
|
||||||
|
local all_infra=true
|
||||||
|
|
||||||
|
while IFS= read -r file; do
|
||||||
|
[ -z "$file" ] && continue
|
||||||
|
|
||||||
|
local filename
|
||||||
|
local dir
|
||||||
|
filename=$(basename "$file")
|
||||||
|
dir=$(dirname "$file")
|
||||||
|
|
||||||
|
if [[ "$dir" == ".github" || "$dir" == .github/* ]] || \
|
||||||
|
[[ "$filename" == *.md ]] || \
|
||||||
|
[[ "$filename" == .eslintrc* ]] || \
|
||||||
|
[[ "$filename" == .prettierrc* ]] || \
|
||||||
|
[[ "$filename" == renovate.json* ]] || \
|
||||||
|
[[ "$filename" == .gitignore ]] || \
|
||||||
|
[[ "$filename" == .editorconfig ]] || \
|
||||||
|
[[ "$filename" == LICENSE ]]; then
|
||||||
|
continue
|
||||||
|
else
|
||||||
|
all_infra=false
|
||||||
|
break
|
||||||
|
fi
|
||||||
|
done
|
||||||
|
|
||||||
|
if [ "$all_infra" = true ]; then
|
||||||
|
echo "pipeline-b"
|
||||||
|
else
|
||||||
|
echo "pipeline-a"
|
||||||
|
fi
|
||||||
|
}
|
||||||
|
|
||||||
|
if [ "${BASH_SOURCE[0]}" = "$0" ]; then
|
||||||
|
detect_pipeline
|
||||||
|
fi
|
||||||
Executable
+110
@@ -0,0 +1,110 @@
|
|||||||
|
#!/usr/bin/env bash
|
||||||
|
set -euo pipefail
|
||||||
|
|
||||||
|
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
||||||
|
source "$SCRIPT_DIR/detect-pipeline.sh"
|
||||||
|
|
||||||
|
PASS=0
|
||||||
|
FAIL=0
|
||||||
|
|
||||||
|
assert_eq() {
|
||||||
|
local test_name="$1" expected="$2" actual="$3"
|
||||||
|
if [ "$expected" = "$actual" ]; then
|
||||||
|
echo "PASS: $test_name"
|
||||||
|
PASS=$((PASS + 1))
|
||||||
|
else
|
||||||
|
echo "FAIL: $test_name (expected=$expected, actual=$actual)"
|
||||||
|
FAIL=$((FAIL + 1))
|
||||||
|
fi
|
||||||
|
}
|
||||||
|
|
||||||
|
run_detect() {
|
||||||
|
echo "$1" | detect_pipeline
|
||||||
|
}
|
||||||
|
|
||||||
|
# --- Pipeline B cases (infra-only) ---
|
||||||
|
|
||||||
|
assert_eq "single .github root file" "pipeline-b" \
|
||||||
|
"$(run_detect ".github/dependabot.yml")"
|
||||||
|
|
||||||
|
assert_eq ".github/workflows subdirectory" "pipeline-b" \
|
||||||
|
"$(run_detect ".github/workflows/ci.yaml")"
|
||||||
|
|
||||||
|
assert_eq "deeply nested .github path" "pipeline-b" \
|
||||||
|
"$(run_detect ".github/workflows/reusable/build.yaml")"
|
||||||
|
|
||||||
|
assert_eq "markdown file at root" "pipeline-b" \
|
||||||
|
"$(run_detect "README.md")"
|
||||||
|
|
||||||
|
assert_eq "markdown in subdirectory" "pipeline-b" \
|
||||||
|
"$(run_detect "docs/CONTRIBUTING.md")"
|
||||||
|
|
||||||
|
assert_eq "eslintrc config" "pipeline-b" \
|
||||||
|
"$(run_detect ".eslintrc.json")"
|
||||||
|
|
||||||
|
assert_eq "prettierrc config" "pipeline-b" \
|
||||||
|
"$(run_detect ".prettierrc.yaml")"
|
||||||
|
|
||||||
|
assert_eq "renovate config" "pipeline-b" \
|
||||||
|
"$(run_detect "renovate.json")"
|
||||||
|
|
||||||
|
assert_eq "renovate config5" "pipeline-b" \
|
||||||
|
"$(run_detect "renovate.json5")"
|
||||||
|
|
||||||
|
assert_eq "gitignore" "pipeline-b" \
|
||||||
|
"$(run_detect ".gitignore")"
|
||||||
|
|
||||||
|
assert_eq "editorconfig" "pipeline-b" \
|
||||||
|
"$(run_detect ".editorconfig")"
|
||||||
|
|
||||||
|
assert_eq "LICENSE" "pipeline-b" \
|
||||||
|
"$(run_detect "LICENSE")"
|
||||||
|
|
||||||
|
assert_eq "mixed infra files" "pipeline-b" \
|
||||||
|
"$(run_detect ".github/workflows/ci.yaml
|
||||||
|
README.md
|
||||||
|
.eslintrc.json
|
||||||
|
LICENSE")"
|
||||||
|
|
||||||
|
assert_eq "workflow + markdown combo" "pipeline-b" \
|
||||||
|
"$(run_detect ".github/workflows/detect-pr-pipeline.yaml
|
||||||
|
.github/workflows/README.md")"
|
||||||
|
|
||||||
|
# --- Pipeline A cases (has non-infra files) ---
|
||||||
|
|
||||||
|
assert_eq "plugin source file" "pipeline-a" \
|
||||||
|
"$(run_detect "headlamp-polaris-plugin/src/index.tsx")"
|
||||||
|
|
||||||
|
assert_eq "plugin package.json" "pipeline-a" \
|
||||||
|
"$(run_detect "headlamp-polaris-plugin/package.json")"
|
||||||
|
|
||||||
|
assert_eq "root source file" "pipeline-a" \
|
||||||
|
"$(run_detect "src/main.ts")"
|
||||||
|
|
||||||
|
assert_eq "mixed infra + code" "pipeline-a" \
|
||||||
|
"$(run_detect ".github/workflows/ci.yaml
|
||||||
|
headlamp-polaris-plugin/src/index.tsx
|
||||||
|
README.md")"
|
||||||
|
|
||||||
|
assert_eq "single non-infra file" "pipeline-a" \
|
||||||
|
"$(run_detect "server.js")"
|
||||||
|
|
||||||
|
# --- Edge cases ---
|
||||||
|
|
||||||
|
assert_eq "empty input" "pipeline-b" \
|
||||||
|
"$(run_detect "")"
|
||||||
|
|
||||||
|
assert_eq "root dot file (not in infra list)" "pipeline-a" \
|
||||||
|
"$(run_detect ".env")"
|
||||||
|
|
||||||
|
assert_eq ".github-like but not .github dir" "pipeline-a" \
|
||||||
|
"$(run_detect ".github-backup/config.yaml")"
|
||||||
|
|
||||||
|
# --- Summary ---
|
||||||
|
|
||||||
|
echo ""
|
||||||
|
echo "Results: $PASS passed, $FAIL failed"
|
||||||
|
|
||||||
|
if [ "$FAIL" -gt 0 ]; then
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
Reference in New Issue
Block a user