htmlreport: Add new command that creates HTML reports of SIG test case failures#92
htmlreport: Add new command that creates HTML reports of SIG test case failures#92
Conversation
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Avoid using global variables (SigFailures and jobRegex); instead declare them inside Generate so the function is reentrant and testable.
- On critical failures (e.g. reading results.json or creating the output file), return an error or exit rather than just logging, and defer closing any opened files to prevent resource leaks.
- When printing the CLI success message, include the full path to the generated report file (including its filename) instead of just the directory for clearer feedback.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Avoid using global variables (SigFailures and jobRegex); instead declare them inside Generate so the function is reentrant and testable.
- On critical failures (e.g. reading results.json or creating the output file), return an error or exit rather than just logging, and defer closing any opened files to prevent resource leaks.
- When printing the CLI success message, include the full path to the generated report file (including its filename) instead of just the directory for clearer feedback.
## Individual Comments
### Comment 1
<location> `README.md:171` </location>
<code_context>
+
+### html-report command
+
+A command that generates html reports of test case failures per SIG. To create a report for SIG Compute failures:
+```
+go run ./cmd/html-report --sig compute --results-path ./output/kubevirt/kubevirt/results.json --path /tmp/
</code_context>
<issue_to_address>
Capitalize 'html' to 'HTML' for correctness.
Use 'HTML' instead of 'html' to align with standard terminology.
</issue_to_address>
### Comment 2
<location> `README.md:176` </location>
<code_context>
+go run ./cmd/html-report --sig compute --results-path ./output/kubevirt/kubevirt/results.json --path /tmp/
+```
+
+This should create a html report called sig-compute-failure-report.html under /tmp/.
</code_context>
<issue_to_address>
Use 'an HTML report' instead of 'a html report'.
Use 'an' before 'HTML' because it starts with a vowel sound, and ensure 'HTML' is capitalized.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
README.md
Outdated
|
|
||
| ### html-report command | ||
|
|
||
| A command that generates html reports of test case failures per SIG. To create a report for SIG Compute failures: |
There was a problem hiding this comment.
suggestion (typo): Capitalize 'html' to 'HTML' for correctness.
Use 'HTML' instead of 'html' to align with standard terminology.
README.md
Outdated
| go run ./cmd/html-report --sig compute --results-path ./output/kubevirt/kubevirt/results.json --path /tmp/ | ||
| ``` | ||
|
|
||
| This should create a html report called sig-compute-failure-report.html under /tmp/. |
There was a problem hiding this comment.
suggestion (typo): Use 'an HTML report' instead of 'a html report'.
Use 'an' before 'HTML' because it starts with a vowel sound, and ensure 'HTML' is capitalized.
374e0b7 to
6e4fd0f
Compare
…rom ci-health This requires kubevirt/ci-health#92 Signed-off-by: Brian Carey <bcarey@redhat.com>
dhiller
left a comment
There was a problem hiding this comment.
@brianmcarey great idea, and a very useful report. Some questions inline.
|
|
||
| */ -}} | ||
|
|
||
| {{- /* sig-failure-report.gohtml */ -}} |
There was a problem hiding this comment.
Why is this comment necessary?
6e4fd0f to
7a894fb
Compare
|
@dhiller I've cleaned up the duplication - can you please take a look? |
dhiller
left a comment
There was a problem hiding this comment.
Looks much better, thanks!
One last comment though - I believe it would be beneficial to use the junit library as mentioned here: #92 (comment)
…e failures This new command will primarily used by periodic prow jobs to generate daily reports on each specific SIG's tescase failures. These reports will help SIG members to identify and tackle the faiures. The reports will be stored on the kubevirt GCS bucket and can be linked to from the ci-health README where we track the number of SIG e2e failures Signed-off-by: Brian Carey <bcarey@redhat.com>
7a894fb to
1769697
Compare
thanks for the great suggestion @dhiller - applied the changes and it helped to simplify things. |
…rom ci-health This requires kubevirt/ci-health#92 Signed-off-by: Brian Carey <bcarey@redhat.com>
…rom ci-health (#4345) This requires kubevirt/ci-health#92 Signed-off-by: Brian Carey <bcarey@redhat.com>
What this PR does / why we need it:
This new command will primarily used by periodic prow jobs to generate daily reports on each specific SIG's tescase failures. These reports will help SIG members to identify and tackle the faiures.
The reports will be stored on the kubevirt GCS bucket and can be linked to from the ci-health README where we track the number of SIG e2e failures
Here is an example report for SIG Network:

Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #
Special notes for your reviewer:
/cc @dhiller @xpivarc
Checklist
This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR.
Approvers are expected to review this list.
Release note: