[CI] Single button press for automated Qase Test Runs#154
[CI] Single button press for automated Qase Test Runs#154git-ival wants to merge 44 commits intorancher:mainfrom
Conversation
…uild files and docs
… parameterised tests within a qase test run and report the results
There was a problem hiding this comment.
Pull request overview
This PR adds automated Qase test run functionality with a single button press by introducing a new gather subcommand to the qase-k6-cli tool and creating a Jenkins pipeline to orchestrate the end-to-end workflow. The tool has been refactored to support subcommands and renamed from qasereporter-k6 to qase-k6-cli throughout the codebase.
Key Changes:
- Introduces
gathersubcommand to retrieve test cases from Qase test runs with parameter combinations - Refactors main.go to support subcommands (
reportandgather) - Adds new Jenkins pipeline (qase-k6-runner.groovy) to automate test execution workflow
- Renames tool from qasereporter-k6 to qase-k6-cli across all files
Reviewed changes
Copilot reviewed 10 out of 13 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| qase-k6-cli/main.go | Refactored to support subcommands; added runGather function and generateCombinations helper for Cartesian product of test parameters; added support for QASE_TEST_CASE_ID environment variable |
| qase-k6-cli/summary.go | Updated log message to reflect new tool name |
| qase-k6-cli/build_qase_k6_cli.sh | Updated variable names and output paths to reflect rename |
| qase-k6-cli/README.md | Updated documentation for new subcommands and tool name |
| internal/qase/status.go | Added new StatusExceededThresholds constant |
| internal/qase/client.go | Modified GetTestRun to accept optional include parameter; added new GetCustomFields method; added QASE_TEST_CASE_ID constant |
| README.md | Updated references to new tool name and functionality |
| Makefile | Updated binary name and paths from qasereporter-k6 to qase-k6-cli |
| Dockerfile | Updated binary paths to reflect rename (contains critical bug - wrong directory path) |
| CI/qase-k6-runner.groovy | New Jenkins pipeline implementing automated test execution workflow (contains several critical bugs) |
| .gitignore | Updated binary path and added .env file |
Comments suppressed due to low confidence (5)
qase-k6-cli/main.go:296
- When a test case does not have the 'AutomationTestName' custom field, no entries are added to results for that case. This means test cases without this field are silently skipped. Consider logging a warning or info message when a test case is skipped due to missing the required custom field, to help with debugging.
qase-k6-cli/main.go:163 - Potential nil pointer dereference: 'resp.Title' is dereferenced without checking if 'resp' is nil. Although the error check on line 159 should prevent this, it's safer to verify that 'resp' is not nil and that 'resp.Title' is not nil before dereferencing.
qase-k6-cli/main.go:347 - When there are no parameters (empty params map), this function will return a slice with one empty map. This means test cases without parameters will still generate one "combination" entry. Consider if this is the intended behavior - it might be clearer to return an empty slice or a slice with a single nil entry when there are no parameters to combine.
qase-k6-cli/main.go:183 - Potential nil pointer dereference: 'testCase.Id' is dereferenced without checking if 'testCase' is nil or if 'testCase.Id' is nil. Although the error check on line 179 should prevent 'testCase' from being nil, it's safer to verify that both 'testCase' and 'testCase.Id' are not nil before dereferencing.
qase-k6-cli/main.go:289 - Potential nil pointer dereference: 'tc.Title' is dereferenced without checking if it's nil. Add a nil check before dereferencing to prevent panics.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (7)
qase-k6-cli/README.md:75
- Missing documentation for new environment variable. The code now supports
K6_WEB_DASHBOARD_EXPORTfor attaching web dashboard reports (see summary.go lines 17 and 86-93), but this environment variable is not documented in the Environment Variables table. Users should be informed about this optional feature.
qase-k6-cli/main.go:352 - Edge case handling for empty parameter map. The generateCombinations function will return an empty slice when given an empty params map (len(keys) == 0). This means a test case with no parameters will generate zero combinations, resulting in no results being added. Consider adding a comment or handling this case explicitly by returning a single empty map to represent "one combination with no parameters".
qase-k6-cli/main.go:286 - Potential nil pointer dereference. The code dereferences
cf.Idwithout checking if it's nil. While the custom field ID is likely always present, defensive programming would suggest adding a nil check before dereferencing, especially since other fields likecf.Titleandcf.Valueare checked for nil.
qase-k6-cli/main.go:300 - The logic for iterating over custom fields has an issue. When a test case has the AutomationTestName field, combinations are added to results. However, if a test case has multiple custom fields but AutomationTestName is not the first one encountered, the else block will execute for every non-matching custom field. This could result in multiple unnecessary iterations. Consider restructuring to check if AutomationTestName exists after the loop completes, or use a flag to track whether it was found.
qase-k6-cli/README.md:75 - Missing documentation for new environment variable. The code now supports
QASE_TEST_CASE_IDas an alternative toQASE_TEST_CASE_NAME(see main.go lines 166-173), but this environment variable is not documented in the Environment Variables table. Users need to know about this option, especially since it's used in the Jenkins pipeline.
qase-k6-cli/main.go:294 - Test cases with no parameters will be silently skipped. When a test case has no parameters, generateCombinations returns an empty slice, causing the loop at line 287 to never execute, resulting in zero entries in the results. This means test cases without parameters but with an AutomationTestName field will not appear in the output. If this is intentional, consider logging a message. Otherwise, handle the empty combinations case by adding at least one result with empty parameters.
qase-k6-cli/main.go:298 - The condition
automationTestNameID == 0inside the else block will always be false at this point. The function already checks and fatally exits ifautomationTestNameID == 0at line 241-243, so this code is unreachable. This else branch appears to handle cases where a test case doesn't have the AutomationTestName field set, but the unreachable condition means the informational log will never execute. Consider removing the inner if check or restructuring the logic.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (10)
qase-k6-cli/main.go:352
- When the input map is empty (no parameters), the function will return a slice containing one empty map. This might be the intended behavior for test cases without parameters, but it should be verified. If a test case has no parameters, should it generate one combination with an empty parameter map, or should it generate zero combinations?
qase-k6-cli/main.go:286 - There's a potential nil pointer dereference at line 286. The code checks if
*cf.Id == automationTestNameID && cf.Value != nil, but it doesn't check ifcf.Iditself is nil before dereferencing it. This could cause a panic if a custom field has a nil Id.
qase-k6-cli/main.go:236 - There's a potential nil pointer dereference at line 236. While the code checks if
cf.Titleis not nil, it doesn't check ifcf.Idis nil before dereferencing it with*cf.Id. This could cause a panic if a custom field has a nil Id.
qase-k6-cli/main.go:163 - There's a potential nil pointer dereference at line 163. The code dereferences
resp.Titlewithout checking if it's nil first. If the test run has no title set, this could cause a panic.
qase-k6-cli/main.go:183 - There's a potential nil pointer dereference at line 183. The code dereferences
testCase.Idwithout checking if it's nil first. While it's unlikely that a test case would have a nil ID, this should be checked for defensive programming.
qase-k6-cli/main.go:290 - There's a potential nil pointer dereference at line 290. The code dereferences
tc.Titlewithout checking if it's nil first. This could cause a panic if a test case has no title.
qase-k6-cli/main.go:298 - The condition
automationTestNameID == 0will always be false at this point. The code already exits with a fatal error at line 241-242 ifautomationTestNameIDis 0, so this block (lines 296-298) can never execute. This appears to be dead code that should be removed.
qase-k6-cli/main.go:300 - The logic in this block seems incorrect. The outer condition checks if the custom field ID matches and has a value, but the else block (line 295) executes when either the ID doesn't match OR the value is nil. This means that for every custom field that is not 'AutomationTestName', the code inside the else block would attempt to execute (though it won't due to the nested condition bug at line 296). Consider restructuring this logic to only add results when the AutomationTestName field is found with a value, and perhaps log a warning outside the custom fields loop if no AutomationTestName was found for a test case.
CI/qase-k6-runner.groovy:128 - The parameters QASE_TESTOPS_PROJECT and QASE_TESTOPS_RUN_ID are not sanitized before being used in the shell command at line 127-128. While QASE_TESTOPS_RUN_ID is later sanitized at line 163 for use in filenames, these parameters should also be sanitized here to prevent potential shell injection, especially QASE_TESTOPS_PROJECT which is directly embedded in the docker command.
-e QASE_TESTOPS_PROJECT="${params.QASE_TESTOPS_PROJECT}" \\
${env.IMAGE_NAME}:latest qase-k6-cli gather -runID ${params.QASE_TESTOPS_RUN_ID} > test_cases.json
qase-k6-cli/main.go:133
- The comment on line 133 is misleading. It says "Use the provided case ID flag, otherwise default to the run ID", but the variable being set is
runIDStr(the run ID string), not a case ID. The comment should be updated to say "Use the provided run ID flag override" or similar to accurately describe what the code does.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…report + custom reports
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (7)
qase-k6-cli/README.md:75
- The Environment Variables/How it works sections still describe only QASE_TEST_CASE_NAME-based lookup and don't mention the new QASE_TEST_CASE_ID option, and they also omit the new K6_WEB_DASHBOARD_EXPORT attachment env var. Update the docs so CI/users know the preferred variables and how to attach the web dashboard export.
qase-k6-cli/main.go:135 - The comment says "case ID flag" but this flag is overriding the test run ID (runIDOverride/runIDStr). This is misleading when troubleshooting CLI usage; update the comment to reflect that it overrides the run ID.
qase-k6-cli/main.go:299 - The inner condition
if automationTestNameID == 0can never be true here because the function already fatals when automationTestNameID is 0. This means the "custom field not found" log path is effectively dead and you'll never log missing AutomationTestName on a per-test-case basis. Consider tracking whether the field was found for the current tc (e.g., foundAutomationName=false) and logging once after the loop when it wasn't found.
qase-k6-cli/main.go:316 - generateCombinations builds
keysby ranging over a map, which produces a nondeterministic order in Go. That makes the generated combination order (and therefore the JSON output ordering) unstable between runs, which can cause flaky Jenkins behavior when iterating by index. Sortkeysbefore recursing to ensure deterministic output.
qase-k6-cli/summary.go:102 - With the addition of a second possible attachment (Web Dashboard export),
attachmentscan now contain multiple files. The upload loop below calls UploadAttachments with the growingfilesslice on each iteration, which will re-upload earlier files and can duplicate hashes/attachments (e.g., the HTML report gets uploaded again when the dashboard file is processed). Consider uploading each file individually or open all files first and call UploadAttachments once.
qase-k6-cli/main.go:287 - This dereferences cf.Id without a nil check. The Qase API models use pointers for nullable fields (as seen elsewhere in this file), so a nil cf.Id would panic. Guard against nil (or use a generated getter) before comparing to automationTestNameID.
qase-k6-cli/main.go:293 - tc.Title is dereferenced without a nil check. If the API returns a test case with a nil Title, this will panic and abort gathering. Consider using tc.GetTitle() (if available) or checking tc.Title != nil before dereferencing, and decide on a fallback title in the JSON output.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| */ | ||
| export function customHandleSummary(data) { | ||
| const prefix = __ENV.K6_TEST ? __ENV.K6_TEST.replace(/\.js$/, '') + "-" : ''; | ||
| const prefix = __ENV.K6_REPORT_PREFIX ? __ENV.K6_REPORT_PREFIX.replace(/\.js$/, '') + "-" : ''; |
There was a problem hiding this comment.
customHandleSummary now only looks at K6_REPORT_PREFIX. Existing CI (e.g., CI/k6-runner.groovy) sets K6_TEST and expects reports like "-summary.json"; after this change, the prefix will be empty unless K6_REPORT_PREFIX is set, and CI artifact collection/reporting will likely break. Consider falling back to K6_TEST when K6_REPORT_PREFIX is unset (or update all callers to set K6_REPORT_PREFIX).
| const prefix = __ENV.K6_REPORT_PREFIX ? __ENV.K6_REPORT_PREFIX.replace(/\.js$/, '') + "-" : ''; | |
| const prefixSource = __ENV.K6_REPORT_PREFIX | |
| ? __ENV.K6_REPORT_PREFIX | |
| : (__ENV.K6_TEST ? __ENV.K6_TEST : ''); | |
| const prefix = prefixSource ? prefixSource.replace(/\.js$/, '') + "-" : ''; |
CI/qase-k6-runner.groovy
Outdated
| sh """ | ||
| echo "Environment file for Case ${caseId} (index ${index}):" | ||
| echo "${envContent}" | ||
| """ | ||
|
|
||
| // 2. Run k6 |
There was a problem hiding this comment.
The envContent string, built from Qase-derived parameters and Jenkins parameters, is interpolated directly into a shell script and printed with echo "${envContent}" in an sh step. If any embedded value contains a double quote or shell metacharacters, this will break out of the quoted string and allow arbitrary commands to be injected and executed on the Jenkins agent. To mitigate this, avoid echoing raw multi-line content containing untrusted values from within a shell script—either log it via Jenkins/Groovy (not the shell), or escape/encode all untrusted values before embedding them into the shell command.
| sh """ | |
| echo "Environment file for Case ${caseId} (index ${index}):" | |
| echo "${envContent}" | |
| """ | |
| // 2. Run k6 | |
| echo "Environment file for Case ${caseId} (index ${index}):" | |
| echo envContent | |
| // 2. Run k6 | |
| // 2. Run k6 |
CI/qase-k6-runner.groovy
Outdated
| --workdir /app \\ | ||
| --entrypoint='' \\ | ||
| -e QASE_TESTOPS_API_TOKEN \\ | ||
| -e QASE_TESTOPS_PROJECT="${params.QASE_TESTOPS_PROJECT}" \\ |
There was a problem hiding this comment.
The Jenkins parameter QASE_TESTOPS_PROJECT is interpolated directly into a shell script string and passed to sh, forming the docker run command without any escaping. If an attacker can influence this parameter, they can inject shell metacharacters (for example, by closing the quote and adding ; followed by arbitrary commands) to execute arbitrary code on the Jenkins agent. To mitigate this, avoid embedding untrusted parameters directly into shell strings—either sanitize/validate the value, or pass it via environment variables/sh environment maps instead of inline string interpolation.
|
Currently working on some fixes for a few things Copilot raised. Additionally, looking into how to ensure there is "enough data" for k6 to actually generate a web dashboard html report on test end. |
Closes #151
Note: Gemini was used to help design and write some of the recursive backtracking logic for generating the test case parameter combinations and for some of the new job's groovy looping logic. It would be good to review these closely, we may want to refactor some of this to make it clearer/easier to understand.