NO-ISSUE: Add TechPreview and feature-gate E2E test filtering infrastructure#16770
NO-ISSUE: Add TechPreview and feature-gate E2E test filtering infrastructure#16770perdasilva wants to merge 8 commits into
Conversation
Spec for populating test-prow-e2e-techpreview.sh and test-prow-playwright-e2e-techpreview.sh with a @tech-preview Playwright tag + techPreviewOnly fixture approach and an inverted Cypress before() guard. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…upport Add @feature-gate tag, requireFeatureGate Playwright fixture, and requireFeatureGate Cypress guard so tests can target a specific feature gate and run on both TechPreview and GA clusters as the gate matures, without needing test changes at graduation time. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
… error handling - Replace page.evaluate() in techPreviewOnly with k8sClient.getClusterCustomResource() to avoid TypeError when window.SERVER_FLAGS is undefined on about:blank - Surface infrastructure errors in requireFeatureGate instead of silently skipping Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@perdasilva: This pull request explicitly references no jira issue. DetailsIn response to this:
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. |
|
/hold |
openshift-ci
Bot
added
the
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: perdasilva The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
WalkthroughAdds documented and implemented TechPreview and feature-gate filtering for Playwright and Cypress E2E tests, including cluster-state guards, tagged test coverage, dedicated CI scripts, and a Playwright ChangesTechPreview E2E filtering
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)Playwright filteringsequenceDiagram
participant CI
participant Playwright
participant k8sClient
participant FeatureGate
CI->>Playwright: Run `@tech-preview`|`@feature-gate` tests
Playwright->>k8sClient: Read cluster FeatureGate
k8sClient->>FeatureGate: Fetch cluster configuration
FeatureGate-->>Playwright: Return featureSet and enabled gates
Playwright-->>CI: Execute or skip tests
Cypress filteringsequenceDiagram
participant CI
participant Cypress
participant tech-preview-guard
participant oc
CI->>Cypress: Run TechPreview packages
Cypress->>tech-preview-guard: Register suite guard
tech-preview-guard->>oc: Query cluster FeatureGate
oc-->>tech-preview-guard: Return featureSet and enabled gates
tech-preview-guard-->>Cypress: Continue or skip suite
Suggested labels: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (12 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/hold experimental |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with 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.
Inline comments:
In `@docs/superpowers/specs/2026-07-16-techpreview-e2e-test-filtering-design.md`:
- Around line 64-80: The documentation describes an outdated SERVER_FLAGS-based
contract; update
docs/superpowers/specs/2026-07-16-techpreview-e2e-test-filtering-design.md at
lines 64-80 to document FeatureGate-CR lookup through k8sClient, lines 89-100 to
show valid narrowing of the CR response, and lines 113-126 to match the existing
helper and return contract. Update
docs/superpowers/plans/2026-07-16-techpreview-e2e-test-filtering.md at lines
28-55 so its implementation steps match the actual fixture behavior.
In `@frontend/e2e/fixtures/index.ts`:
- Around line 63-79: Validate the FeatureGate payload in the techPreviewOnly
fixture before calling test.skip: require spec.featureSet to be a string and
fail with an infrastructure/data error when missing or malformed. Also update
the active status.featureGates handling in the same fixture to require the
section and enabled entries to be present and correctly shaped, failing rather
than treating malformed data as a disabled gate; apply these changes at
frontend/e2e/fixtures/index.ts lines 63-79 and 83-105.
In `@frontend/e2e/tests/olm/olmv1-catalog.spec.ts`:
- Around line 9-10: Update the URL assertion in the OLM catalog test to group
the route alternatives and anchor the regex at the route boundary or URL end,
allowing only /k8s/cluster/clusterserviceversions or /software-catalog matches
while preserving the existing timeout.
In `@frontend/integration-tests/.eslintrc.cjs`:
- Around line 9-13: Update the parserOptions configuration in the ESLint config
to set sourceType to module, allowing the exported requireFeatureGate symbol in
tech-preview-guard.ts to parse correctly while preserving the existing parser
and project settings.
In `@frontend/integration-tests/support/tech-preview-guard.ts`:
- Around line 14-21: Update the before hook around cy.exec for the featuregate
lookup to inspect result.exitCode, falling back to result.code for older Cypress
versions, and fail immediately when the command fails. Only call this.skip()
after a successful command whose trimmed stdout is not TechPreviewNoUpgrade;
apply the same handling to both hooks if the file contains another equivalent oc
check.
In `@test-prow-playwright-e2e-techpreview.sh`:
- Around line 35-39: Split the setup assignments in
test-prow-playwright-e2e-techpreview.sh lines 35-39 and
test-prow-e2e-techpreview.sh lines 22-26: assign the kubeadmin password and
console URL to temporary variables, export them only after successful retrieval,
and explicitly fail when either required value is empty. Preserve the existing
set +x protection around the password.
In `@test-prow-playwright-e2e.sh`:
- Around line 62-67: Update the scenario list in the header of
test-prow-playwright-e2e.sh to include feature-gate alongside e2e, release, and
smoke, keeping the documentation consistent with the SCENARIO branch and
unknown-scenario message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 56cf2a0e-1aeb-4fa8-b869-b97371e0a05b
📒 Files selected for processing (11)
docs/superpowers/plans/2026-07-16-techpreview-e2e-test-filtering.mddocs/superpowers/specs/2026-07-16-techpreview-e2e-test-filtering-design.mdfrontend/.eslintignorefrontend/e2e/fixtures/index.tsfrontend/e2e/tests/olm/olmv1-catalog.spec.tsfrontend/integration-tests/.eslintrc.cjsfrontend/integration-tests/support/tech-preview-guard.tsfrontend/integration-tests/tsconfig.jsontest-prow-e2e-techpreview.shtest-prow-playwright-e2e-techpreview.shtest-prow-playwright-e2e.sh
| Added to the `TestFixtures` type and `base.extend()` chain in `frontend/e2e/fixtures/index.ts`. | ||
| Reads `SERVER_FLAGS.techPreview` from the page and skips if the cluster is not on TechPreview. | ||
|
|
||
| ```ts | ||
| type TestFixtures = { | ||
| cleanup: CleanupFixture; | ||
| techPreviewOnly: void; | ||
| requireFeatureGate: (gateName: string) => Promise<void>; | ||
| }; | ||
|
|
||
| // In base.extend(): | ||
| techPreviewOnly: async ({ page }, use) => { | ||
| const isTechPreview = await page.evaluate(() => window.SERVER_FLAGS.techPreview); | ||
| test.skip(!isTechPreview, 'This test requires a TechPreviewNoUpgrade cluster'); | ||
| await use(); | ||
| }, | ||
| ``` |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
The Playwright documentation describes an older contract.
docs/superpowers/specs/2026-07-16-techpreview-e2e-test-filtering-design.md#L64-L80: document FeatureGate-CR lookup throughk8sClient, notSERVER_FLAGS.docs/superpowers/specs/2026-07-16-techpreview-e2e-test-filtering-design.md#L89-L100: show valid narrowing for the CR response.docs/superpowers/specs/2026-07-16-techpreview-e2e-test-filtering-design.md#L113-L126: describe the existing helper and its current return contract.docs/superpowers/plans/2026-07-16-techpreview-e2e-test-filtering.md#L28-L55: update the implementation steps to match the actual fixture.
📍 Affects 2 files
docs/superpowers/specs/2026-07-16-techpreview-e2e-test-filtering-design.md#L64-L80(this comment)docs/superpowers/specs/2026-07-16-techpreview-e2e-test-filtering-design.md#L89-L100docs/superpowers/specs/2026-07-16-techpreview-e2e-test-filtering-design.md#L113-L126docs/superpowers/plans/2026-07-16-techpreview-e2e-test-filtering.md#L28-L55
🤖 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 `@docs/superpowers/specs/2026-07-16-techpreview-e2e-test-filtering-design.md`
around lines 64 - 80, The documentation describes an outdated SERVER_FLAGS-based
contract; update
docs/superpowers/specs/2026-07-16-techpreview-e2e-test-filtering-design.md at
lines 64-80 to document FeatureGate-CR lookup through k8sClient, lines 89-100 to
show valid narrowing of the CR response, and lines 113-126 to match the existing
helper and return contract. Update
docs/superpowers/plans/2026-07-16-techpreview-e2e-test-filtering.md at lines
28-55 so its implementation steps match the actual fixture behavior.
| techPreviewOnly: async ({ k8sClient }, use) => { | ||
| let isTechPreview = false; | ||
| try { | ||
| const fg = await k8sClient.getClusterCustomResource( | ||
| 'config.openshift.io', | ||
| 'v1', | ||
| 'featuregates', | ||
| 'cluster', | ||
| ); | ||
| isTechPreview = (fg as any)?.spec?.featureSet === 'TechPreviewNoUpgrade'; | ||
| } catch { | ||
| // If the CR can't be read, don't skip — surface the infrastructure error | ||
| throw new Error( | ||
| `techPreviewOnly: could not read FeatureGate CR 'cluster': check cluster connectivity`, | ||
| ); | ||
| } | ||
| test.skip(!isTechPreview, 'This test requires a TechPreviewNoUpgrade cluster'); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files frontend/e2e/fixtures/index.ts
wc -l frontend/e2e/fixtures/index.ts
cat -n frontend/e2e/fixtures/index.ts | sed -n '1,180p'Repository: openshift/console
Length of output: 4895
🏁 Script executed:
#!/bin/bash
set -euo pipefail
wc -l frontend/e2e/fixtures/index.ts
sed -n '1,180p' frontend/e2e/fixtures/index.ts | nl -baRepository: openshift/console
Length of output: 230
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the Kubernetes client method and nearby usages of feature gate handling.
ast-grep outline frontend/e2e/clients/kubernetes-client.ts --view expanded || true
printf '\n---\n'
rg -n "getClusterCustomResource|featureGates|featureSet|TechPreviewNoUpgrade" frontend/e2e -S
printf '\n---\n'
cat -n frontend/e2e/clients/kubernetes-client.ts | sed -n '1,260p'Repository: openshift/console
Length of output: 14376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline frontend/e2e/clients/kubernetes-client.ts --view expanded || true
printf '\n---\n'
rg -n "getClusterCustomResource|featureGates|featureSet|TechPreviewNoUpgrade" frontend/e2e -S
printf '\n---\n'
sed -n '1,260p' frontend/e2e/clients/kubernetes-client.ts | cat -nRepository: openshift/console
Length of output: 14376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "interface FeatureGate|type FeatureGate|featureSet:|featureGates:" frontend -S
printf '\n---\n'
rg -n "FeatureGate" frontend -g '!**/node_modules/**' -SRepository: openshift/console
Length of output: 1250
🏁 Script executed:
#!/bin/bash
set -euo pipefail
wc -l frontend/integration-tests/support/tech-preview-guard.ts
sed -n '1,220p' frontend/integration-tests/support/tech-preview-guard.ts | cat -nRepository: openshift/console
Length of output: 1629
Guard against malformed FeatureGate payloads before skipping tests.
frontend/e2e/fixtures/index.ts#L63-L79: fail whenspec.featureSetis missing or not a string; otherwise a bad CR looks like a non-TechPreview cluster and silently skips the suite.frontend/e2e/fixtures/index.ts#L83-L105: fail when the activestatus.featureGatessection orenabledentries are missing or malformed; otherwise a bad CR looks like the gate is disabled.
📍 Affects 1 file
frontend/e2e/fixtures/index.ts#L63-L79(this comment)frontend/e2e/fixtures/index.ts#L83-L105
🤖 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 `@frontend/e2e/fixtures/index.ts` around lines 63 - 79, Validate the
FeatureGate payload in the techPreviewOnly fixture before calling test.skip:
require spec.featureSet to be a string and fail with an infrastructure/data
error when missing or malformed. Also update the active status.featureGates
handling in the same fixture to require the section and enabled entries to be
present and correctly shaped, failing rather than treating malformed data as a
disabled gate; apply these changes at frontend/e2e/fixtures/index.ts lines 63-79
and 83-105.
Source: Path instructions
| await expect(page).toHaveURL(/\/k8s\/cluster\/clusterserviceversions|\/software-catalog/, { | ||
| timeout: 30_000, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- target file ---'
sed -n '1,120p' frontend/e2e/tests/olm/olmv1-catalog.spec.ts | cat -n
echo
echo '--- similar URL assertions in frontend/e2e/tests/olm ---'
rg -n "toHaveURL\\(" frontend/e2e/tests/olm -SRepository: openshift/console
Length of output: 884
Constrain the URL regex. Group the alternation and anchor it to the route boundary/end of URL so only /k8s/cluster/clusterserviceversions or /software-catalog matches here.
🤖 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 `@frontend/e2e/tests/olm/olmv1-catalog.spec.ts` around lines 9 - 10, Update the
URL assertion in the OLM catalog test to group the route alternatives and anchor
the regex at the route boundary or URL end, allowing only
/k8s/cluster/clusterserviceversions or /software-catalog matches while
preserving the existing timeout.
Source: Path instructions
| parser: '@typescript-eslint/parser', | ||
| parserOptions: { | ||
| project: './tsconfig.json', | ||
| tsconfigRootDir: __dirname, | ||
| ecmaVersion: 2018, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
rg -n "sourceType|export function" frontend/integration-tests/.eslintrc.cjs frontend/integration-tests/support/tech-preview-guard.tsRepository: openshift/console
Length of output: 276
Configure ESLint for module syntax. tech-preview-guard.ts exports requireFeatureGate, but this config still parses the file as a script. Add parserOptions.sourceType: 'module' so ESLint can handle the export syntax.
🤖 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 `@frontend/integration-tests/.eslintrc.cjs` around lines 9 - 13, Update the
parserOptions configuration in the ESLint config to set sourceType to module,
allowing the exported requireFeatureGate symbol in tech-preview-guard.ts to
parse correctly while preserving the existing parser and project settings.
| before(function () { | ||
| cy.exec('oc get featuregate cluster -o jsonpath={.spec.featureSet}', { | ||
| failOnNonZeroExit: false, | ||
| }).then((result) => { | ||
| if (result.stdout.trim() !== 'TechPreviewNoUpgrade') { | ||
| this.skip(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -e
git ls-files frontend/integration-tests/support/tech-preview-guard.ts
wc -l frontend/integration-tests/support/tech-preview-guard.ts
cat -n frontend/integration-tests/support/tech-preview-guard.tsRepository: openshift/console
Length of output: 1686
🏁 Script executed:
set -e
rg -n "requireFeatureGate|tech-preview-guard|failOnNonZeroExit" frontend/integration-tests -SRepository: openshift/console
Length of output: 775
🌐 Web query:
Cypress cy.exec failOnNonZeroExit false result.code stdout documentation
💡 Result:
When using cy.exec in Cypress, setting the option failOnNonZeroExit to false prevents the test from failing if the executed command returns a non-zero exit code [1][2]. The cy.exec command yields an object that allows you to inspect the results of the command. As of Cypress v15, this result object includes the following key properties [2][3]: exitCode: The numeric exit code returned by the command (formerly named code in older versions) [3]. stdout: The standard output string from the command [2]. stderr: The standard error string from the command [2]. Example usage: cy.exec('your-command', { failOnNonZeroExit: false }).then((result) => { // Use result.exitCode to check if the command succeeded or failed expect(result.exitCode).to.eq(1); // Use result.stdout or result.stderr to verify output cy.log(result.stdout); }); Note: If you are upgrading from an older version of Cypress, ensure you use exitCode, as the code property was renamed in v15 [3]. The command must still eventually exit, or it will time out according to the configured execTimeout [1][2].
Citations:
- 1: https://docs.cypress.io/llm/markdown/api/commands/exec.md
- 2: https://github.com/cypress-io/cypress-documentation/blob/main/docs/api/commands/exec.mdx
- 3: [v15]
cy.execreturn type incorrect,codeshould be renamed toexitCodecypress-io/cypress#32875
Surface oc failures instead of skipping the suite. Both hooks ignore the exit status. If oc get featuregate fails, failOnNonZeroExit: false can turn an infrastructure problem into this.skip(). Check result.exitCode (or result.code on older Cypress) and fail before skipping.
🤖 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 `@frontend/integration-tests/support/tech-preview-guard.ts` around lines 14 -
21, Update the before hook around cy.exec for the featuregate lookup to inspect
result.exitCode, falling back to result.code for older Cypress versions, and
fail immediately when the command fails. Only call this.skip() after a
successful command whose trimmed stdout is not TechPreviewNoUpgrade; apply the
same handling to both hooks if the file contains another equivalent oc check.
| # don't log kubeadmin-password | ||
| set +x | ||
| export BRIDGE_KUBEADMIN_PASSWORD="$(cat "${KUBEADMIN_PASSWORD_FILE:-${INSTALLER_DIR}/auth/kubeadmin-password}")" | ||
| set -x | ||
| export BRIDGE_BASE_ADDRESS="$(oc get consoles.config.openshift.io cluster -o jsonpath='{.status.consoleURL}')" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== test-prow-playwright-e2e-techpreview.sh ==\n'
nl -ba test-prow-playwright-e2e-techpreview.sh | sed -n '1,120p'
printf '\n== test-prow-e2e-techpreview.sh ==\n'
nl -ba test-prow-e2e-techpreview.sh | sed -n '1,120p'
printf '\n== search for set -e / export patterns ==\n'
rg -n 'set -e|set \+e|set -u|set -x|export BRIDGE_KUBEADMIN_PASSWORD|consoleURL|kubeadmin-password' test-prow-playwright-e2e-techpreview.sh test-prow-e2e-techpreview.shRepository: openshift/console
Length of output: 242
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Probe shell semantics for export + command substitution failure handling.
bash -lc '
set -u
printf "No set -e:\n"
( export X="$(false)" ); echo "status=$?"
set -e
printf "With set -e in subshell:\n"
( export X="$(false)" ); echo "status=$?" || true
'
# Also probe assignment-only and export separately.
bash -lc '
set -e
f() { return 42; }
( y="$(f)"; echo "assign status=$?" )
( export y="$(f)"; echo "export status=$?" )
'Repository: openshift/console
Length of output: 211
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read only: inspect script headers and relevant lines with context.
printf '\n== test-prow-playwright-e2e-techpreview.sh ==\n'
sed -n '1,120p' test-prow-playwright-e2e-techpreview.sh | nl -ba | sed -n '1,120p'
printf '\n== test-prow-e2e-techpreview.sh ==\n'
sed -n '1,120p' test-prow-e2e-techpreview.sh | nl -ba | sed -n '1,120p'Repository: openshift/console
Length of output: 242
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import pathlib, re
for path in ["test-prow-playwright-e2e-techpreview.sh", "test-prow-e2e-techpreview.sh"]:
text = pathlib.Path(path).read_text()
print(f"\n== {path} ==")
for pat in [r"set -e", r"set \+e", r"export BRIDGE_KUBEADMIN_PASSWORD", r"consoleURL", r"kubeadmin-password"]:
print(f"-- {pat}")
for i, line in enumerate(text.splitlines(), 1):
if re.search(pat, line):
print(f"{i}: {line}")
PYRepository: openshift/console
Length of output: 1238
🌐 Web query:
bash export command substitution exit status set -e export VAR=$(false)
💡 Result:
When you run the command export VAR=$(false), the exit status (
Citations:
- 1: https://stackoverflow.com/questions/56470288/why-does-export-fail-on-bad-substitutions-but-not-command-failures
- 2: https://stackoverflow.com/questions/47609226/how-to-assign-the-output-of-a-command-to-a-variable-while-checking-if-the-comman
- 3: https://stackoverflow.com/questions/37910905/bash-command-substitution-on-external-script-function-receives-incorrect-exit-st
- 4: https://syntblazellc.mintlify.app/bash/script-structure/command-substitution
- 5: https://unix.stackexchange.com/questions/256873/error-exit-script-from-within-command-substitution
Split these setup assignments so failures surface.
export BRIDGE_KUBEADMIN_PASSWORD="$(...)" and export BRIDGE_BASE_ADDRESS="$(...)" can still exit 0 when cat or oc get fails. Assign first, export second, and fail on empty required values in:
test-prow-playwright-e2e-techpreview.sh#L35-L39test-prow-e2e-techpreview.sh#L22-L26
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 37-37: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 39-39: Declare and assign separately to avoid masking return values.
(SC2155)
📍 Affects 2 files
test-prow-playwright-e2e-techpreview.sh#L35-L39(this comment)test-prow-e2e-techpreview.sh#L22-L26
🤖 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 `@test-prow-playwright-e2e-techpreview.sh` around lines 35 - 39, Split the
setup assignments in test-prow-playwright-e2e-techpreview.sh lines 35-39 and
test-prow-e2e-techpreview.sh lines 22-26: assign the kubeadmin password and
console URL to temporary variables, export them only after successful retrieval,
and explicitly fail when either required value is empty. Preserve the existing
set +x protection around the password.
Source: Linters/SAST tools
| elif [ "$SCENARIO" == "feature-gate" ]; then | ||
| # Run only tests tagged @feature-gate; the requireFeatureGate fixture skips any | ||
| # test whose gate is not enabled on this cluster. | ||
| ./integration-tests/test-playwright-e2e.sh -- --grep @feature-gate "$@" | ||
| else | ||
| echo "error: unknown scenario '$SCENARIO' (use: e2e, release, or smoke)" >&2 | ||
| echo "error: unknown scenario '$SCENARIO' (use: e2e, release, smoke, or feature-gate)" >&2 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the new feature-gate scenario.
The implementation adds this scenario, but the script header still lists only e2e, release, and smoke. Add feature-gate to the documented scenario list.
🤖 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 `@test-prow-playwright-e2e.sh` around lines 62 - 67, Update the scenario list
in the header of test-prow-playwright-e2e.sh to include feature-gate alongside
e2e, release, and smoke, keeping the documentation consistent with the SCENARIO
branch and unknown-scenario message.
|
I'm debating whether we should just have a new test suite just for tech preview features instead of running all of the e2e tests twice. For the most part, tech preview shouldn't impact most of the UI except places where we explicitly gate a behavior, so this seems fairly low risk. |
|
/retest |
|
@perdasilva: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
3 participants
Analysis / Root cause:
test-prow-e2e-techpreview.shandtest-prow-playwright-e2e-techpreview.shwere empty placeholders. CI jobs running against TechPreviewNoUpgrade clusters
had no mechanism to select only the tests that require TechPreview or a specific
feature gate to be enabled.
Solution description:
@tech-previewand@feature-gatePlaywright tags as the selectionmechanism for CI scripts.
frontend/e2e/fixtures/index.ts:techPreviewOnly— skips the test if the cluster's FeatureGatespec.featureSetis notTechPreviewNoUpgrade(reads via k8sClient).requireFeatureGate(gateName)— skips unless the named gate appears inthe cluster's enabled list; throws on infrastructure errors so failures
are visible rather than silently skipped.
@tech-previewtest (e2e/tests/olm/olmv1-catalog.spec.ts)as the first consumer of the new fixtures.
test-prow-playwright-e2e-techpreview.shto run--grep "@tech-preview|@feature-gate".feature-gatescenario totest-prow-playwright-e2e.shsofeature-gate tests also run on standard clusters (fixture skips when the
gate is not enabled; tests pass naturally as gates graduate to GA).
frontend/integration-tests/support/tech-preview-guard.tswith aCypress
before()guard (skips suite unlessTechPreviewNoUpgrade) and arequireFeatureGate(gateName)helper.test-prow-e2e-techpreview.shto run theolmCypress package(OLMv1 UI is active on TechPreview clusters).
Screenshots / screen recording:
N/A — CI infrastructure change.
Test setup:
--list.featureSet: TechPreviewNoUpgrade.Test cases:
playwright test --list --grep "@tech-preview"showsolmv1-catalog.spec.ts@tech-previewtests run;techPreviewOnlyskips nothing.techPreviewOnlyskips the test;requireFeatureGateskips tests whose gates are not enabled.
requireFeatureGatethrows visiblyrather than silently skipping.
Browser conformance: N/A
Additional info:
Feature gate graduation is handled transparently: when a gate moves from
TechPreview to GA,
requireFeatureGatefinds it in the cluster's enabled liston GA clusters without any test changes. The
@feature-gatetag runs in boththe regular and techpreview scripts.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation