Skip to content

Improvements and fixes for new Feature Gate page #3779

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-bot merged 8 commits into from
Jul 15, 2026
Merged

Improvements and fixes for new Feature Gate page #3779

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
811082c
Fix hook ordering
dgoodwin Jul 14, 2026
249eaba
Fix ui_details links for feature gates
dgoodwin Jul 14, 2026
49c3856
Continue using install success for install feature gates
dgoodwin Jul 15, 2026
1e48e68
Fix clickthrough on test details from fg details page
dgoodwin Jul 15, 2026
c066822
Re-use test details table on feature gate details page
dgoodwin Jul 15, 2026
1f25eec
Tooltip for why test count differs from table
dgoodwin Jul 15, 2026
99607ae
Auto-display tests by capability if annotations tab is empty
dgoodwin Jul 15, 2026
8d9ec0f
Review fixes
dgoodwin Jul 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions cmd/sippy/seed_data.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,12 +185,22 @@ var syntheticJobs = []syntheticJobDef{
"Capability": "NetworkSegmentation",
},
},
{
nameTemplate: "periodic-ci-openshift-release-master-ci-%s-e2e-aws-ovn-amd64-capability-awsdualstackinstall",
variants: map[string]string{
"Platform": "aws", "Architecture": "amd64", "Network": "ovn",
"Topology": "ha", "Installer": "ipi", "FeatureSet": "default",
"Suite": "parallel", "Upgrade": "none", "LayeredProduct": "none",
"Capability": "AWSDualStackInstall",
},
},
}

// Job template constants for referencing specific jobs in test specs.
const awsAmd64Parallel = "periodic-ci-openshift-release-master-ci-%s-e2e-aws-ovn-amd64"
const awsArm64Parallel = "periodic-ci-openshift-release-master-ci-%s-e2e-aws-ovn-arm64"
const gcpAmd64Parallel = "periodic-ci-openshift-release-master-ci-%s-e2e-gcp-ovn-amd64"
const awsAmd64CapabilityAWSDualStackInstall = "periodic-ci-openshift-release-master-ci-%s-e2e-aws-ovn-amd64-capability-awsdualstackinstall"

// allJobTemplates returns name templates from syntheticJobs for use in test specs
// that should run on every job (e.g. install tests).
Expand Down Expand Up @@ -363,6 +373,13 @@ var syntheticTests = []syntheticTestSpec{
awsAmd64Parallel: {"4.22": {50, 48, 0}},
},
},
{
testID: "test-cap-aws-dual-stack-install", testName: "install should succeed: infrastructure",
component: "Installer / openshift-installer", capabilities: []string{"install"},
jobCounts: map[string]map[string]testCount{
awsAmd64CapabilityAWSDualStackInstall: {"4.22": {50, 47, 0}},
},
},
Comment thread
coderabbitai[bot] marked this conversation as resolved.
// --- Install / health indicator tests: run on every job, every release ---
{
testID: "test-install-overall", testName: "install should succeed: overall",
Expand Down
11 changes: 9 additions & 2 deletions pkg/sippyserver/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -750,17 +750,24 @@ func injectFeatureGateHATEOASLinks(fg *apitype.FeatureGate, release, baseAPIURL,
}
fg.Links["tests_by_annotation"] = buildFilteredTestsURL(baseAPIURL, release, annotationFilter)

// Installer gates currently run a broad conformance suite where full passes aren't
// required, so "install should succeed" is the meaningful signal. Switch to
// "openshift-tests should work" once installer jobs run a minimal conformance suite.
capabilityTestName := "openshift-tests should work"
if strings.Contains(fg.FeatureGate, "Install") {
capabilityTestName = "install should succeed"
}
capabilityFilter := filter.Filter{
Items: []filter.FilterItem{
{Field: "name", Operator: filter.OperatorContains, Value: "openshift-tests should work"},
{Field: "name", Operator: filter.OperatorContains, Value: capabilityTestName},
{Field: "variants", Operator: filter.OperatorContains, Value: fmt.Sprintf("Capability:%s", fg.FeatureGate)},
},
LinkOperator: filter.LinkOperatorAnd,
}
fg.Links["tests_by_capability"] = buildFilteredTestsURL(baseAPIURL, release, capabilityFilter)

fg.Links["ui_detail"] = fmt.Sprintf(
"%s/feature_gates/%s/%s",
"%s/sippy-ng/feature_gates/%s/%s",
baseFrontendURL, release, url.PathEscape(fg.FeatureGate))
}

Expand Down
17 changes: 14 additions & 3 deletions sippy-ng/src/App.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,12 +260,23 @@ const JobsWrapper = () => {
}

const FeatureGateDetailWrapper = () => {
let { release, feature_gate } = useParams()
const { release, feature_gate } = useParams()
const navigate = useNavigate()
const releases = React.useContext(ReleasesContext)

const effectiveRelease =
release === 'latest' ? findFirstNonGARelease(releases) : release

React.useEffect(() => {
if (release === 'latest' && effectiveRelease) {
navigate(`/feature_gates/${effectiveRelease}/${feature_gate}`, {
replace: true,
})
}
}, [release, effectiveRelease, feature_gate, navigate])

if (release === 'latest') {
release = findFirstNonGARelease(React.useContext(ReleasesContext))
navigate(`/feature_gates/${release}/${feature_gate}`, { replace: true })
return null
}

return (
Expand Down
194 changes: 71 additions & 123 deletions sippy-ng/src/tests/FeatureGateDetail.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,126 +7,17 @@ import {
Container,
Tab,
Tabs,
Tooltip,
Typography,
} from '@mui/material'
import { DataGrid } from '@mui/x-data-grid'
import { Link } from 'react-router-dom'
import { pathForTestByVariant, safeEncodeURIComponent } from '../helpers'
import { safeEncodeURIComponent } from '../helpers'
import Alert from '@mui/material/Alert'
import HelpOutlineIcon from '@mui/icons-material/HelpOutline'
import PropTypes from 'prop-types'
import React, { Fragment, useEffect } from 'react'
import SimpleBreadcrumbs from '../components/SimpleBreadcrumbs'

function TestResultsSection({ title, apiUrl, release }) {
const [rows, setRows] = React.useState([])
const [isLoaded, setLoaded] = React.useState(false)
const [fetchError, setFetchError] = React.useState('')
const [sortModel, setSortModel] = React.useState([
{ field: 'current_pass_percentage', sort: 'asc' },
])

useEffect(() => {
if (!apiUrl) {
setRows([])
setLoaded(true)
return
}
setLoaded(false)
setFetchError('')

fetch(apiUrl)
.then((response) => {
if (response.status !== 200) {
throw new Error('server returned ' + response.status)
}
return response.json()
})
.then((json) => {
setRows(json || [])
setLoaded(true)
})
.catch((error) => {
setFetchError('Could not retrieve ' + title + ': ' + error)
setLoaded(true)
})
}, [apiUrl, title])

const columns = [
{
field: 'name',
headerName: 'Test Name',
flex: 4,
renderCell: (params) => (
<Link to={pathForTestByVariant(release, params.value)}>
{params.value}
</Link>
),
},
{
field: 'current_successes',
headerName: 'Current Successes',
type: 'number',
flex: 1,
},
{
field: 'current_failures',
headerName: 'Current Failures',
type: 'number',
flex: 1,
},
{
field: 'current_flakes',
headerName: 'Current Flakes',
type: 'number',
flex: 1,
},
{
field: 'current_pass_percentage',
headerName: 'Current Pass %',
type: 'number',
flex: 1,
renderCell: (params) =>
params.value != null ? params.value.toFixed(2) + '%' : '',
},
{
field: 'current_runs',
headerName: 'Current Runs',
type: 'number',
flex: 1,
},
]

if (fetchError) {
return <Alert severity="error">{fetchError}</Alert>
}

return (
<Box sx={{ mt: 2 }}>
<Typography variant="h6" sx={{ mb: 1 }}>
{title} ({isLoaded ? rows.length : '...'} tests)
</Typography>
<DataGrid
loading={!isLoaded}
rows={rows}
columns={columns}
getRowId={(row) => row.name}
getRowHeight={() => 'auto'}
autoHeight={true}
rowsPerPageOptions={[10, 25, 50]}
pageSize={25}
sortModel={sortModel}
onSortModelChange={setSortModel}
disableSelectionOnClick
/>
</Box>
)
}

TestResultsSection.propTypes = {
title: PropTypes.string.isRequired,
apiUrl: PropTypes.string,
release: PropTypes.string.isRequired,
}
import TestTable from './TestTable'

export default function FeatureGateDetail(props) {
const { release, featureGate } = props
Expand Down Expand Up @@ -178,6 +69,50 @@ export default function FeatureGateDetail(props) {
})
}, [release, featureGate])

const tabAutoSelected = React.useRef(false)

// <= 1 because the API always returns an implicit "Overall" row
const handleAnnotationDataLoaded = React.useCallback((count) => {
if (!tabAutoSelected.current && count <= 1) {
setActiveTab(1)
tabAutoSelected.current = true
}
}, [])
Comment on lines +75 to +80

@coderabbitai coderabbitai Bot Jul 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Only auto-switch when annotations are empty.

Line 75 uses count <= 1, so a gate with one annotated test is redirected to capability results despite having annotation data. Change this to count === 0.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@sippy-ng/src/tests/FeatureGateDetail.js` around lines 74 - 79, Update
handleAnnotationDataLoaded so automatic tab selection occurs only when count ===
0, preserving the existing setActiveTab and tabAutoSelected behavior for empty
annotation data.

@dgoodwin dgoodwin Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is required because there's an implicit Overall row. We should comment this.

@coderabbitai coderabbitai Bot Jul 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dgoodwin Thanks for the clarification — that makes sense, and it means my original suggestion to change this to count === 0 was incorrect. Since TestTable always includes an implicit "Overall" row, count === 1 actually means there are no real per-test annotation rows, so count <= 1 is the right threshold.

Here's a suggested comment to make that explicit:

const handleAnnotationDataLoaded = React.useCallback((count) => {
  // TestTable always includes an implicit "Overall" row, so a count of 1
  // means there are no real annotated tests. Auto-switch to the capability
  // tab in that case.
  if (!tabAutoSelected.current && count <= 1) {
    setActiveTab(1)
    tabAutoSelected.current = true
  }
}, [])
✏️ Learnings added 
Learnt from: dgoodwin
Repo: openshift/sippy PR: 3779
File: sippy-ng/src/tests/FeatureGateDetail.js:74-79
Timestamp: 2026-07-15T18:06:01.572Z
Learning: In sippy-ng/src/tests/FeatureGateDetail.js, `TestTable` (sippy-ng/src/tests/TestTable.js) always includes an implicit "Overall" row in its loaded row count reported via `onDataLoaded`. So in `handleAnnotationDataLoaded`, checking `count <= 1` (not `count === 0`) is the correct way to detect "no real annotated tests" and auto-switch the active tab to "Tests by Capability".

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


const annotationFilter = {
items: [
{
columnField: 'name',
operatorValue: 'contains',
value: `FeatureGate:${featureGate}]`,
},
],
}

// Installer gates currently run a broad conformance suite where full passes
// aren't required, so "install should succeed" is the meaningful signal.
// Switch to "openshift-tests should work" once installer jobs run a minimal
// conformance suite.
const capabilityTestName = featureGate.includes('Install')
? 'install should succeed'
: 'openshift-tests should work'

const capabilityFilter = {
items: [
{
columnField: 'name',
operatorValue: 'contains',
value: capabilityTestName,
},
{
columnField: 'variants',
operatorValue: 'has entry containing',
value: `Capability:${featureGate}`,
},
],
linkOperator: 'and',
}

if (fetchError) {
return (
<Container size="xl">
Expand Down Expand Up @@ -231,6 +166,16 @@ export default function FeatureGateDetail(props) {
</Typography>
<Typography variant="body1" sx={{ mb: 1 }}>
<strong>Total Test Count:</strong> {gate.unique_test_count}
<Tooltip title="Unique test names. The tables below may show more rows due to per-variant breakdowns.">
<HelpOutlineIcon
fontSize="small"
sx={{
ml: 0.5,
verticalAlign: 'middle',
color: 'text.secondary',
}}
/>
</Tooltip>
</Typography>
<Typography variant="body1" component="div">
<strong>Enabled:</strong>{' '}
Expand All @@ -253,21 +198,24 @@ export default function FeatureGateDetail(props) {
</Tabs>
</Box>

{activeTab === 0 && gate.links && (
<TestResultsSection
title="Tests tagged with FeatureGate annotation"
apiUrl={gate.links.tests_by_annotation}
<Box sx={{ display: activeTab === 0 ? 'block' : 'none' }}>
<TestTable
key={'fg-annotation-' + featureGate}
release={release}
collapse={false}
filterModel={annotationFilter}
onDataLoaded={handleAnnotationDataLoaded}
/>
)}
</Box>

{activeTab === 1 && gate.links && (
<TestResultsSection
title="Tests matching capability variant"
apiUrl={gate.links.tests_by_capability}
<Box sx={{ display: activeTab === 1 ? 'block' : 'none' }}>
<TestTable
key={'fg-capability-' + featureGate}
release={release}
collapse={false}
filterModel={capabilityFilter}
/>
)}
</Box>
Comment on lines +201 to +218

@coderabbitai coderabbitai Bot Jul 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Isolate URL state for the two TestTable instances.

Both mounted tables use the same URL-backed query keys (filters, sort, sortField, view, period, and pageSize). Filtering or sorting one tab updates the hidden tab too; when revisited, it can fetch with the other tab’s filter and show incorrect results. Add a per-instance query-state prefix (or make this state fully controlled) and pass distinct values for annotation and capability tables.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@sippy-ng/src/tests/FeatureGateDetail.js` around lines 200 - 217, Update the
two TestTable instances in FeatureGateDetail so their URL-backed query state is
isolated per tab. Provide distinct query-state prefixes or controlled values for
the annotation and capability tables covering filters, sort, sortField, view,
period, and pageSize, while preserving each table’s existing filterModel
behavior.

@dgoodwin dgoodwin Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beyond scope for this I think.

@coderabbitai coderabbitai Bot Jul 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dgoodwin, understood — acknowledged as out of scope for this PR. Let me know if you'd like me to open a follow-up issue to track isolating the URL-backed query state (filters, sort, sortField, view, period, pageSize) for the two TestTable instances, and I'll create it.

</Container>
</Fragment>
)
Expand Down
4 changes: 3 additions & 1 deletion sippy-ng/src/tests/FeatureGates.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,9 @@ export default function FeatureGates(props) {
if (params.row.links && params.row.links.ui_detail) {
try {
const url = new URL(params.row.links.ui_detail)
return url.pathname
const basename = '/sippy-ng'
const path = url.pathname
return path.startsWith(basename) ? path.slice(basename.length) : path
} catch (e) {
// fall through
}
Expand Down
15 changes: 9 additions & 6 deletions sippy-ng/src/tests/TestTable.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -962,13 +962,13 @@ function TestTable(props) {
return response.json()
})
.then((json) => {
if (json != null) {
setRows(json)
} else {
setRows([])
}
const data = json != null ? json : []
setRows(data)
setLoaded(true)
setSearching(false)
if (props.onDataLoaded) {
props.onDataLoaded(data.length)
}
})
.catch((error) => {
setFetchError(
Expand All @@ -991,10 +991,12 @@ function TestTable(props) {
setSearching(true)
fetchData()
} else {
// Mark as loaded so we don't show loading spinner, and clear any stale data
setRows([])
setLoaded(true)
setSearching(false)
if (props.onDataLoaded) {
props.onDataLoaded(0)
}
}

prevLocation.current = location
Expand Down Expand Up @@ -1178,6 +1180,7 @@ TestTable.propTypes = {
sortField: PropTypes.string,
rowsPerPageOptions: PropTypes.array,
view: PropTypes.string,
onDataLoaded: PropTypes.func,
}

export default withStyles(generateClasses(TEST_THRESHOLDS))(TestTable)
Loading