-
Notifications
You must be signed in to change notification settings - Fork 591
Comments
NO-JIRA: feat: update the html for feature promotion#2714
NO-JIRA: feat: update the html for feature promotion #2714eggfoobar wants to merge 1 commit intoopenshift:master from
Conversation
openshift-ci-robot
commented
Feb 17, 2026
Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.
For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.
This repository is configured in: LGTM mode
openshift-ci-robot
commented
Feb 17, 2026
@eggfoobar: This pull request explicitly references no jira issue.
Details
In response to this:
small quality of life improvement to the html for feature promotion, this addition will allow sorting by pass rate and make it easier to identify where a feature is lacking validation by color coating the pass rate.
Example:
image
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.
Hello @eggfoobar! Some important instructions when contributing to openshift/api:
API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.
📝 WalkthroughWalkthroughThis change refactors the HTML generation logic for feature gate testing analysis. It replaces direct HTML assembly with a template-based rendering approach. A new utils file introduces data structures (HTMLTemplateData, HTMLFeatureGate, HTMLVariantColumn, HTMLTestRow, HTMLTestCell) and an HTML template constant. The featuregate-test-analyzer command is updated to build structured data via buildHTMLFeatureGateData, render through writeHTMLFromTemplate using the shared template, and output results to file. The hardcoded HTML header constant and blackfriday dependency are removed, with html/template imported to support template-based rendering. 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.5.0)Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented Comment |
i You are approaching your monthly quota for Qodo. Upgrade your plan
Review Summary by Qodo
Enhance feature promotion HTML with templating and interactive sorting
✨ Enhancement
Walkthroughs
Description
• Replace markdown-based HTML generation with Go template system • Add interactive sorting by test name and pass rate percentage • Implement color-coded pass/fail cell visualization for test results • Create structured data types for HTML template rendering
Diagram
flowchart LR
A["Markdown Generation<br/>blackfriday"] -->|Replace| B["Go Template System<br/>html/template"]
C["Static HTML Header<br/>Constant"] -->|Replace| D["Dynamic Template<br/>with Data Structures"]
E["Test Results Data"] -->|Transform| F["HTMLFeatureGate<br/>Structures"]
F -->|Render| G["Interactive HTML<br/>with Sorting & Colors"]
File Changes
1. tools/codegen/cmd/featuregate-test-analyzer.go
✨ Enhancement +88/-26
Refactor HTML generation to use templates
• Replace blackfriday markdown library with Go's html/template for HTML generation • Remove hardcoded HTML header constant and implement writeHTMLFromTemplate() function • Add buildHTMLFeatureGateData() function to transform test results into structured HTML data • Collect feature gate data in featureGateHTMLData slice during test analysis loop
2. tools/codegen/pkg/utils/html.go
✨ Enhancement +166/-0
Add HTML template structures and interactive styling
• Define HTMLTemplateData, HTMLFeatureGate, HTMLVariantColumn, HTMLTestRow, and HTMLTestCell data structures • Implement comprehensive HTML template with Bootstrap styling and responsive design • Add interactive JavaScript for sortable table headers with ascending/descending indicators • Include color-coded cells (red for failures, green for passes) with pass percentage display • Support sorting by test name (text) and pass rate (numeric) with visual sort state indicators
i You are approaching your monthly quota for Qodo. Upgrade your plan
Code Review by Qodo
🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0)
1. HTML shows 95%% 🐞 Bug ✓ Correctness
Description
The new HTML template contains a literal "95%%" which will render as "95%%" in the report. This is a user-facing correctness/UX bug and contradicts the intended 95% threshold display.
Code
tools/codegen/pkg/utils/html.go[R78-82]+ <li>At least five tests are expected for a feature</li> + <li>Tests must be run on every TechPreview platform (ask for an exception if your feature doesn't support a variant)</li> + <li>All tests must run at least 14 times on every platform</li> + <li>All tests must pass at least 95%% of the time</li> + </ul>
Evidence
The HTML template hardcodes 95%% (template output is not printf-formatted), while the markdown path uses 95%% intentionally within Textf formatting to produce a single % in the rendered markdown—showing the HTML should use 95% instead.
tools/codegen/pkg/utils/html.go[72-83]
tools/codegen/cmd/featuregate-test-analyzer.go[190-199]
Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution ### Issue description The HTML template uses a literal `95%%`, which renders incorrectly as `95%%` in the generated report. ### Issue Context In Go templates, `%%` is not a special escape sequence (unlike printf-style formatting used by `md.Textf`). ### Fix Focus Areas - tools/codegen/pkg/utils/html.go[78-82]
i Copy this prompt and use it to remediate the issue with your preferred AI generation tools
2. HTML perms inconsistent 🐞 Bug ⛯ Reliability
Description
The HTML report is now created via os.Create, which uses default permissions (umask-dependent), while the markdown report is written with explicit 0644. This can lead to inconsistent artifact permissions across environments/CI runs.
Code
tools/codegen/cmd/featuregate-test-analyzer.go[R293-297]+ f, err := os.Create(filename) + if err != nil { + return fmt.Errorf("error creating HTML file: %w", err) + } + defer f.Close()
Evidence
The code explicitly sets permissions for the markdown output (0644) but not for the HTML output (os.Create default). This is an introduced inconsistency in the output-writing behavior.
tools/codegen/cmd/featuregate-test-analyzer.go[205-215]
tools/codegen/cmd/featuregate-test-analyzer.go[282-301]
Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution ### Issue description HTML report file permissions are umask-dependent due to `os.Create`, unlike the markdown report which is explicitly `0644`. ### Issue Context In CI or shared environments, differing umasks can cause HTML artifacts to be group-writable or otherwise inconsistent. ### Fix Focus Areas - tools/codegen/cmd/featuregate-test-analyzer.go[282-301]
i Copy this prompt and use it to remediate the issue with your preferred AI generation tools
i The new review experience is currently in Beta. Learn more
small quality of life improvement to the html for feature promotion, this addition will allow sorting by pass rate and make it easier to identify where a feature is lacking validation by color coating the pass rate. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: ehila <ehila@redhat.com>
78eecda to
9650576
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick comments (2)
tools/codegen/cmd/featuregate-test-analyzer.go (1)
222-235: Make column ordering deterministic across NetworkStack variants.Line 224 uses
OrderedJobVariants, which doesn’t compareNetworkStack. When Topology/Cloud/Architecture match (ipv4/ipv6/dual), the sort order can fluctuate, changingColIndexbetween runs and making the report harder to scan. Consider addingNetworkStackas a final tie-breaker (or use a stable sort).🔧 Suggested comparator update (outside this block)
func (a OrderedJobVariants) Less(i, j int) bool { if strings.Compare(a[i].Topology, a[j].Topology) < 0 { return true } else if strings.Compare(a[i].Topology, a[j].Topology) > 0 { return false } if strings.Compare(a[i].Cloud, a[j].Cloud) < 0 { return true } else if strings.Compare(a[i].Cloud, a[j].Cloud) > 0 { return false } if strings.Compare(a[i].Architecture, a[j].Architecture) < 0 { return true } else if strings.Compare(a[i].Architecture, a[j].Architecture) > 0 { return false } + + if strings.Compare(a[i].NetworkStack, a[j].NetworkStack) < 0 { + return true + } else if strings.Compare(a[i].NetworkStack, a[j].NetworkStack) > 0 { + return false + } return false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/codegen/cmd/featuregate-test-analyzer.go` around lines 222 - 235, The current sorting of jobVariants uses OrderedJobVariants which doesn't include NetworkStack, causing nondeterministic ColIndex when Topology/Cloud/Architecture tie; update the comparator used by OrderedJobVariants (the comparator implementation for OrderedJobVariants) to include NetworkStack as the final tie-breaker (or switch to a stable sort while adding NetworkStack comparison) so jobVariants (the unsorted list produced from jobVariantsSet) is deterministically ordered before building variants (utils.HTMLVariantColumn) and assigning ColIndex.tools/codegen/pkg/utils/html.go (1)
89-162: Add keyboard support for sortable headers.Line 89 and Line 91 create clickable
<th>elements, but they’re mouse-only. Consider addingtabindex/roleand a key handler (Enter/Space) so sorting is accessible via keyboard.♿ Proposed accessibility tweak
- <th class="sortable sort-indicator" data-table="{{$fgIdx}}" data-col="0" data-sort="text">Test Name</th> + <th class="sortable sort-indicator" data-table="{{$fgIdx}}" data-col="0" data-sort="text" role="button" tabindex="0">Test Name</th> ... - <th class="sortable sort-indicator" data-table="{{$fgIdx}}" data-col="{{$v.ColIndex}}" data-sort="percent"> + <th class="sortable sort-indicator" data-table="{{$fgIdx}}" data-col="{{$v.ColIndex}}" data-sort="percent" role="button" tabindex="0"> ... - document.querySelectorAll('th.sortable').forEach(function(th) { - th.addEventListener('click', function() { - sortTable(this.dataset.table, parseInt(this.dataset.col), this.dataset.sort, this); - }); - }); + document.querySelectorAll('th.sortable').forEach(function(th) { + function invokeSort() { + sortTable(th.dataset.table, parseInt(th.dataset.col), th.dataset.sort, th); + } + th.addEventListener('click', invokeSort); + th.addEventListener('keydown', function(e) { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + invokeSort(); + } + }); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/codegen/pkg/utils/html.go` around lines 89 - 162, The sortable table headers are only clickable by mouse; update the <th class="sortable"> generation to include tabindex="0" and role="button" (where headers are created in the template loop for $fg.Variants and the Test Name header) and add a keydown handler alongside the existing click handler in the DOMContentLoaded setup to call sortTable when Enter (keyCode 13 or key === 'Enter') or Space (keyCode 32 or key === ' ') is pressed (call sortTable with this.dataset.table, parseInt(this.dataset.col), this.dataset.sort, this), ensuring you preventDefault for Space to avoid scrolling; keep the existing sortTable function unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tools/codegen/cmd/featuregate-test-analyzer.go`:
- Around line 222-235: The current sorting of jobVariants uses
OrderedJobVariants which doesn't include NetworkStack, causing nondeterministic
ColIndex when Topology/Cloud/Architecture tie; update the comparator used by
OrderedJobVariants (the comparator implementation for OrderedJobVariants) to
include NetworkStack as the final tie-breaker (or switch to a stable sort while
adding NetworkStack comparison) so jobVariants (the unsorted list produced from
jobVariantsSet) is deterministically ordered before building variants
(utils.HTMLVariantColumn) and assigning ColIndex.
In `@tools/codegen/pkg/utils/html.go`:
- Around line 89-162: The sortable table headers are only clickable by mouse;
update the <th class="sortable"> generation to include tabindex="0" and
role="button" (where headers are created in the template loop for $fg.Variants
and the Test Name header) and add a keydown handler alongside the existing click
handler in the DOMContentLoaded setup to call sortTable when Enter (keyCode 13
or key === 'Enter') or Space (keyCode 32 or key === ' ') is pressed (call
sortTable with this.dataset.table, parseInt(this.dataset.col),
this.dataset.sort, this), ensuring you preventDefault for Space to avoid
scrolling; keep the existing sortTable function unchanged.
@eggfoobar: all tests passed!
Full PR test history. Your PR dashboard.
Details
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not blocking for this PR, but I wonder if we oughta remove the additional markdown file artifact? I never knew it existed until now, and I suspect it isn't actually used all that often.
It doesn't hurt to keep it, but if it isn't actually buying us anything it also seems like we could reduce a failure mode if it can't write the markdown file.
@JoelSpeed Do you know of any specific uses of the markdown file artifact for feature promotion runs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing specific that I know of no
@everettraven
everettraven
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a worthwhile change and I don't have any specific concerns about the implementation here.
/lgtm
openshift-ci-robot
commented
Feb 18, 2026
Pipeline controller notification
No second-stage tests were triggered for this PR.
This can happen when:
- The changed files don't match any
pipeline_run_if_changedpatterns - All files match
pipeline_skip_if_only_changedpatterns - No pipeline-controlled jobs are defined for the
masterbranch
Use /test ? to see all available tests.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: everettraven
The full list of commands accepted by this bot can be found here.
The pull request process is described here
Details
Needs approval from an approver in each of these files:(削除) OWNERS (削除ここまで)[everettraven]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
small quality of life improvement to the html for feature promotion, this addition will allow sorting by pass rate and make it easier to identify where a feature is lacking validation by color coating the pass rate.
Example:
image