From d473d27a97da1ab136a7afbe53fe48d039c854f3 Mon Sep 17 00:00:00 2001 From: lml2468 Date: Mon, 1 Jun 2026 16:12:56 +0800 Subject: [PATCH 1/5] ci: add reusable Go + Node test quality workflows Phase 1 of test quality CI improvements: - reusable-go-quality.yml: gotestsum + JUnit XML + coverage + octocov diff coverage + time.Sleep test lint - reusable-node-quality.yml: typecheck + lint + test (optional) + build for pnpm/npm projects These reusable workflows will be called by individual repo CI pipelines. --- .github/workflows/reusable-go-quality.yml | 417 ++++++++++++++++++++ .github/workflows/reusable-node-quality.yml | 190 +++++++++ 2 files changed, 607 insertions(+) create mode 100644 .github/workflows/reusable-go-quality.yml create mode 100644 .github/workflows/reusable-node-quality.yml diff --git a/.github/workflows/reusable-go-quality.yml b/.github/workflows/reusable-go-quality.yml new file mode 100644 index 0000000..9c3400a --- /dev/null +++ b/.github/workflows/reusable-go-quality.yml @@ -0,0 +1,417 @@ +# Reusable workflow: Go Test Quality +# +# Provides gotestsum-based testing with JUnit XML + coverage output, +# diff coverage via octocov, and test lint (time.Sleep detection). +# +# Usage: +# jobs: +# quality: +# uses: Mininglamp-OSS/.github/.github/workflows/reusable-go-quality.yml@main +# with: +# go-version: '1.25.x' +# services: true +# core-packages: './modules/user/...,./modules/thread/...' + +name: Reusable — Go Test Quality + +on: + workflow_call: + inputs: + go-version: + description: 'Go version to use' + required: true + type: string + services: + description: 'Start MySQL + Redis service containers' + required: false + type: boolean + default: false + coverage-threshold: + description: 'Overall coverage threshold (%)' + required: false + type: number + default: 60 + core-packages: + description: 'Core package paths (comma-separated, e.g. ./modules/user/...,./modules/thread/...)' + required: false + type: string + default: '' + core-coverage-threshold: + description: 'Core packages coverage threshold (%)' + required: false + type: number + default: 75 + coverage-exclude: + description: 'Exclude paths from coverage (comma-separated glob fragments)' + required: false + type: string + default: 'mock,pb.go,_gen.go,vendor,migrations' + +permissions: + contents: read + pull-requests: write + +jobs: + # ── Test (with MySQL + Redis services) ────────────────────────────── + # GitHub Actions does not support conditional `services:` blocks, so we + # define two mutually-exclusive test jobs gated by `inputs.services`. + test-with-services: + if: inputs.services + name: Test (services) + runs-on: ubuntu-latest + services: + mysql: + image: mysql:8.0 + env: + MYSQL_ROOT_PASSWORD: demo + MYSQL_DATABASE: test + ports: + - 3306:3306 + options: >- + --health-cmd="mysqladmin ping -h 127.0.0.1 -uroot -pdemo" + --health-interval=5s + --health-timeout=5s + --health-retries=20 + redis: + image: redis:7-alpine + ports: + - 6379:6379 + options: >- + --health-cmd="redis-cli ping" + --health-interval=5s + --health-timeout=5s + --health-retries=20 + env: + OCTO_MASTER_KEY: 0123456789abcdef0123456789abcdef + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-go@v5 + with: + go-version: ${{ inputs.go-version }} + + - name: Install gotestsum + run: go install gotest.tools/gotestsum@latest + + - name: Install MySQL/Redis client tooling + run: | + sudo apt-get update -qq + sudo apt-get install -y -qq mysql-client redis-tools + + - name: Wait for MySQL + run: | + for _ in $(seq 1 30); do + if mysqladmin ping -h 127.0.0.1 -uroot -pdemo --silent; then + echo "mysql ready" + exit 0 + fi + sleep 2 + done + echo "mysql did not become ready" >&2 + exit 1 + + - name: Run tests (per-package with DB reset) + id: test + run: | + set -euo pipefail + fail=0 + echo "mode: atomic" > coverage.out + pkg_index=0 + total_pass=0 + total_fail=0 + total_skip=0 + + while read -r pkg; do + mysql -h 127.0.0.1 -uroot -pdemo -e \ + "DROP DATABASE IF EXISTS test; CREATE DATABASE test CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci;" + redis-cli -h 127.0.0.1 -p 6379 FLUSHALL >/dev/null + + echo "::group::testing $pkg" + if ! gotestsum \ + --junitfile "junit-${pkg_index}.xml" \ + --format short-verbose \ + -- -race -shuffle=on -count=1 -timeout 5m \ + -coverprofile="coverage-${pkg_index}.out" \ + -covermode=atomic "$pkg"; then + fail=1 + fi + + # Accumulate coverage data (skip mode line from each fragment) + if [ -f "coverage-${pkg_index}.out" ]; then + grep -v '^mode:' "coverage-${pkg_index}.out" >> coverage.out || true + fi + + pkg_index=$((pkg_index + 1)) + echo "::endgroup::" + done < <(go list ./...) + + # Merge JUnit XMLs + if compgen -G "junit-*.xml" > /dev/null; then + gotestsum tool merge junit-*.xml > junit.xml 2>/dev/null || { + # Fallback: use the first available XML + first_xml=$(ls junit-*.xml 2>/dev/null | head -1) + [ -n "$first_xml" ] && cp "$first_xml" junit.xml + } + fi + + # Parse test counts from JUnit XML for summary + if [ -f junit.xml ]; then + total_pass=$(grep -o 'tests="[0-9]*"' junit.xml | head -1 | grep -o '[0-9]*' || echo 0) + total_fail=$(grep -o 'failures="[0-9]*"' junit.xml | head -1 | grep -o '[0-9]*' || echo 0) + total_skip=$(grep -o 'skipped="[0-9]*"' junit.xml | head -1 | grep -o '[0-9]*' || echo 0) + fi + + echo "test_passed=${total_pass}" >> "$GITHUB_OUTPUT" + echo "test_failed=${total_fail}" >> "$GITHUB_OUTPUT" + echo "test_skipped=${total_skip}" >> "$GITHUB_OUTPUT" + echo "test_exit_code=${fail}" >> "$GITHUB_OUTPUT" + + exit $fail + + - name: Upload test artifacts + if: always() + uses: actions/upload-artifact@v4 + with: + name: test-results + path: | + junit.xml + junit-*.xml + coverage.out + retention-days: 14 + + - name: Write PR summary + if: always() + run: | + passed="${{ steps.test.outputs.test_passed }}" + failed="${{ steps.test.outputs.test_failed }}" + skipped="${{ steps.test.outputs.test_skipped }}" + exit_code="${{ steps.test.outputs.test_exit_code }}" + + if [ "$exit_code" = "0" ]; then + verdict="✅ **PASSED**" + else + verdict="❌ **FAILED**" + fi + + cat >> "$GITHUB_STEP_SUMMARY" <> "$GITHUB_OUTPUT" + echo "test_failed=${total_fail}" >> "$GITHUB_OUTPUT" + echo "test_skipped=${total_skip}" >> "$GITHUB_OUTPUT" + + - name: Upload test artifacts + if: always() + uses: actions/upload-artifact@v4 + with: + name: test-results + path: | + junit.xml + coverage.out + retention-days: 14 + + - name: Write PR summary + if: always() + run: | + passed="${{ steps.test.outputs.test_passed }}" + failed="${{ steps.test.outputs.test_failed }}" + skipped="${{ steps.test.outputs.test_skipped }}" + outcome="${{ steps.test.outcome }}" + + if [ "$outcome" = "success" ]; then + verdict="✅ **PASSED**" + else + verdict="❌ **FAILED**" + fi + + cat >> "$GITHUB_STEP_SUMMARY" < .octocov.yml <> "$GITHUB_STEP_SUMMARY" + echo "" >> "$GITHUB_STEP_SUMMARY" + + IFS=',' read -ra CORE_PKGS <<< "${{ inputs.core-packages }}" + threshold=${{ inputs.core-coverage-threshold }} + all_pass=true + + for pkg in "${CORE_PKGS[@]}"; do + pkg=$(echo "$pkg" | xargs) + [ -z "$pkg" ] && continue + + # Extract coverage for this package from coverage.out + # Use go tool cover to get the percentage + pkg_coverage=$(go tool cover -func=coverage.out 2>/dev/null | grep "^${pkg}" | tail -1 | awk '{print $NF}' | tr -d '%' || echo "0") + + if [ -z "$pkg_coverage" ] || [ "$pkg_coverage" = "0" ]; then + # Try broader match + pkg_base=$(echo "$pkg" | sed 's|/\.\.\.$||') + pkg_coverage=$(go tool cover -func=coverage.out 2>/dev/null | grep "${pkg_base}" | awk '{sum+=$NF; n++} END {if(n>0) printf "%.1f", sum/n; else print "0"}' || echo "0") + fi + + if awk "BEGIN {exit !($pkg_coverage < $threshold)}"; then + echo "| ❌ | \`$pkg\` | ${pkg_coverage}% | >=${threshold}% |" >> "$GITHUB_STEP_SUMMARY" + all_pass=false + else + echo "| ✅ | \`$pkg\` | ${pkg_coverage}% | >=${threshold}% |" >> "$GITHUB_STEP_SUMMARY" + fi + done + + if [ "$all_pass" = false ]; then + echo "" >> "$GITHUB_STEP_SUMMARY" + echo "⚠️ Some core packages are below the ${threshold}% coverage threshold." >> "$GITHUB_STEP_SUMMARY" + fi + + # ── Test Lint (time.Sleep detection) ──────────────────────────────── + test-lint: + name: Test Lint + runs-on: ubuntu-latest + continue-on-error: true + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-go@v5 + with: + go-version: ${{ inputs.go-version }} + + - name: Detect time.Sleep in tests + run: | + echo "## 🔍 Test Lint: time.Sleep Detection" >> "$GITHUB_STEP_SUMMARY" + echo "" >> "$GITHUB_STEP_SUMMARY" + + found=0 + while IFS= read -r -d '' file; do + matches=$(grep -n 'time\.Sleep' "$file" 2>/dev/null || true) + if [ -n "$matches" ]; then + found=1 + echo "::warning file=${file}::time.Sleep detected in test file — consider using a clock abstraction or test helper" + echo "- \`${file}\`:" >> "$GITHUB_STEP_SUMMARY" + while IFS= read -r line; do + echo " - ${line}" >> "$GITHUB_STEP_SUMMARY" + done <<< "$matches" + fi + done < <(find . -name '*_test.go' -not -path './vendor/*' -print0) + + if [ "$found" -eq 0 ]; then + echo "✅ No \`time.Sleep\` found in test files." >> "$GITHUB_STEP_SUMMARY" + else + echo "" >> "$GITHUB_STEP_SUMMARY" + echo "⚠️ \`time.Sleep\` in tests can cause flakiness. Consider using clock abstractions or polling helpers." >> "$GITHUB_STEP_SUMMARY" + fi diff --git a/.github/workflows/reusable-node-quality.yml b/.github/workflows/reusable-node-quality.yml new file mode 100644 index 0000000..b05e0bf --- /dev/null +++ b/.github/workflows/reusable-node-quality.yml @@ -0,0 +1,190 @@ +# Reusable workflow: Node Test Quality +# +# Provides typecheck, lint, test (optional), and build jobs for Node.js +# projects using pnpm or npm. +# +# Usage: +# jobs: +# quality: +# uses: Mininglamp-OSS/.github/.github/workflows/reusable-node-quality.yml@main +# with: +# package-manager: pnpm +# node-version: '20' +# has-tests: false + +name: Reusable — Node Test Quality + +on: + workflow_call: + inputs: + package-manager: + description: 'Package manager: pnpm or npm' + required: false + type: string + default: 'pnpm' + node-version: + description: 'Node.js version' + required: false + type: string + default: '20' + working-directory: + description: 'Working directory for all commands' + required: false + type: string + default: '.' + has-tests: + description: 'Whether the project has tests to run' + required: false + type: boolean + default: false + +permissions: + contents: read + pull-requests: write + +jobs: + # ── Type Check ────────────────────────────────────────────────────── + typecheck: + name: Type Check + runs-on: ubuntu-latest + defaults: + run: + working-directory: ${{ inputs.working-directory }} + steps: + - uses: actions/checkout@v4 + + - uses: pnpm/action-setup@v4 + if: inputs.package-manager == 'pnpm' + with: + run_install: false + + - uses: actions/setup-node@v4 + with: + node-version: ${{ inputs.node-version }} + cache: ${{ inputs.package-manager == 'pnpm' && 'pnpm' || 'npm' }} + + - name: Install dependencies + run: | + if [ "${{ inputs.package-manager }}" = "pnpm" ]; then + pnpm install --frozen-lockfile + else + npm ci + fi + + - name: Type check + run: npx tsc --noEmit + + # ── Lint ──────────────────────────────────────────────────────────── + lint: + name: Lint + runs-on: ubuntu-latest + defaults: + run: + working-directory: ${{ inputs.working-directory }} + steps: + - uses: actions/checkout@v4 + + - uses: pnpm/action-setup@v4 + if: inputs.package-manager == 'pnpm' + with: + run_install: false + + - uses: actions/setup-node@v4 + with: + node-version: ${{ inputs.node-version }} + cache: ${{ inputs.package-manager == 'pnpm' && 'pnpm' || 'npm' }} + + - name: Install dependencies + run: | + if [ "${{ inputs.package-manager }}" = "pnpm" ]; then + pnpm install --frozen-lockfile + else + npm ci + fi + + - name: Lint + run: | + if [ "${{ inputs.package-manager }}" = "pnpm" ]; then + pnpm lint + else + npm run lint + fi + + # ── Test (only when has-tests is true) ────────────────────────────── + test: + if: inputs.has-tests + name: Test + runs-on: ubuntu-latest + defaults: + run: + working-directory: ${{ inputs.working-directory }} + steps: + - uses: actions/checkout@v4 + + - uses: pnpm/action-setup@v4 + if: inputs.package-manager == 'pnpm' + with: + run_install: false + + - uses: actions/setup-node@v4 + with: + node-version: ${{ inputs.node-version }} + cache: ${{ inputs.package-manager == 'pnpm' && 'pnpm' || 'npm' }} + + - name: Install dependencies + run: | + if [ "${{ inputs.package-manager }}" = "pnpm" ]; then + pnpm install --frozen-lockfile + else + npm ci + fi + + - name: Run tests with coverage + run: | + if [ "${{ inputs.package-manager }}" = "pnpm" ]; then + pnpm test --coverage + else + npm test -- --coverage + fi + + - name: Coverage report + if: always() && github.event_name == 'pull_request' + uses: davelosert/vitest-coverage-report-action@v2 + with: + working-directory: ${{ inputs.working-directory }} + + # ── Build ─────────────────────────────────────────────────────────── + build: + name: Build + runs-on: ubuntu-latest + defaults: + run: + working-directory: ${{ inputs.working-directory }} + steps: + - uses: actions/checkout@v4 + + - uses: pnpm/action-setup@v4 + if: inputs.package-manager == 'pnpm' + with: + run_install: false + + - uses: actions/setup-node@v4 + with: + node-version: ${{ inputs.node-version }} + cache: ${{ inputs.package-manager == 'pnpm' && 'pnpm' || 'npm' }} + + - name: Install dependencies + run: | + if [ "${{ inputs.package-manager }}" = "pnpm" ]; then + pnpm install --frozen-lockfile + else + npm ci + fi + + - name: Build + run: | + if [ "${{ inputs.package-manager }}" = "pnpm" ]; then + pnpm build + else + npm run build + fi From 8a6c3837c69c4ddd715379723197b6b3983c11db Mon Sep 17 00:00:00 2001 From: lml2468 Date: Mon, 1 Jun 2026 18:35:03 +0800 Subject: [PATCH 2/5] fix: address PR #55 review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P0 — shellcheck/actionlint fixes: - SC2012: replace `ls | head` with `find` for JUnit XML fallback - SC2129: group multiple `echo >> GITHUB_OUTPUT` into single block - SC2001: replace `sed` with parameter expansion for pkg_base P1-1: core package coverage now enforced with `exit 1` P1-2: coverage-exclude patterns now written into .octocov.yml exclude section P1-3: replace non-existent `gotestsum tool merge` with Python XML merger P2-1: all actions pinned to commit SHA (actions/checkout, setup-go, setup-node, pnpm/action-setup, upload-artifact, download-artifact, k1LoW/octocov-action, davelosert/vitest-coverage-report-action) P2-2: permissions moved from top-level to per-job; top-level set to `{}` only diff-coverage and test (node, for coverage report) get pull-requests: write --- .github/workflows/reusable-go-quality.yml | 153 +++++++++++++------- .github/workflows/reusable-node-quality.yml | 39 +++-- 2 files changed, 123 insertions(+), 69 deletions(-) diff --git a/.github/workflows/reusable-go-quality.yml b/.github/workflows/reusable-go-quality.yml index 9c3400a..e902165 100644 --- a/.github/workflows/reusable-go-quality.yml +++ b/.github/workflows/reusable-go-quality.yml @@ -47,9 +47,7 @@ on: type: string default: 'mock,pb.go,_gen.go,vendor,migrations' -permissions: - contents: read - pull-requests: write +permissions: {} jobs: # ── Test (with MySQL + Redis services) ────────────────────────────── @@ -59,6 +57,8 @@ jobs: if: inputs.services name: Test (services) runs-on: ubuntu-latest + permissions: + contents: read services: mysql: image: mysql:8.0 @@ -84,9 +84,9 @@ jobs: env: OCTO_MASTER_KEY: 0123456789abcdef0123456789abcdef steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - - uses: actions/setup-go@v5 + - uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5 with: go-version: ${{ inputs.go-version }} @@ -145,13 +145,39 @@ jobs: echo "::endgroup::" done < <(go list ./...) - # Merge JUnit XMLs + # Merge JUnit XMLs using Python (gotestsum has no merge subcommand) if compgen -G "junit-*.xml" > /dev/null; then - gotestsum tool merge junit-*.xml > junit.xml 2>/dev/null || { - # Fallback: use the first available XML - first_xml=$(ls junit-*.xml 2>/dev/null | head -1) - [ -n "$first_xml" ] && cp "$first_xml" junit.xml - } + python3 - <<'PYEOF' + import xml.etree.ElementTree as ET, glob, sys + + xmls = sorted(glob.glob('junit-*.xml')) + if not xmls: + sys.exit(0) + + root = ET.parse(xmls[0]).getroot() + if root.tag != 'testsuites': + wrapper = ET.Element('testsuites') + wrapper.append(root) + root = wrapper + + for xml in xmls[1:]: + tree = ET.parse(xml).getroot() + if tree.tag == 'testsuites': + for child in tree: + root.append(child) + else: + root.append(tree) + + tests = sum(int(ts.get('tests', 0)) for ts in root.findall('testsuite')) + failures = sum(int(ts.get('failures', 0)) for ts in root.findall('testsuite')) + skipped = sum(int(ts.get('skipped', 0)) for ts in root.findall('testsuite')) + root.set('tests', str(tests)) + root.set('failures', str(failures)) + root.set('skipped', str(skipped)) + + ET.ElementTree(root).write('junit.xml', xml_declaration=True, encoding='utf-8') + print(f'Merged {len(xmls)} JUnit XMLs: {tests} tests, {failures} failures, {skipped} skipped') + PYEOF fi # Parse test counts from JUnit XML for summary @@ -161,16 +187,18 @@ jobs: total_skip=$(grep -o 'skipped="[0-9]*"' junit.xml | head -1 | grep -o '[0-9]*' || echo 0) fi - echo "test_passed=${total_pass}" >> "$GITHUB_OUTPUT" - echo "test_failed=${total_fail}" >> "$GITHUB_OUTPUT" - echo "test_skipped=${total_skip}" >> "$GITHUB_OUTPUT" - echo "test_exit_code=${fail}" >> "$GITHUB_OUTPUT" + { + echo "test_passed=${total_pass}" + echo "test_failed=${total_fail}" + echo "test_skipped=${total_skip}" + echo "test_exit_code=${fail}" + } >> "$GITHUB_OUTPUT" exit $fail - name: Upload test artifacts if: always() - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 with: name: test-results path: | @@ -214,10 +242,12 @@ jobs: if: ${{ !inputs.services }} name: Test runs-on: ubuntu-latest + permissions: + contents: read steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - - uses: actions/setup-go@v5 + - uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5 with: go-version: ${{ inputs.go-version }} @@ -241,13 +271,15 @@ jobs: total_fail=$(grep -o 'failures="[0-9]*"' junit.xml | head -1 | grep -o '[0-9]*' || echo 0) total_skip=$(grep -o 'skipped="[0-9]*"' junit.xml | head -1 | grep -o '[0-9]*' || echo 0) - echo "test_passed=${total_pass}" >> "$GITHUB_OUTPUT" - echo "test_failed=${total_fail}" >> "$GITHUB_OUTPUT" - echo "test_skipped=${total_skip}" >> "$GITHUB_OUTPUT" + { + echo "test_passed=${total_pass}" + echo "test_failed=${total_fail}" + echo "test_skipped=${total_skip}" + } >> "$GITHUB_OUTPUT" - name: Upload test artifacts if: always() - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 with: name: test-results path: | @@ -294,53 +326,65 @@ jobs: (needs.test-with-services.result == 'success' || needs.test-standalone.result == 'success') name: Diff Coverage runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: write steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: fetch-depth: 0 - - uses: actions/setup-go@v5 + - uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5 with: go-version: ${{ inputs.go-version }} - name: Download test results - uses: actions/download-artifact@v4 + uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4 with: name: test-results - name: Generate octocov config run: | # Build exclude list from comma-separated input - EXCLUDES="" - IFS=',' read -ra PARTS <<< "${{ inputs.coverage-exclude }}" - for part in "${PARTS[@]}"; do - part=$(echo "$part" | xargs) # trim whitespace - if [ -n "$part" ]; then - EXCLUDES="${EXCLUDES} - '**/*${part}*'\n" + EXCLUDE_YAML="" + IFS=',' read -ra PATTERNS <<< "${{ inputs.coverage-exclude }}" + for p in "${PATTERNS[@]}"; do + p="${p// /}" + [ -z "$p" ] && continue + # If ends with / (directory), use **/p**; otherwise use **/*p + if [[ "$p" == */ ]]; then + EXCLUDE_YAML="${EXCLUDE_YAML} - \"**/${p}**\"\n" + else + EXCLUDE_YAML="${EXCLUDE_YAML} - \"**/*${p}\"\n" fi done - cat > .octocov.yml < .octocov.yml echo "Generated .octocov.yml:" cat .octocov.yml - name: Run octocov - uses: k1LoW/octocov-action@v1 + uses: k1LoW/octocov-action@b3b6ee60482a667950f87553abf1df63217235d9 # v1 - name: Core packages coverage check if: inputs.core-packages != '' @@ -361,8 +405,8 @@ jobs: pkg_coverage=$(go tool cover -func=coverage.out 2>/dev/null | grep "^${pkg}" | tail -1 | awk '{print $NF}' | tr -d '%' || echo "0") if [ -z "$pkg_coverage" ] || [ "$pkg_coverage" = "0" ]; then - # Try broader match - pkg_base=$(echo "$pkg" | sed 's|/\.\.\.$||') + # Try broader match — use parameter expansion instead of sed (SC2001) + pkg_base="${pkg%/...}" pkg_coverage=$(go tool cover -func=coverage.out 2>/dev/null | grep "${pkg_base}" | awk '{sum+=$NF; n++} END {if(n>0) printf "%.1f", sum/n; else print "0"}' || echo "0") fi @@ -374,20 +418,23 @@ jobs: fi done - if [ "$all_pass" = false ]; then + if [ "$all_pass" = "false" ]; then echo "" >> "$GITHUB_STEP_SUMMARY" - echo "⚠️ Some core packages are below the ${threshold}% coverage threshold." >> "$GITHUB_STEP_SUMMARY" + echo "❌ One or more core packages are below the ${threshold}% threshold" >> "$GITHUB_STEP_SUMMARY" + exit 1 fi # ── Test Lint (time.Sleep detection) ──────────────────────────────── test-lint: name: Test Lint runs-on: ubuntu-latest + permissions: + contents: read continue-on-error: true steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - - uses: actions/setup-go@v5 + - uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5 with: go-version: ${{ inputs.go-version }} diff --git a/.github/workflows/reusable-node-quality.yml b/.github/workflows/reusable-node-quality.yml index b05e0bf..7862ed7 100644 --- a/.github/workflows/reusable-node-quality.yml +++ b/.github/workflows/reusable-node-quality.yml @@ -38,27 +38,27 @@ on: type: boolean default: false -permissions: - contents: read - pull-requests: write +permissions: {} jobs: # ── Type Check ────────────────────────────────────────────────────── typecheck: name: Type Check runs-on: ubuntu-latest + permissions: + contents: read defaults: run: working-directory: ${{ inputs.working-directory }} steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - - uses: pnpm/action-setup@v4 + - uses: pnpm/action-setup@b906affcce14559ad1aafd4ab0e942779e9f58b1 # v4 if: inputs.package-manager == 'pnpm' with: run_install: false - - uses: actions/setup-node@v4 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: ${{ inputs.node-version }} cache: ${{ inputs.package-manager == 'pnpm' && 'pnpm' || 'npm' }} @@ -78,18 +78,20 @@ jobs: lint: name: Lint runs-on: ubuntu-latest + permissions: + contents: read defaults: run: working-directory: ${{ inputs.working-directory }} steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - - uses: pnpm/action-setup@v4 + - uses: pnpm/action-setup@b906affcce14559ad1aafd4ab0e942779e9f58b1 # v4 if: inputs.package-manager == 'pnpm' with: run_install: false - - uses: actions/setup-node@v4 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: ${{ inputs.node-version }} cache: ${{ inputs.package-manager == 'pnpm' && 'pnpm' || 'npm' }} @@ -115,18 +117,21 @@ jobs: if: inputs.has-tests name: Test runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: write defaults: run: working-directory: ${{ inputs.working-directory }} steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - - uses: pnpm/action-setup@v4 + - uses: pnpm/action-setup@b906affcce14559ad1aafd4ab0e942779e9f58b1 # v4 if: inputs.package-manager == 'pnpm' with: run_install: false - - uses: actions/setup-node@v4 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: ${{ inputs.node-version }} cache: ${{ inputs.package-manager == 'pnpm' && 'pnpm' || 'npm' }} @@ -149,7 +154,7 @@ jobs: - name: Coverage report if: always() && github.event_name == 'pull_request' - uses: davelosert/vitest-coverage-report-action@v2 + uses: davelosert/vitest-coverage-report-action@02f3c2e641286b7fa308cd3e430783103ce6103b # v2 with: working-directory: ${{ inputs.working-directory }} @@ -157,18 +162,20 @@ jobs: build: name: Build runs-on: ubuntu-latest + permissions: + contents: read defaults: run: working-directory: ${{ inputs.working-directory }} steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - - uses: pnpm/action-setup@v4 + - uses: pnpm/action-setup@b906affcce14559ad1aafd4ab0e942779e9f58b1 # v4 if: inputs.package-manager == 'pnpm' with: run_install: false - - uses: actions/setup-node@v4 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: ${{ inputs.node-version }} cache: ${{ inputs.package-manager == 'pnpm' && 'pnpm' || 'npm' }} From ad5d63ba40013454a0a0fdff3cd8cea90fb9e111 Mon Sep 17 00:00:00 2001 From: lml2468 Date: Mon, 1 Jun 2026 19:37:16 +0800 Subject: [PATCH 3/5] fix: address all blocking/warning issues from PR #55 review B2: Fix go list false-green by capturing output to temp file B3: Rewrite core packages coverage with Python parser B4: Add quality-gate job to both Go and Node workflows B5: Fix inputs injection risk via env variables (Node workflow) W1: Disable PR comment for fork PRs in octocov config W2: Fix coverage-exclude glob patterns (both dir and suffix) W9: Pin gotestsum to v1.12.0 W12: Add cache-dependency-path to all setup-node steps Nit-1: Fix Go summary Passed count calculation Nit-3: Add table header to core coverage summary Nit-4: Remove hardcoded OCTO_MASTER_KEY --- .github/workflows/reusable-go-quality.yml | 207 +++++++++++++++----- .github/workflows/reusable-node-quality.yml | 97 ++++++++- 2 files changed, 249 insertions(+), 55 deletions(-) diff --git a/.github/workflows/reusable-go-quality.yml b/.github/workflows/reusable-go-quality.yml index e902165..30a1874 100644 --- a/.github/workflows/reusable-go-quality.yml +++ b/.github/workflows/reusable-go-quality.yml @@ -81,8 +81,6 @@ jobs: --health-interval=5s --health-timeout=5s --health-retries=20 - env: - OCTO_MASTER_KEY: 0123456789abcdef0123456789abcdef steps: - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 @@ -91,7 +89,7 @@ jobs: go-version: ${{ inputs.go-version }} - name: Install gotestsum - run: go install gotest.tools/gotestsum@latest + run: go install gotest.tools/gotestsum@v1.12.0 - name: Install MySQL/Redis client tooling run: | @@ -121,6 +119,17 @@ jobs: total_fail=0 total_skip=0 + # B2: Capture go list output safely; fail if it errors + pkg_file="$(mktemp)" + if ! go list ./... > "$pkg_file"; then + echo "::error::go list ./... failed" >&2 + exit 1 + fi + if [ ! -s "$pkg_file" ]; then + echo "::error::no Go packages found" >&2 + exit 1 + fi + while read -r pkg; do mysql -h 127.0.0.1 -uroot -pdemo -e \ "DROP DATABASE IF EXISTS test; CREATE DATABASE test CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci;" @@ -143,7 +152,8 @@ jobs: pkg_index=$((pkg_index + 1)) echo "::endgroup::" - done < <(go list ./...) + done < "$pkg_file" + rm -f "$pkg_file" # Merge JUnit XMLs using Python (gotestsum has no merge subcommand) if compgen -G "junit-*.xml" > /dev/null; then @@ -180,11 +190,12 @@ jobs: PYEOF fi - # Parse test counts from JUnit XML for summary + # Parse test counts from JUnit XML for summary (Nit-1: correct Passed count) if [ -f junit.xml ]; then - total_pass=$(grep -o 'tests="[0-9]*"' junit.xml | head -1 | grep -o '[0-9]*' || echo 0) + total_tests=$(grep -o 'tests="[0-9]*"' junit.xml | head -1 | grep -o '[0-9]*' || echo 0) total_fail=$(grep -o 'failures="[0-9]*"' junit.xml | head -1 | grep -o '[0-9]*' || echo 0) total_skip=$(grep -o 'skipped="[0-9]*"' junit.xml | head -1 | grep -o '[0-9]*' || echo 0) + total_pass=$((total_tests - total_fail - total_skip)) fi { @@ -252,7 +263,7 @@ jobs: go-version: ${{ inputs.go-version }} - name: Install gotestsum - run: go install gotest.tools/gotestsum@latest + run: go install gotest.tools/gotestsum@v1.12.0 - name: Run tests id: test @@ -266,10 +277,11 @@ jobs: -coverprofile=coverage.out \ -covermode=atomic ./... - # Parse test counts - total_pass=$(grep -o 'tests="[0-9]*"' junit.xml | head -1 | grep -o '[0-9]*' || echo 0) + # Parse test counts (Nit-1: correct Passed count) + total_tests=$(grep -o 'tests="[0-9]*"' junit.xml | head -1 | grep -o '[0-9]*' || echo 0) total_fail=$(grep -o 'failures="[0-9]*"' junit.xml | head -1 | grep -o '[0-9]*' || echo 0) total_skip=$(grep -o 'skipped="[0-9]*"' junit.xml | head -1 | grep -o '[0-9]*' || echo 0) + total_pass=$((total_tests - total_fail - total_skip)) { echo "test_passed=${total_pass}" @@ -344,19 +356,31 @@ jobs: name: test-results - name: Generate octocov config + env: + COVERAGE_EXCLUDE: ${{ inputs.coverage-exclude }} + COVERAGE_THRESHOLD: ${{ inputs.coverage-threshold }} run: | - # Build exclude list from comma-separated input + # W1: Disable PR comment for fork PRs (token lacks pull-requests: write) + IS_FORK="false" + if [ "${{ github.event.pull_request.head.repo.full_name }}" != "${{ github.repository }}" ]; then + IS_FORK="true" + fi + + COMMENT_ENABLE="true" + if [ "$IS_FORK" = "true" ]; then + COMMENT_ENABLE="false" + fi + + # W2: Build exclude list from comma-separated input EXCLUDE_YAML="" - IFS=',' read -ra PATTERNS <<< "${{ inputs.coverage-exclude }}" + IFS=',' read -ra PATTERNS <<< "$COVERAGE_EXCLUDE" for p in "${PATTERNS[@]}"; do p="${p// /}" [ -z "$p" ] && continue - # If ends with / (directory), use **/p**; otherwise use **/*p - if [[ "$p" == */ ]]; then - EXCLUDE_YAML="${EXCLUDE_YAML} - \"**/${p}**\"\n" - else - EXCLUDE_YAML="${EXCLUDE_YAML} - \"**/*${p}\"\n" - fi + # Directory patterns (vendor, migrations, mock as dir) + EXCLUDE_YAML="${EXCLUDE_YAML} - \"**/${p}/**\"\n" + # File suffix patterns (pb.go, _gen.go, _mock.go) + EXCLUDE_YAML="${EXCLUDE_YAML} - \"**/*${p}\"\n" done # Write octocov config with exclude paths @@ -365,16 +389,16 @@ jobs: echo "coverage:" echo " paths:" echo " - coverage.out" - echo " acceptable: ${{ inputs.coverage-threshold }}%" + echo " acceptable: ${COVERAGE_THRESHOLD}%" if [ -n "$EXCLUDE_YAML" ]; then echo " exclude:" printf '%b' "$EXCLUDE_YAML" fi echo "diff:" echo " coverage:" - echo " acceptable: ${{ inputs.coverage-threshold }}%" + echo " acceptable: ${COVERAGE_THRESHOLD}%" echo "comment:" - echo " enable: true" + echo " enable: $COMMENT_ENABLE" echo " hideFooterLink: false" echo "summary:" echo " enable: true" @@ -388,41 +412,91 @@ jobs: - name: Core packages coverage check if: inputs.core-packages != '' + env: + CORE_PACKAGES: ${{ inputs.core-packages }} + CORE_THRESHOLD: ${{ inputs.core-coverage-threshold }} run: | - echo "## 📊 Core Packages Coverage" >> "$GITHUB_STEP_SUMMARY" - echo "" >> "$GITHUB_STEP_SUMMARY" + python3 - <<'PYEOF' + import sys, os, subprocess - IFS=',' read -ra CORE_PKGS <<< "${{ inputs.core-packages }}" - threshold=${{ inputs.core-coverage-threshold }} - all_pass=true + core_pkgs_input = os.environ.get("CORE_PACKAGES", "") + threshold = float(os.environ.get("CORE_THRESHOLD", "75")) + coverage_file = "coverage.out" - for pkg in "${CORE_PKGS[@]}"; do - pkg=$(echo "$pkg" | xargs) - [ -z "$pkg" ] && continue + if not core_pkgs_input.strip(): + sys.exit(0) - # Extract coverage for this package from coverage.out - # Use go tool cover to get the percentage - pkg_coverage=$(go tool cover -func=coverage.out 2>/dev/null | grep "^${pkg}" | tail -1 | awk '{print $NF}' | tr -d '%' || echo "0") + # Expand each core package pattern using go list + core_import_paths = set() + for pkg_pattern in core_pkgs_input.split(","): + pkg_pattern = pkg_pattern.strip() + if not pkg_pattern: + continue + result = subprocess.run( + ["go", "list", "-f", "{{.ImportPath}}", pkg_pattern], + capture_output=True, text=True + ) + if result.returncode == 0: + for line in result.stdout.strip().splitlines(): + line = line.strip() + if line: + core_import_paths.add(line) + + if not core_import_paths: + print("Warning: no packages matched core-packages patterns") + sys.exit(0) - if [ -z "$pkg_coverage" ] || [ "$pkg_coverage" = "0" ]; then - # Try broader match — use parameter expansion instead of sed (SC2001) - pkg_base="${pkg%/...}" - pkg_coverage=$(go tool cover -func=coverage.out 2>/dev/null | grep "${pkg_base}" | awk '{sum+=$NF; n++} END {if(n>0) printf "%.1f", sum/n; else print "0"}' || echo "0") - fi + # Parse coverage.out: format is "pkg/path/file.go:start.col,end.col stmts count" + total_stmts = 0 + covered_stmts = 0 + + with open(coverage_file) as f: + for line in f: + line = line.strip() + if line.startswith("mode:") or not line: + continue + # e.g. github.com/org/repo/pkg/file.go:10.5,12.10 3 1 + try: + file_part, rest = line.rsplit(":", 1) + # file_part is import_path/filename, extract import path (dir) + import_path = "/".join(file_part.split("/")[:-1]) + parts = rest.split() + # rest = "10.5,12.10 3 1" -> parts = ["10.5,12.10", "3", "1"] + stmts = int(parts[1]) if len(parts) >= 2 else 0 + count = int(parts[2]) if len(parts) >= 3 else 0 + except (ValueError, IndexError): + continue + + if import_path in core_import_paths: + total_stmts += stmts + if count > 0: + covered_stmts += stmts + + summary_path = os.environ.get("GITHUB_STEP_SUMMARY", "/dev/stderr") + + with open(summary_path, "a") as f: + f.write("## 📊 Core Packages Coverage\n\n") + f.write("| Status | Package | Coverage | Required |\n") + f.write("|--------|---------|----------|----------|\n") + + if total_stmts == 0: + print("Warning: no statements found for core packages — check import paths") + with open(summary_path, "a") as f: + f.write("| ⚠️ | (all core packages) | N/A — no statements found | check patterns |\n") + sys.exit(0) - if awk "BEGIN {exit !($pkg_coverage < $threshold)}"; then - echo "| ❌ | \`$pkg\` | ${pkg_coverage}% | >=${threshold}% |" >> "$GITHUB_STEP_SUMMARY" - all_pass=false - else - echo "| ✅ | \`$pkg\` | ${pkg_coverage}% | >=${threshold}% |" >> "$GITHUB_STEP_SUMMARY" - fi - done + coverage_pct = covered_stmts / total_stmts * 100 + status = "✅" if coverage_pct >= threshold else "❌" - if [ "$all_pass" = "false" ]; then - echo "" >> "$GITHUB_STEP_SUMMARY" - echo "❌ One or more core packages are below the ${threshold}% threshold" >> "$GITHUB_STEP_SUMMARY" - exit 1 - fi + with open(summary_path, "a") as f: + f.write(f"| {status} | Core packages ({len(core_import_paths)} pkgs) | {coverage_pct:.1f}% | >={threshold:.0f}% |\n") + + print(f"Core coverage: {coverage_pct:.1f}% ({covered_stmts}/{total_stmts} statements), threshold: {threshold}%") + + if coverage_pct < threshold: + print(f"::error::Core packages coverage {coverage_pct:.1f}% is below threshold {threshold}%") + sys.exit(1) + PYEOF # ── Test Lint (time.Sleep detection) ──────────────────────────────── test-lint: @@ -462,3 +536,40 @@ jobs: echo "" >> "$GITHUB_STEP_SUMMARY" echo "⚠️ \`time.Sleep\` in tests can cause flakiness. Consider using clock abstractions or polling helpers." >> "$GITHUB_STEP_SUMMARY" fi + + # ── Quality Gate (stable required check for branch protection) ────── + quality-gate: + name: Quality Gate + runs-on: ubuntu-latest + if: always() + needs: [test-with-services, test-standalone, diff-coverage, test-lint] + permissions: + contents: read + steps: + - name: Evaluate results + run: | + # Determine which test job ran + if [ "${{ inputs.services }}" = "true" ]; then + test_result="${{ needs.test-with-services.result }}" + else + test_result="${{ needs.test-standalone.result }}" + fi + + echo "test result: $test_result" + echo "diff-coverage result: ${{ needs.diff-coverage.result }}" + + if [ "$test_result" != "success" ]; then + echo "::error::Test job did not pass (result: $test_result)" + exit 1 + fi + + # diff-coverage only runs on PR; skip check for push events + if [ "${{ github.event_name }}" = "pull_request" ]; then + cov_result="${{ needs.diff-coverage.result }}" + if [ "$cov_result" != "success" ] && [ "$cov_result" != "skipped" ]; then + echo "::error::Diff coverage check did not pass (result: $cov_result)" + exit 1 + fi + fi + + echo "✅ All quality gates passed" diff --git a/.github/workflows/reusable-node-quality.yml b/.github/workflows/reusable-node-quality.yml index 7862ed7..cd221f9 100644 --- a/.github/workflows/reusable-node-quality.yml +++ b/.github/workflows/reusable-node-quality.yml @@ -62,10 +62,19 @@ jobs: with: node-version: ${{ inputs.node-version }} cache: ${{ inputs.package-manager == 'pnpm' && 'pnpm' || 'npm' }} + cache-dependency-path: | + ${{ inputs.working-directory }}/pnpm-lock.yaml + ${{ inputs.working-directory }}/package-lock.json - name: Install dependencies + env: + PACKAGE_MANAGER: ${{ inputs.package-manager }} run: | - if [ "${{ inputs.package-manager }}" = "pnpm" ]; then + case "$PACKAGE_MANAGER" in + pnpm|npm) ;; + *) echo "::error::unsupported package-manager: $PACKAGE_MANAGER" >&2; exit 2 ;; + esac + if [ "$PACKAGE_MANAGER" = "pnpm" ]; then pnpm install --frozen-lockfile else npm ci @@ -95,18 +104,29 @@ jobs: with: node-version: ${{ inputs.node-version }} cache: ${{ inputs.package-manager == 'pnpm' && 'pnpm' || 'npm' }} + cache-dependency-path: | + ${{ inputs.working-directory }}/pnpm-lock.yaml + ${{ inputs.working-directory }}/package-lock.json - name: Install dependencies + env: + PACKAGE_MANAGER: ${{ inputs.package-manager }} run: | - if [ "${{ inputs.package-manager }}" = "pnpm" ]; then + case "$PACKAGE_MANAGER" in + pnpm|npm) ;; + *) echo "::error::unsupported package-manager: $PACKAGE_MANAGER" >&2; exit 2 ;; + esac + if [ "$PACKAGE_MANAGER" = "pnpm" ]; then pnpm install --frozen-lockfile else npm ci fi - name: Lint + env: + PACKAGE_MANAGER: ${{ inputs.package-manager }} run: | - if [ "${{ inputs.package-manager }}" = "pnpm" ]; then + if [ "$PACKAGE_MANAGER" = "pnpm" ]; then pnpm lint else npm run lint @@ -135,18 +155,29 @@ jobs: with: node-version: ${{ inputs.node-version }} cache: ${{ inputs.package-manager == 'pnpm' && 'pnpm' || 'npm' }} + cache-dependency-path: | + ${{ inputs.working-directory }}/pnpm-lock.yaml + ${{ inputs.working-directory }}/package-lock.json - name: Install dependencies + env: + PACKAGE_MANAGER: ${{ inputs.package-manager }} run: | - if [ "${{ inputs.package-manager }}" = "pnpm" ]; then + case "$PACKAGE_MANAGER" in + pnpm|npm) ;; + *) echo "::error::unsupported package-manager: $PACKAGE_MANAGER" >&2; exit 2 ;; + esac + if [ "$PACKAGE_MANAGER" = "pnpm" ]; then pnpm install --frozen-lockfile else npm ci fi - name: Run tests with coverage + env: + PACKAGE_MANAGER: ${{ inputs.package-manager }} run: | - if [ "${{ inputs.package-manager }}" = "pnpm" ]; then + if [ "$PACKAGE_MANAGER" = "pnpm" ]; then pnpm test --coverage else npm test -- --coverage @@ -179,19 +210,71 @@ jobs: with: node-version: ${{ inputs.node-version }} cache: ${{ inputs.package-manager == 'pnpm' && 'pnpm' || 'npm' }} + cache-dependency-path: | + ${{ inputs.working-directory }}/pnpm-lock.yaml + ${{ inputs.working-directory }}/package-lock.json - name: Install dependencies + env: + PACKAGE_MANAGER: ${{ inputs.package-manager }} run: | - if [ "${{ inputs.package-manager }}" = "pnpm" ]; then + case "$PACKAGE_MANAGER" in + pnpm|npm) ;; + *) echo "::error::unsupported package-manager: $PACKAGE_MANAGER" >&2; exit 2 ;; + esac + if [ "$PACKAGE_MANAGER" = "pnpm" ]; then pnpm install --frozen-lockfile else npm ci fi - name: Build + env: + PACKAGE_MANAGER: ${{ inputs.package-manager }} run: | - if [ "${{ inputs.package-manager }}" = "pnpm" ]; then + if [ "$PACKAGE_MANAGER" = "pnpm" ]; then pnpm build else npm run build fi + + # ── Quality Gate ──────────────────────────────────────────────────── + quality-gate: + name: Quality Gate + runs-on: ubuntu-latest + if: always() + needs: [typecheck, lint, build, test] + permissions: + contents: read + steps: + - name: Evaluate results + run: | + tc="${{ needs.typecheck.result }}" + ln="${{ needs.lint.result }}" + bd="${{ needs.build.result }}" + ts="${{ needs.test.result }}" + + failed=false + + if [ "$tc" != "success" ]; then + echo "::error::typecheck failed (result: $tc)" + failed=true + fi + if [ "$ln" != "success" ]; then + echo "::error::lint failed (result: $ln)" + failed=true + fi + if [ "$bd" != "success" ]; then + echo "::error::build failed (result: $bd)" + failed=true + fi + # test is optional (has-tests), skipped is ok + if [ "$ts" != "success" ] && [ "$ts" != "skipped" ]; then + echo "::error::test failed (result: $ts)" + failed=true + fi + + if [ "$failed" = "true" ]; then + exit 1 + fi + echo "✅ All quality gates passed" From fd96a89cba9aba84ea2da9505cb5c7ad941f8e9b Mon Sep 17 00:00:00 2001 From: lml2468 Date: Tue, 2 Jun 2026 09:41:54 +0800 Subject: [PATCH 4/5] =?UTF-8?q?fix:=20R4=20blockers=20=E2=80=94=20mock=20g?= =?UTF-8?q?lob,=20Node=20test=20permissions=20split,=20quality-gate=20env?= =?UTF-8?q?=20hardening?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - coverage-exclude: distinguish .go suffix / bare token / directory fragments - bare tokens (mock) → **//** + **/**.go (correctly matches user_mock.go) - .go suffixes (pb.go, _gen.go) → **/* (filename suffix, unchanged) - directory names (vendor, migrations) → **//** only - Node test job: remove pull-requests:write; split coverage reporting into separate report-coverage job that only downloads artifact and posts comment - Go quality-gate: move inputs.services + needs results out of shell interpolation into env vars (complete the shell injection hardening) --- .github/workflows/reusable-go-quality.yml | 37 ++++++++---- .github/workflows/reusable-node-quality.yml | 66 ++++++++++++++++----- 2 files changed, 75 insertions(+), 28 deletions(-) diff --git a/.github/workflows/reusable-go-quality.yml b/.github/workflows/reusable-go-quality.yml index 30a1874..3ea444b 100644 --- a/.github/workflows/reusable-go-quality.yml +++ b/.github/workflows/reusable-go-quality.yml @@ -372,15 +372,23 @@ jobs: fi # W2: Build exclude list from comma-separated input + # Fragment forms: + # *.go suffix (pb.go, _gen.go) → **/* (filename suffix match) + # bare token (mock) → **//** (directory) + **/**.go (filename substring) + # dotted name (foo.bar) → **//** (directory only) EXCLUDE_YAML="" IFS=',' read -ra PATTERNS <<< "$COVERAGE_EXCLUDE" for p in "${PATTERNS[@]}"; do p="${p// /}" [ -z "$p" ] && continue - # Directory patterns (vendor, migrations, mock as dir) - EXCLUDE_YAML="${EXCLUDE_YAML} - \"**/${p}/**\"\n" - # File suffix patterns (pb.go, _gen.go, _mock.go) - EXCLUDE_YAML="${EXCLUDE_YAML} - \"**/*${p}\"\n" + if [[ "$p" == *.go ]]; then + EXCLUDE_YAML="${EXCLUDE_YAML} - \"**/*${p}\"\n" + elif [[ "$p" != *.* ]]; then + EXCLUDE_YAML="${EXCLUDE_YAML} - \"**/${p}/**\"\n" + EXCLUDE_YAML="${EXCLUDE_YAML} - \"**/*${p}*.go\"\n" + else + EXCLUDE_YAML="${EXCLUDE_YAML} - \"**/${p}/**\"\n" + fi done # Write octocov config with exclude paths @@ -547,16 +555,22 @@ jobs: contents: read steps: - name: Evaluate results + env: + SERVICES: ${{ inputs.services }} + TEST_RESULT_WITH_SERVICES: ${{ needs.test-with-services.result }} + TEST_RESULT_STANDALONE: ${{ needs.test-standalone.result }} + DIFF_COVERAGE_RESULT: ${{ needs.diff-coverage.result }} + EVENT_NAME: ${{ github.event_name }} run: | # Determine which test job ran - if [ "${{ inputs.services }}" = "true" ]; then - test_result="${{ needs.test-with-services.result }}" + if [ "$SERVICES" = "true" ]; then + test_result="$TEST_RESULT_WITH_SERVICES" else - test_result="${{ needs.test-standalone.result }}" + test_result="$TEST_RESULT_STANDALONE" fi echo "test result: $test_result" - echo "diff-coverage result: ${{ needs.diff-coverage.result }}" + echo "diff-coverage result: $DIFF_COVERAGE_RESULT" if [ "$test_result" != "success" ]; then echo "::error::Test job did not pass (result: $test_result)" @@ -564,10 +578,9 @@ jobs: fi # diff-coverage only runs on PR; skip check for push events - if [ "${{ github.event_name }}" = "pull_request" ]; then - cov_result="${{ needs.diff-coverage.result }}" - if [ "$cov_result" != "success" ] && [ "$cov_result" != "skipped" ]; then - echo "::error::Diff coverage check did not pass (result: $cov_result)" + if [ "$EVENT_NAME" = "pull_request" ]; then + if [ "$DIFF_COVERAGE_RESULT" != "success" ] && [ "$DIFF_COVERAGE_RESULT" != "skipped" ]; then + echo "::error::Diff coverage check did not pass (result: $DIFF_COVERAGE_RESULT)" exit 1 fi fi diff --git a/.github/workflows/reusable-node-quality.yml b/.github/workflows/reusable-node-quality.yml index cd221f9..deaff0b 100644 --- a/.github/workflows/reusable-node-quality.yml +++ b/.github/workflows/reusable-node-quality.yml @@ -139,7 +139,6 @@ jobs: runs-on: ubuntu-latest permissions: contents: read - pull-requests: write defaults: run: working-directory: ${{ inputs.working-directory }} @@ -183,8 +182,37 @@ jobs: npm test -- --coverage fi + - name: Upload coverage artifact + if: always() + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 + with: + name: node-coverage + path: | + ${{ inputs.working-directory }}/coverage/ + retention-days: 7 + + # ── Coverage Report (separate job with write permissions) ─────────── + report-coverage: + if: inputs.has-tests && github.event_name == 'pull_request' + name: Coverage Report + needs: [test] + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: write + defaults: + run: + working-directory: ${{ inputs.working-directory }} + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + + - name: Download coverage artifact + uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4 + with: + name: node-coverage + path: ${{ inputs.working-directory }}/coverage + - name: Coverage report - if: always() && github.event_name == 'pull_request' uses: davelosert/vitest-coverage-report-action@02f3c2e641286b7fa308cd3e430783103ce6103b # v2 with: working-directory: ${{ inputs.working-directory }} @@ -243,34 +271,40 @@ jobs: name: Quality Gate runs-on: ubuntu-latest if: always() - needs: [typecheck, lint, build, test] + needs: [typecheck, lint, build, test, report-coverage] permissions: contents: read steps: - name: Evaluate results + env: + TYPECHECK_RESULT: ${{ needs.typecheck.result }} + LINT_RESULT: ${{ needs.lint.result }} + BUILD_RESULT: ${{ needs.build.result }} + TEST_RESULT: ${{ needs.test.result }} + COVERAGE_REPORT_RESULT: ${{ needs.report-coverage.result }} run: | - tc="${{ needs.typecheck.result }}" - ln="${{ needs.lint.result }}" - bd="${{ needs.build.result }}" - ts="${{ needs.test.result }}" - failed=false - if [ "$tc" != "success" ]; then - echo "::error::typecheck failed (result: $tc)" + if [ "$TYPECHECK_RESULT" != "success" ]; then + echo "::error::typecheck failed (result: $TYPECHECK_RESULT)" failed=true fi - if [ "$ln" != "success" ]; then - echo "::error::lint failed (result: $ln)" + if [ "$LINT_RESULT" != "success" ]; then + echo "::error::lint failed (result: $LINT_RESULT)" failed=true fi - if [ "$bd" != "success" ]; then - echo "::error::build failed (result: $bd)" + if [ "$BUILD_RESULT" != "success" ]; then + echo "::error::build failed (result: $BUILD_RESULT)" failed=true fi # test is optional (has-tests), skipped is ok - if [ "$ts" != "success" ] && [ "$ts" != "skipped" ]; then - echo "::error::test failed (result: $ts)" + if [ "$TEST_RESULT" != "success" ] && [ "$TEST_RESULT" != "skipped" ]; then + echo "::error::test failed (result: $TEST_RESULT)" + failed=true + fi + # report-coverage only runs on PRs with has-tests, skipped is ok + if [ "$COVERAGE_REPORT_RESULT" != "success" ] && [ "$COVERAGE_REPORT_RESULT" != "skipped" ]; then + echo "::error::coverage report failed (result: $COVERAGE_REPORT_RESULT)" failed=true fi From 449cf9f4d3e318dc04b64a23962a2e94872018ce Mon Sep 17 00:00:00 2001 From: lml2468 Date: Tue, 2 Jun 2026 10:38:12 +0800 Subject: [PATCH 5/5] =?UTF-8?q?fix:=20R5=20blockers=20=E2=80=94=20valid=20?= =?UTF-8?q?octocov=20config=20keys=20+=20core-coverage=20fail-closed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P1-1: Replace invalid octocov config keys with correct schema - coverage.acceptable: expression 'current >= N && diff >= 0' gates both overall threshold and no-regression (replaces invalid diff.coverage.acceptable) - comment.if: 'is_pull_request && env.IS_FORK != true' suppresses fork comments (replaces invalid comment.enable boolean; export IS_FORK for octocov env access) - Remove invalid summary.enable (summary always shown by default) P1-2: Core-package coverage gate now fails closed - go list non-zero exit -> ::error:: + exit 1 (not silent skip) - no packages matched patterns -> ::error:: + exit 1 (not exit 0) - total_stmts == 0 -> ::error:: + exit 1 (not exit 0) --- .github/workflows/reusable-go-quality.yml | 36 ++++++++++------------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/.github/workflows/reusable-go-quality.yml b/.github/workflows/reusable-go-quality.yml index 3ea444b..3c203d3 100644 --- a/.github/workflows/reusable-go-quality.yml +++ b/.github/workflows/reusable-go-quality.yml @@ -366,11 +366,6 @@ jobs: IS_FORK="true" fi - COMMENT_ENABLE="true" - if [ "$IS_FORK" = "true" ]; then - COMMENT_ENABLE="false" - fi - # W2: Build exclude list from comma-separated input # Fragment forms: # *.go suffix (pb.go, _gen.go) → **/* (filename suffix match) @@ -397,21 +392,18 @@ jobs: echo "coverage:" echo " paths:" echo " - coverage.out" - echo " acceptable: ${COVERAGE_THRESHOLD}%" + echo " acceptable: \"current >= ${COVERAGE_THRESHOLD} && diff >= 0\"" if [ -n "$EXCLUDE_YAML" ]; then echo " exclude:" printf '%b' "$EXCLUDE_YAML" fi - echo "diff:" - echo " coverage:" - echo " acceptable: ${COVERAGE_THRESHOLD}%" echo "comment:" - echo " enable: $COMMENT_ENABLE" + echo " if: \"is_pull_request && env.IS_FORK != 'true'\"" echo " hideFooterLink: false" - echo "summary:" - echo " enable: true" } > .octocov.yml + export IS_FORK + echo "Generated .octocov.yml:" cat .octocov.yml @@ -444,15 +436,17 @@ jobs: ["go", "list", "-f", "{{.ImportPath}}", pkg_pattern], capture_output=True, text=True ) - if result.returncode == 0: - for line in result.stdout.strip().splitlines(): - line = line.strip() - if line: - core_import_paths.add(line) + if result.returncode != 0: + print(f"::error::go list failed for pattern '{pkg_pattern}': {result.stderr.strip()}") + sys.exit(1) + for line in result.stdout.strip().splitlines(): + line = line.strip() + if line: + core_import_paths.add(line) if not core_import_paths: - print("Warning: no packages matched core-packages patterns") - sys.exit(0) + print("::error::no packages matched core-packages patterns") + sys.exit(1) # Parse coverage.out: format is "pkg/path/file.go:start.col,end.col stmts count" total_stmts = 0 @@ -488,10 +482,10 @@ jobs: f.write("|--------|---------|----------|----------|\n") if total_stmts == 0: - print("Warning: no statements found for core packages — check import paths") + print("::error::no statements found for core packages — check import paths") with open(summary_path, "a") as f: f.write("| ⚠️ | (all core packages) | N/A — no statements found | check patterns |\n") - sys.exit(0) + sys.exit(1) coverage_pct = covered_stmts / total_stmts * 100 status = "✅" if coverage_pct >= threshold else "❌"