fix(unit_test_v2): restore deleted new_code_coverage.go referenced by action.yaml#427
Conversation
There was a problem hiding this comment.
2 issues found across 1 file
Confidence score: 2/5
- In
backend/unit_test_v2/calc_new_code_coverage/new_code_coverage.go, the CLI arg-length guard is off by one (len(os.Args) < 3before readingos.Args[3]), so running with exactly program + diff-file + coverage-file will panic at runtime and break the coverage calculation flow — fix the guard/indexing (or validate required args explicitly) before merging. - In
backend/unit_test_v2/calc_new_code_coverage/new_code_coverage.go,parseNewFiles/newFilesnaming suggests only newly created files, but it actually collects all+++ b/paths (including modified files), which can mislead future changes and reviews — rename the function/variable (or add a clarifying comment) to reduce maintenance risk.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="backend/unit_test_v2/calc_new_code_coverage/new_code_coverage.go">
<violation number="1" location="backend/unit_test_v2/calc_new_code_coverage/new_code_coverage.go:21">
P0: Index out of bounds panic when `os.Args` has exactly 3 elements (program + diff-file + coverage-file). The guard `len(os.Args) < 3` allows execution to proceed with 3 args, but `os.Args[3]` accesses an index that doesn't exist. While the action.yaml always passes 3 positional arguments making CI work, any manual invocation with just diff-file and coverage-file (as shown in the Usage message) will panic.</violation>
<violation number="2" location="backend/unit_test_v2/calc_new_code_coverage/new_code_coverage.go:58">
P3: Function name `parseNewFiles` and variable name `newFiles` are misleading — the function captures ALL files from `+++ b/` lines in the diff (i.e., all added and modified files, not just new files). The `--- a/` deletion logic doesn't achieve new-file-only filtering because in standard unified diffs the `---` line precedes `+++`, so the deletion runs first (no-op) and the subsequent `+++` line adds the file anyway.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Workflow as GitHub Workflow
participant ActionYAML as unit_test_v2/action.yaml
participant CoverageGo as calc_new_code_coverage/new_code_coverage.go
participant DiffFile as Diff File (from git)
participant CoverageFile as Go Coverage Output
participant GoTool as go tool cover
Note over Workflow,GoTool: Unit Test v2 Composite Action Runtime Flow
Workflow->>ActionYAML: Trigger "Run coverage analysis" step
ActionYAML->>CoverageGo: Execute go run new_code_coverage.go <diff-file> <coverage-file> <path-scanned>
CoverageGo->>DiffFile: Parse diff file for changed files
DiffFile-->>CoverageGo: Returns map of changed file paths
CoverageGo->>GoTool: Run "go tool cover -func <coverage-file>"
GoTool-->>CoverageGo: Returns coverage data per function/file
CoverageGo->>CoverageGo: Filter coverage by changed files and path-scanned
CoverageGo->>CoverageGo: Calculate per-file coverage percentage
loop For each changed file in coverage data
alt Coverage >= 80%
CoverageGo->>CoverageGo: Append to "Coverage accepted" output
else Coverage < 80%
CoverageGo->>CoverageGo: Append to "Coverage below threshold" output
end
end
CoverageGo-->>ActionYAML: Print coverage results to stdout
ActionYAML-->>Workflow: Return step output (stdout captured by workflow)
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
|
|
||
| diffFile := os.Args[1] | ||
| coverageFile := os.Args[2] | ||
| pathScanned := os.Args[3] |
There was a problem hiding this comment.
P0: Index out of bounds panic when os.Args has exactly 3 elements (program + diff-file + coverage-file). The guard len(os.Args) < 3 allows execution to proceed with 3 args, but os.Args[3] accesses an index that doesn't exist. While the action.yaml always passes 3 positional arguments making CI work, any manual invocation with just diff-file and coverage-file (as shown in the Usage message) will panic.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/unit_test_v2/calc_new_code_coverage/new_code_coverage.go, line 21:
<comment>Index out of bounds panic when `os.Args` has exactly 3 elements (program + diff-file + coverage-file). The guard `len(os.Args) < 3` allows execution to proceed with 3 args, but `os.Args[3]` accesses an index that doesn't exist. While the action.yaml always passes 3 positional arguments making CI work, any manual invocation with just diff-file and coverage-file (as shown in the Usage message) will panic.</comment>
<file context>
@@ -0,0 +1,114 @@
+
+ diffFile := os.Args[1]
+ coverageFile := os.Args[2]
+ pathScanned := os.Args[3]
+
+ // Parse the diff file to get changed files
</file context>
| } | ||
| } | ||
|
|
||
| func parseNewFiles(filename string) (map[string]bool, error) { |
There was a problem hiding this comment.
P3: Function name parseNewFiles and variable name newFiles are misleading — the function captures ALL files from +++ b/ lines in the diff (i.e., all added and modified files, not just new files). The --- a/ deletion logic doesn't achieve new-file-only filtering because in standard unified diffs the --- line precedes +++, so the deletion runs first (no-op) and the subsequent +++ line adds the file anyway.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/unit_test_v2/calc_new_code_coverage/new_code_coverage.go, line 58:
<comment>Function name `parseNewFiles` and variable name `newFiles` are misleading — the function captures ALL files from `+++ b/` lines in the diff (i.e., all added and modified files, not just new files). The `--- a/` deletion logic doesn't achieve new-file-only filtering because in standard unified diffs the `---` line precedes `+++`, so the deletion runs first (no-op) and the subsequent `+++` line adds the file anyway.</comment>
<file context>
@@ -0,0 +1,114 @@
+ }
+}
+
+func parseNewFiles(filename string) (map[string]bool, error) {
+ file, err := os.Open(filename)
+ if err != nil {
</file context>
Summary
The
backend/unit_test_v2/action.yamlreferences${{ github.action_path }}/calc_new_code_coverage/new_code_coverage.goin its "Run coverage analysis" step, but this file was deleted in commitf9d55729while the action YAML was not updated.This causes all workflows using
kitabisa/composite-actions/backend/unit_test_v2@v2(or any v2 tag) to fail with:Fix
Restored the file from the last known good commit (
2ec3f1a) before it was deleted.Summary by cubic
Restores the deleted coverage helper referenced by the unit test v2 composite action so CI workflows run successfully again. Unblocks any workflow using
kitabisa/composite-actions/backend/unit_test_v2@v2.backend/unit_test_v2/calc_new_code_coverage/new_code_coverage.goreferenced bybackend/unit_test_v2/action.yaml.Written for commit a8e8c6b. Summary will update on new commits.