ci: integrate reusable workflows for code quality and coverage#80
ci: integrate reusable workflows for code quality and coverage#80pkking wants to merge 1 commit into
Conversation
- Add go_coverage_check.sh script for incremental coverage analysis - Add ci-reusable.yml workflow calling organization reusable workflows: - go-reusable: build + coverage (full + incremental) - sast-reusable: static analysis - gitleaks-reusable: secret scanning - trivy-vulnerability: vulnerability scanning - trivy-license: license scanning - check-branch-naming: branch naming validation (PR only) - check-label: PR label validation (PR only) - document-gate: documentation gate (PR only)
Welcome To opensourceways CommunityHey @pkking , thanks for your contribution to the community. Bot Usage ManualI'm the Bot here serving you. You can find the instructions on how to interact with me at Here . That means you can comment below every pull request or issue to trigger Bot Commands. Contact GuideIf you have any questions, please contact the SIG: infratructure , |
CLA Signature Passpkking, thanks for your pull request. All authors of the commits have signed the CLA. 👍 |
Linking Issue Notice@pkking , the pull request must be linked to at least one issue. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a unified Go coverage check script (scripts/go_coverage_check.sh) designed to enforce both full and incremental coverage thresholds. The script includes logic to parse git diffs and map changed lines to Go coverage blocks. Feedback focused on improving the robustness of the shell script, specifically regarding handling file paths with spaces, ensuring compatibility with older Bash versions (like those on macOS), and optimizing the iteration logic for processing changed lines.
| local changed_lines=$(awk ' | ||
| # Match .go files, exclude _test.go | ||
| /^\+\+\+ b\/.*\.go/ && !/.*_test\.go/ { | ||
| file = substr($NF, 3); |
There was a problem hiding this comment.
The use of $NF to extract the filename will fail if the file path contains spaces, as awk splits fields by whitespace by default. Using substr($0, 7) is a more robust way to capture the full path after the +++ b/ prefix in a standard git diff.
| file = substr($NF, 3); | |
| file = substr($0, 7); |
| echo "$changed_lines" | head -20 | ||
|
|
||
| # Use associative array for processed blocks (more reliable than string matching) | ||
| declare -A processed_blocks=() |
There was a problem hiding this comment.
This script uses associative arrays (declare -A), which require Bash 4.0 or higher. Some environments (notably the default shell on macOS) still use Bash 3.2. It is recommended to add a check for the Bash version at the beginning of the script or document this requirement to prevent confusing errors for users running the script locally.
| for info in $changed_lines; do | ||
| local target_file="${info%:*}" | ||
| local target_line="${info#*:}" | ||
|
|
||
| # Get blocks for this file from memory | ||
| local blocks_data="${coverage_blocks_by_file[$target_file]}" | ||
| if [ -z "$blocks_data" ]; then | ||
| continue | ||
| fi | ||
|
|
||
| # Find blocks containing this line | ||
| while IFS= read -r block_data; do | ||
| [[ -z "$block_data" ]] && continue | ||
|
|
||
| # Parse: start_line.col,end_line.col stmts hits | ||
| local line_range stmts hits | ||
| read -r line_range stmts hits <<< "$block_data" | ||
|
|
||
| local start_line="${line_range%%.*}" | ||
| local tmp="${line_range#*,}" | ||
| local end_line="${tmp%%.*}" | ||
|
|
||
| # Check if changed line is within block range | ||
| if ((target_line >= start_line && target_line <= end_line)); then | ||
| local block_uid="${target_file}:${line_range}" | ||
|
|
||
| if [[ -z "${processed_blocks[$block_uid]}" ]]; then | ||
| total_inc_stmts=$((total_inc_stmts + stmts)) | ||
|
|
||
| if ((hits > 0)); then | ||
| covered_inc_stmts=$((covered_inc_stmts + stmts)) | ||
| echo " ✅ Covered: ${block_uid} (${stmts} stmts)" | ||
| else | ||
| echo " ❌ Uncovered: ${block_uid} (${stmts} stmts)" | ||
| fi | ||
|
|
||
| processed_blocks[$block_uid]=1 | ||
| fi | ||
| break | ||
| fi | ||
| done <<< "$(echo -e "$blocks_data")" | ||
| done |
There was a problem hiding this comment.
Iterating over changed_lines using a for loop with unquoted expansion will fail if any filenames contain spaces. Additionally, the nested loop structure (iterating over all coverage blocks for every changed line) is inefficient ($O(L \times B)$).
Using a while read loop with a here-string is more robust for handling filenames with spaces and avoids subshell issues with variable scope.
while read -r info; do
[[ -z "$info" ]] && continue
local target_file="${info%:*}"
local target_line="${info#*:}"
# Get blocks for this file from memory
local blocks_data="${coverage_blocks_by_file[$target_file]}"
if [ -z "$blocks_data" ]; then
continue
fi
# Find blocks containing this line
while IFS= read -r block_data; do
[[ -z "$block_data" ]] && continue
# Parse: start_line.col,end_line.col stmts hits
local line_range stmts hits
read -r line_range stmts hits <<< "$block_data"
local start_line="${line_range%%.*}"
local tmp="${line_range#*,}"
local end_line="${tmp%%.*}"
# Check if changed line is within block range
if ((target_line >= start_line && target_line <= end_line)); then
local block_uid="${target_file}:${line_range}"
if [[ -z "${processed_blocks[$block_uid]}" ]]; then
total_inc_stmts=$((total_inc_stmts + stmts))
if ((hits > 0)); then
covered_inc_stmts=$((covered_inc_stmts + stmts))
echo " ✅ Covered: ${block_uid} (${stmts} stmts)"
else
echo " ❌ Uncovered: ${block_uid} (${stmts} stmts)"
fi
processed_blocks[$block_uid]=1
fi
break
fi
done <<< "$(echo -e "$blocks_data")"
done <<< "$changed_lines"
2 participants
Summary
Integrate organization reusable workflows to improve code quality, documentation quality, and test coverage.
Changes
1. scripts/go_coverage_check.sh
Add unified Go coverage check script with full and incremental coverage threshold checks.
2. .github/workflows/ci-reusable.yml
New CI pipeline calling organization reusable workflows:
Related