Skip to content

added ci workflow - #1

Merged
schultzp2020 merged 3 commits into
redhat-developer:mainfrom
schultzp2020:main
Jan 25, 2023
Merged

added ci workflow#1
schultzp2020 merged 3 commits into
redhat-developer:mainfrom
schultzp2020:main

Conversation

@schultzp2020

Copy link
Copy Markdown
Contributor

Signed-off-by: Paul Schultz pschultz@pobox.com

What does this PR do / why we need it

  • Adds ci workflow for pr commits
  • Ran linter against repo

Which issue(s) does this PR fix

N/A

PR acceptance criteria

  • Unit Tests
  • E2E Tests
  • Documentation

How to test changes / Special notes to the reviewer

Signed-off-by: Paul Schultz <pschultz@pobox.com>
Signed-off-by: Paul Schultz <pschultz@pobox.com>
Signed-off-by: Paul Schultz <pschultz@pobox.com>
@schultzp2020
schultzp2020 merged commit 6548d94 into redhat-developer:main Jan 25, 2023
zdrapela referenced this pull request in zdrapela/rhdh Sep 23, 2024
zdrapela referenced this pull request in zdrapela/rhdh Sep 24, 2024
openshift-merge-bot Bot pushed a commit that referenced this pull request Sep 26, 2024
* Add showcase-aks to Playwright & filter tests

* Define values.yaml for AKS

* Update env_variables.sh

* Add AKS deployment initiation

* Add AKS cluster URL

* Override cluster router base for AKS

* AKS check and test + namespace

* Add exceptions for apply_yaml_files for AKS

* Add override to force run as AKS job

* Local dev (REVERT)

* Revert "Local dev (REVERT)"

This reverts commit 21f921a9634993c0dff2f7d0ff3e70dc540bbc1b.

* Bypass cluster TLS

* Update secrets-rhdh-secrets.yaml

* Comment out broken sed

* Disable GitHub org integration

* Update openshift-ci-tests.sh

* Change scope to testMatch

* Change where is analytics enabled

* Use diff value file for AKS

* Don't use pipelines operator on AKS

* Add `az` and cluster start-stop

* Update Dockerfile

* Add approuting check and enable

* Revert "Add override to force run as AKS job"

This reverts commit 35e7319.

* Replace bsCluster with env values

* Uncomment sed

* Avoid shorthand that fails locally

* Cluster information from `az`

* Add RBAC tests to AKS

* Fix GH APP from vault

* Use second GH app for auth AKS

* Fix typo

* Revert "Change where is analytics enabled"

This reverts commit 879e3aa.

* Merge value files with `yq`

* Reapply "Add override to force run as AKS job"

This reverts commit 9bd004991ff7bfc6f16874acbebd1a8335955523.

* Fixes

* Local

* Fix rbac values

* Revert "Local"

This reverts commit 489914aeb60543a2fd78e8250838429e4462f53b.

* Skip for AKS, because of strange `sed: -e expression #1, char 136: unterminated `s' command`

* Skip broken RBAC test on AKS

* Bullet proofing

* Skip `droute` and broken test on AKS

* Fix test name

* Typo

* Disable broken analytics test

* Update openshift-ci-tests.sh

* Disable `az` start and stop

* Add Tekton pipelines

* Fix

* Update openshift-ci-tests.sh

* Disable RBAC

* Enable az start stop

* Remove `JOB_NAME` override

* Enable RBAC tests

* Revert "Remove `JOB_NAME` override"

This reverts commit 5f75435.

* Reapply "Remove `JOB_NAME` override"

This reverts commit 3f71d15.

* Add showcase-aks to Playwright & filter tests

* Define values.yaml for AKS

* Update env_variables.sh

* Add AKS deployment initiation

* Add AKS cluster URL

* Override cluster router base for AKS

* AKS check and test + namespace

* Add exceptions for apply_yaml_files for AKS

* Add override to force run as AKS job

* Local dev (REVERT)

* Revert "Local dev (REVERT)"

This reverts commit 21f921a9634993c0dff2f7d0ff3e70dc540bbc1b.

* Bypass cluster TLS

* Update secrets-rhdh-secrets.yaml

* Comment out broken sed

* Disable GitHub org integration

* Update openshift-ci-tests.sh

* Change scope to testMatch

* Change where is analytics enabled

* Use diff value file for AKS

* Don't use pipelines operator on AKS

* Add `az` and cluster start-stop

* Update Dockerfile

* Add approuting check and enable

* Revert "Add override to force run as AKS job"

This reverts commit 35e7319.

* Replace bsCluster with env values

* Uncomment sed

* Avoid shorthand that fails locally

* Cluster information from `az`

* Add RBAC tests to AKS

* Fix GH APP from vault

* Use second GH app for auth AKS

* Fix typo

* Revert "Change where is analytics enabled"

This reverts commit 879e3aa.

* Merge value files with `yq`

* Reapply "Add override to force run as AKS job"

This reverts commit 9bd004991ff7bfc6f16874acbebd1a8335955523.

* Fixes

* Local

* Fix rbac values

* Revert "Local"

This reverts commit 489914aeb60543a2fd78e8250838429e4462f53b.

* Skip for AKS, because of strange `sed: -e expression #1, char 136: unterminated `s' command`

* Skip broken RBAC test on AKS

* Bullet proofing

* Skip `droute` and broken test on AKS

* Fix test name

* Typo

* Disable broken analytics test

* Update openshift-ci-tests.sh

* Disable `az` start and stop

* Add Tekton pipelines

* Fix

* Update openshift-ci-tests.sh

* Disable RBAC

* Enable az start stop

* Remove `JOB_NAME` override

* Enable RBAC tests

* Revert "Remove `JOB_NAME` override"

This reverts commit 5f75435.

* Reapply "Remove `JOB_NAME` override"

This reverts commit 3f71d15.

* Better merging of base and diff value files

* Fix indentation of value files

* Switch from testMatch to testIgnore for AKS

* Add test ignore

* Update playwright.config.ts

* Revert "Reapply "Remove `JOB_NAME` override""

This reverts commit ef883d2.

* Increase timeout

* Reapply "Reapply "Remove `JOB_NAME` override""

This reverts commit 3c25521.

* Move `az_*` to `utils.sh`
chadcrum referenced this pull request in chadcrum/rhdh Aug 3, 2025
…loper#1472)

* Add showcase-aks to Playwright & filter tests

* Define values.yaml for AKS

* Update env_variables.sh

* Add AKS deployment initiation

* Add AKS cluster URL

* Override cluster router base for AKS

* AKS check and test + namespace

* Add exceptions for apply_yaml_files for AKS

* Add override to force run as AKS job

* Local dev (REVERT)

* Revert "Local dev (REVERT)"

This reverts commit 21f921a9634993c0dff2f7d0ff3e70dc540bbc1b.

* Bypass cluster TLS

* Update secrets-rhdh-secrets.yaml

* Comment out broken sed

* Disable GitHub org integration

* Update openshift-ci-tests.sh

* Change scope to testMatch

* Change where is analytics enabled

* Use diff value file for AKS

* Don't use pipelines operator on AKS

* Add `az` and cluster start-stop

* Update Dockerfile

* Add approuting check and enable

* Revert "Add override to force run as AKS job"

This reverts commit 35e7319.

* Replace bsCluster with env values

* Uncomment sed

* Avoid shorthand that fails locally

* Cluster information from `az`

* Add RBAC tests to AKS

* Fix GH APP from vault

* Use second GH app for auth AKS

* Fix typo

* Revert "Change where is analytics enabled"

This reverts commit 879e3aa.

* Merge value files with `yq`

* Reapply "Add override to force run as AKS job"

This reverts commit 9bd004991ff7bfc6f16874acbebd1a8335955523.

* Fixes

* Local

* Fix rbac values

* Revert "Local"

This reverts commit 489914aeb60543a2fd78e8250838429e4462f53b.

* Skip for AKS, because of strange `sed: -e expression #1, char 136: unterminated `s' command`

* Skip broken RBAC test on AKS

* Bullet proofing

* Skip `droute` and broken test on AKS

* Fix test name

* Typo

* Disable broken analytics test

* Update openshift-ci-tests.sh

* Disable `az` start and stop

* Add Tekton pipelines

* Fix

* Update openshift-ci-tests.sh

* Disable RBAC

* Enable az start stop

* Remove `JOB_NAME` override

* Enable RBAC tests

* Revert "Remove `JOB_NAME` override"

This reverts commit 5f75435.

* Reapply "Remove `JOB_NAME` override"

This reverts commit 3f71d15.

* Add showcase-aks to Playwright & filter tests

* Define values.yaml for AKS

* Update env_variables.sh

* Add AKS deployment initiation

* Add AKS cluster URL

* Override cluster router base for AKS

* AKS check and test + namespace

* Add exceptions for apply_yaml_files for AKS

* Add override to force run as AKS job

* Local dev (REVERT)

* Revert "Local dev (REVERT)"

This reverts commit 21f921a9634993c0dff2f7d0ff3e70dc540bbc1b.

* Bypass cluster TLS

* Update secrets-rhdh-secrets.yaml

* Comment out broken sed

* Disable GitHub org integration

* Update openshift-ci-tests.sh

* Change scope to testMatch

* Change where is analytics enabled

* Use diff value file for AKS

* Don't use pipelines operator on AKS

* Add `az` and cluster start-stop

* Update Dockerfile

* Add approuting check and enable

* Revert "Add override to force run as AKS job"

This reverts commit 35e7319.

* Replace bsCluster with env values

* Uncomment sed

* Avoid shorthand that fails locally

* Cluster information from `az`

* Add RBAC tests to AKS

* Fix GH APP from vault

* Use second GH app for auth AKS

* Fix typo

* Revert "Change where is analytics enabled"

This reverts commit 879e3aa.

* Merge value files with `yq`

* Reapply "Add override to force run as AKS job"

This reverts commit 9bd004991ff7bfc6f16874acbebd1a8335955523.

* Fixes

* Local

* Fix rbac values

* Revert "Local"

This reverts commit 489914aeb60543a2fd78e8250838429e4462f53b.

* Skip for AKS, because of strange `sed: -e expression #1, char 136: unterminated `s' command`

* Skip broken RBAC test on AKS

* Bullet proofing

* Skip `droute` and broken test on AKS

* Fix test name

* Typo

* Disable broken analytics test

* Update openshift-ci-tests.sh

* Disable `az` start and stop

* Add Tekton pipelines

* Fix

* Update openshift-ci-tests.sh

* Disable RBAC

* Enable az start stop

* Remove `JOB_NAME` override

* Enable RBAC tests

* Revert "Remove `JOB_NAME` override"

This reverts commit 5f75435.

* Reapply "Remove `JOB_NAME` override"

This reverts commit 3f71d15.

* Better merging of base and diff value files

* Fix indentation of value files

* Switch from testMatch to testIgnore for AKS

* Add test ignore

* Update playwright.config.ts

* Revert "Reapply "Remove `JOB_NAME` override""

This reverts commit ef883d2.

* Increase timeout

* Reapply "Reapply "Remove `JOB_NAME` override""

This reverts commit 3c25521.

* Move `az_*` to `utils.sh`
openshift-merge-bot Bot pushed a commit that referenced this pull request Nov 14, 2025
* fix(auto-approve): add checkout step to fix 'not a git repository' error

The gh CLI commands were failing because the repository wasn't checked out.
Added actions/checkout@v4 step to ensure git context is available.

* test: temporarily enable workflow for polasudo to test functionality

* test: add fix/auto-approve-bot to eligible patterns for testing

* fix(auto-approve): add issues:write permission for label management

* fix(auto-approve): use pull_request_target for proper permissions

Using pull_request_target instead of pull_request allows the workflow
to run with write permissions even for PRs from forks or bot accounts.
This is necessary for adding labels and approving PRs.

* Fix auto-approve workflow: use REST API for adding labels instead of gh CLI

* test: trigger workflow re-run

* test: remove trigger file

* test: temporarily disable user condition to test workflow

* fix(auto-approve): use pull_request_target and add issues:write permission

- Change trigger from pull_request to pull_request_target for proper permissions
- Add issues:write permission required for label management
- Add --repo flag to gh commands for explicit repository context
- Remove checkout step (not needed with pull_request_target)
- Fixes GraphQL: Resource not accessible by integration error

* fix(auto-approve): add fix/auto-approve-bot pattern for testing

* feat(auto-approve): add push trigger for faster testing feedback

- Add push trigger on fix/auto-approve-bot branch for immediate testing
- Handle both push and pull_request_target events in workflow logic
- Add logging step for push events to show eligibility check results
- Only run label/approve actions on actual PR events

* test: trigger workflow rerun

* test: simplify push logging to debug workflow triggering

* test: add temporary PR labeling simulation for fork testing

* test: enable real PR labeling on push events for testing

* test: target PR #1 in fork for labeling test

* test: target PR #2 for labeling demonstration

* feat: clean up auto-approve workflow for production use

- Remove all testing code (push triggers, test PR numbers, debug logging)
- Simplify workflow to only run on pull_request_target events
- Only run for rhdh-bot PRs
- Keep fix/auto-approve-bot pattern for final testing before merge
- Ready for production deployment

* test: temporarily allow polasudo for final workflow testing

* feat: finalize auto-approve workflow for production

- Remove temporary polasudo testing allowance
- Remove fix/auto-approve-bot test pattern
- Clean, production-ready workflow for rhdh-bot PRs
- Ready to merge and deploy

* test: trigger workflow to demonstrate PR #2 labeling

* Delete test-workflow-final.md

* chore: reorganize auto-approve workflow flags for better readability
nickboldt pushed a commit that referenced this pull request Nov 24, 2025
* fix(auto-approve): add checkout step to fix 'not a git repository' error

The gh CLI commands were failing because the repository wasn't checked out.
Added actions/checkout@v4 step to ensure git context is available.

* test: temporarily enable workflow for polasudo to test functionality

* test: add fix/auto-approve-bot to eligible patterns for testing

* fix(auto-approve): add issues:write permission for label management

* fix(auto-approve): use pull_request_target for proper permissions

Using pull_request_target instead of pull_request allows the workflow
to run with write permissions even for PRs from forks or bot accounts.
This is necessary for adding labels and approving PRs.

* Fix auto-approve workflow: use REST API for adding labels instead of gh CLI

* test: trigger workflow re-run

* test: remove trigger file

* test: temporarily disable user condition to test workflow

* fix(auto-approve): use pull_request_target and add issues:write permission

- Change trigger from pull_request to pull_request_target for proper permissions
- Add issues:write permission required for label management
- Add --repo flag to gh commands for explicit repository context
- Remove checkout step (not needed with pull_request_target)
- Fixes GraphQL: Resource not accessible by integration error

* fix(auto-approve): add fix/auto-approve-bot pattern for testing

* feat(auto-approve): add push trigger for faster testing feedback

- Add push trigger on fix/auto-approve-bot branch for immediate testing
- Handle both push and pull_request_target events in workflow logic
- Add logging step for push events to show eligibility check results
- Only run label/approve actions on actual PR events

* test: trigger workflow rerun

* test: simplify push logging to debug workflow triggering

* test: add temporary PR labeling simulation for fork testing

* test: enable real PR labeling on push events for testing

* test: target PR #1 in fork for labeling test

* test: target PR #2 for labeling demonstration

* feat: clean up auto-approve workflow for production use

- Remove all testing code (push triggers, test PR numbers, debug logging)
- Simplify workflow to only run on pull_request_target events
- Only run for rhdh-bot PRs
- Keep fix/auto-approve-bot pattern for final testing before merge
- Ready for production deployment

* test: temporarily allow polasudo for final workflow testing

* feat: finalize auto-approve workflow for production

- Remove temporary polasudo testing allowance
- Remove fix/auto-approve-bot test pattern
- Clean, production-ready workflow for rhdh-bot PRs
- Ready to merge and deploy

* test: trigger workflow to demonstrate PR #2 labeling

* Delete test-workflow-final.md

* chore: reorganize auto-approve workflow flags for better readability

* fix(ci): add checkout step to auto-approve-bot-prs workflow to fix git repository error

- Adds actions/checkout@v4 to provide git context for gh CLI commands
- Fixes 'fatal: not a git repository' error in auto-approve workflow
Fortune-Ndlovu pushed a commit to Fortune-Ndlovu/rhdh that referenced this pull request Dec 1, 2025
…eveloper#3717)

* fix(auto-approve): add checkout step to fix 'not a git repository' error

The gh CLI commands were failing because the repository wasn't checked out.
Added actions/checkout@v4 step to ensure git context is available.

* test: temporarily enable workflow for polasudo to test functionality

* test: add fix/auto-approve-bot to eligible patterns for testing

* fix(auto-approve): add issues:write permission for label management

* fix(auto-approve): use pull_request_target for proper permissions

Using pull_request_target instead of pull_request allows the workflow
to run with write permissions even for PRs from forks or bot accounts.
This is necessary for adding labels and approving PRs.

* Fix auto-approve workflow: use REST API for adding labels instead of gh CLI

* test: trigger workflow re-run

* test: remove trigger file

* test: temporarily disable user condition to test workflow

* fix(auto-approve): use pull_request_target and add issues:write permission

- Change trigger from pull_request to pull_request_target for proper permissions
- Add issues:write permission required for label management
- Add --repo flag to gh commands for explicit repository context
- Remove checkout step (not needed with pull_request_target)
- Fixes GraphQL: Resource not accessible by integration error

* fix(auto-approve): add fix/auto-approve-bot pattern for testing

* feat(auto-approve): add push trigger for faster testing feedback

- Add push trigger on fix/auto-approve-bot branch for immediate testing
- Handle both push and pull_request_target events in workflow logic
- Add logging step for push events to show eligibility check results
- Only run label/approve actions on actual PR events

* test: trigger workflow rerun

* test: simplify push logging to debug workflow triggering

* test: add temporary PR labeling simulation for fork testing

* test: enable real PR labeling on push events for testing

* test: target PR redhat-developer#1 in fork for labeling test

* test: target PR redhat-developer#2 for labeling demonstration

* feat: clean up auto-approve workflow for production use

- Remove all testing code (push triggers, test PR numbers, debug logging)
- Simplify workflow to only run on pull_request_target events
- Only run for rhdh-bot PRs
- Keep fix/auto-approve-bot pattern for final testing before merge
- Ready for production deployment

* test: temporarily allow polasudo for final workflow testing

* feat: finalize auto-approve workflow for production

- Remove temporary polasudo testing allowance
- Remove fix/auto-approve-bot test pattern
- Clean, production-ready workflow for rhdh-bot PRs
- Ready to merge and deploy

* test: trigger workflow to demonstrate PR redhat-developer#2 labeling

* Delete test-workflow-final.md

* chore: reorganize auto-approve workflow flags for better readability

* fix(ci): add checkout step to auto-approve-bot-prs workflow to fix git repository error

- Adds actions/checkout@v4 to provide git context for gh CLI commands
- Fixes 'fatal: not a git repository' error in auto-approve workflow

Assisted-by: Cursor
Signed-off-by: Fortune Ndlovu <fndlovu@redhat.com>
openshift-merge-bot Bot pushed a commit that referenced this pull request Apr 24, 2026
* feat(e2e-coverage): scaffold Playwright page.coverage collection (RHIDP-13243)

Scaffolding for E2E frontend coverage on the rhdh repo (part of
RHDHPLAN-851, Epic RHIDP-13242). Opt-in via COLLECT_COVERAGE=true;
default behavior unchanged so existing E2E runs are unaffected.

Infrastructure only
- Extended test fixture at e2e-tests/playwright/support/coverage/test.ts
  that wraps page.coverage.startJSCoverage / stopJSCoverage around each
  test when COLLECT_COVERAGE=true.
- Playwright reporter at e2e-tests/playwright/support/coverage/reporter.ts
  that aggregates raw V8 output into a merged Istanbul LCOV + HTML report
  via monocart-coverage-reports.
- Reporter registered conditionally in playwright.config.ts so it is a
  no-op unless COLLECT_COVERAGE=true.
- monocart-coverage-reports@2.12.11 added to e2e-tests devDependencies.
- Docs at docs/coverage/e2e-rhdh.md covering usage, migration pattern,
  env vars, known limitations.
- .gitignore tweaks so source code under paths containing "coverage"
  (docs/coverage, e2e-tests/playwright/support/coverage) is tracked while
  the generated "coverage/" output dirs remain ignored.

No specs are migrated in this PR. Specs opt in via:
  import { test, expect } from "../support/coverage/test";

Follow-ups (separate PRs)
- Migrate one spec (e.g. smoke-test) to validate the pipeline end-to-end.
- Wire CI step to upload lcov.info to Codecov with flag rhdh-e2e-frontend.
- Evaluate instrumented showcase image variant (RHIDP-13244 spike).

Refs
- https://issues.redhat.com/browse/RHIDP-13243
- https://issues.redhat.com/browse/RHIDP-13242
- https://issues.redhat.com/browse/RHDHPLAN-851

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore(e2e-coverage): update yarn.lock for monocart-coverage-reports

Previous commit added monocart-coverage-reports@2.12.11 to
e2e-tests/package.json but did not update e2e-tests/yarn.lock. Yarn's
hardened mode on public PRs forbids lockfile modifications during
install, which blocks the CI lint/prettier/shellcheck step.

Regenerated the lockfile locally with `yarn install`. Only new entries
for monocart-coverage-reports and its transitive deps.

Refs
- #4680
- https://issues.redhat.com/browse/RHIDP-13243

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(e2e-coverage): satisfy ESLint naming and no-unused-vars rules

CI lint step rejected the new files under e2e-tests/playwright/support/coverage/
because of the project eslint-config naming rules. Fixes:

- Rename top-level `const` variables to camelCase
  (COVERAGE_RAW_DIR/COVERAGE_REPORT_DIR/COVERAGE_OUTPUT_DIR → camelCase)
- Drop unused `_config` and `_result` parameters on Reporter.onBegin /
  onEnd (the Reporter interface allows narrower implementations).
- Replace destructuring `const { CoverageReport } = await import(...)`
  with `const monocart = await import(...)` + `new monocart.CoverageReport(...)`
  so the PascalCase class name does not trip the variable naming rule.
- Keep the Playwright-idiomatic `export const test` and `export const expect`
  names, guarded by `eslint-disable-next-line @typescript-eslint/naming-convention`
  with a comment explaining the rationale (renaming would force every
  consumer to alias on import, degrading the DX).

Verified locally:
- `npx eslint playwright/support/coverage/test.ts playwright/support/coverage/reporter.ts` clean
- `yarn prettier:check playwright/support/coverage/` clean
- `yarn tsc:check` clean

Refs
- #4680
- https://issues.redhat.com/browse/RHIDP-13243

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(e2e-coverage): address review feedback on scaffolding

Harden the Playwright coverage scaffolding based on review notes so the
first COLLECT_COVERAGE=true runs do not surface papercuts in CI.

Correctness
- Filename collision under parallel workers: raw V8 coverage files now
  include workerIndex and retry in the filename, and the title is taken
  from testInfo.titlePath (describe chain included) rather than just
  title. Two specs with identical titles in different describe blocks
  no longer collide.
- Silent error swallow in reporter.readdir: the catch now only swallows
  ENOENT (directory missing on first run). Any other I/O error rethrows
  so CI logs show the real cause instead of the misleading
  "no coverage collected" warning.
- JSON shape guard: entries loaded from raw files are verified to be
  arrays before handing to monocart. Corrupted or unexpected files are
  skipped with a clear warning instead of failing deep inside the
  library.

Structure
- Extracted the env-var-to-path resolution into a shared paths.ts, so
  the fixture and reporter can never drift on COVERAGE_OUTPUT_DIR /
  COVERAGE_REPORT_DIR. Exports follow the project UPPER_CASE convention
  for exported const.

Hardening
- Timeout wrapper (default 2 min, COVERAGE_GENERATE_TIMEOUT_MS override)
  around monocart.CoverageReport.generate(). A hung aggregation can no
  longer hold the Playwright run open in CI.
- Parallel file reads via Promise.all. Scales better as the full 64-spec
  suite starts emitting coverage.

Readability
- playwright.config.ts uses `satisfies ReporterDescription[]` instead of
  the earlier `as [string]` cast — removes the type gymnastics while
  keeping the conditional registration.
- Inline comments on startJSCoverage options explain the rationale for
  resetOnNavigation: false and reportAnonymousScripts: false.
- Reworded the dynamic-import comment to match what it actually does
  (defers monocart load until generation time; the dep is installed
  regardless because it's in devDependencies).

Docs
- docs/coverage/e2e-rhdh.md updated with:
  - new env var COVERAGE_GENERATE_TIMEOUT_MS
  - note on the new filename scheme (workerIndex + retry) so the
    parallel collision question is explicit
  - import path examples for specs at depths 2, 3, and 4 (there is no
    tsconfig path alias configured)

Local validation
- npx eslint playwright/support/coverage/*.ts playwright.config.ts: clean
- yarn tsc: clean
- yarn prettier:check (coverage files + playwright.config.ts + docs): clean

Refs
- #4680
- https://issues.redhat.com/browse/RHIDP-13243

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(e2e-coverage): address qodo review findings (bugs 2, 5, 7)

Addresses actionable findings from the Qodo automated review. Three of
the seven flagged items are real bugs that are fixed here; the other
four are either false positives or scope decisions covered in the PR
reply.

Fixed
- Stale raw files across runs (qodo #2). The reporter's onBegin now
  does `fs.rm COVERAGE_RAW_DIR` followed by `mkdir`, so the merged
  report only reflects the current Playwright run. Without this the
  raw dir accumulated *.json across runs and produced an incorrect
  LCOV.
- Coverage teardown could fail tests (qodo #5). Both startJSCoverage
  and the file-write teardown are now wrapped in try/catch with a
  console.warn. Coverage collection is best-effort and can never fail
  a test run, including scenarios like unsupported browsers, pages
  closed before teardown, or transient I/O errors.
- Custom-context specs silently skipped (qodo #7). Exported two
  helpers — startCoverageForPage(page) and stopCoverageForPage(page,
  testInfo) — so specs that manage their own BrowserContext/Page via
  browser.newContext() (e.g. plugins/adoption-insights, plugins/
  scorecard) can opt in explicitly. Helpers are no-ops when
  COLLECT_COVERAGE is unset and share the same error-safe wrappers as
  the auto fixture.

Not changed (with reason)
- Fixture typing (qodo #1) — false positive. Reproduced locally: a
  migrated spec destructuring `{ page }` from the extended test
  compiles clean under `yarn tsc`. Kept `<NonNullable<unknown>>`.
- Global fixture for every spec (qodo #3) — intentional scope. A
  per-spec import migration is phased so each batch lands in its own
  reviewable PR. Playwright does not offer a mechanism to override the
  test import globally without editing every spec file.
- Codecov upload step (qodo #4) — intentional scope, depends on
  RHIDP-13230 (Codecov GitHub App + Vault token) landing first. The
  upload step is tracked for the follow-up CI wiring PR.
- Source-map validation (qodo #6) — validation requires a running
  RHDH instance against which to run the instrumented suite. Planned
  as part of the first real COLLECT_COVERAGE=true run rather than a
  scaffolding PR.

Docs updated
- docs/coverage/e2e-rhdh.md now documents the custom-context pattern
  with a full example using startCoverageForPage / stopCoverageForPage,
  plus a "Specs that have not migrated" note making the phased
  migration explicit.

Local validation
- npx eslint playwright/support/coverage/*.ts: clean
- yarn tsc: clean
- yarn prettier:check (coverage + docs): clean

Refs
- #4680
- https://issues.redhat.com/browse/RHIDP-13243

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(e2e-coverage): address PR review feedback on gitignore patterns

Replace broad **/coverage ignore with scoped rules per zdrapela's review:
- Root .gitignore: target dynamic-plugins/packages/plugins coverage dirs
- e2e-tests/.gitignore: use /coverage/ instead of exclusion patterns
- Add coverage to e2e-tests/.prettierignore and eslint ignores

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* docs(e2e-coverage): add AI assistant rule for instrumented imports

Adds a rulesync rule that instructs AI assistants (Cursor, Claude Code,
OpenCode) to use the coverage-instrumented test/expect imports instead
of @playwright/test when creating or modifying spec files.

Suggested-by: zdrapela

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* refactor(e2e-coverage): add @support path alias for coverage imports

Add a TypeScript path alias @support/* → playwright/support/* in
e2e-tests/tsconfig.json so specs can use a clean, depth-independent
import instead of fragile relative paths.

Update docs, code comments, and AI assistant rules accordingly.

Suggested-by: zdrapela

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
nickboldt pushed a commit that referenced this pull request Apr 27, 2026
* feat(ci): parallelize base and RBAC helm deployments

Run base_deployment and rbac_deployment concurrently in initiate_deployments()
and initiate_deployments_osd_gcp(). Both target disjoint namespaces (NAME_SPACE
vs NAME_SPACE_RBAC + NAME_SPACE_POSTGRES_DB), so there is no resource conflict.
Overlapping the two helm upgrades and their internal readiness waits saves
~3-5 min of wall-clock time on CI.

The _run_parallel_deployments helper captures each background job's exit code
via `wait "$pid" || rc=$?` and returns 1 if either fails, so failures always
propagate and are individually reported in the logs. Tests continue to run
sequentially downstream (parallel Playwright would OOM given CI memory limits).

Resolves: RHIDP-12296

* refactor(ci): add defensive checks and improved logging to parallel deployments

Address code review findings from PR #4679:

1. **Namespace validation**: Add defensive check to ensure NAME_SPACE and
   NAME_SPACE_RBAC are different and non-empty before starting parallel
   deployments. Prevents subtle race conditions if misconfigured.

2. **Improved logging**:
   - Log both namespace names at the start for debugging
   - Add synchronization point logs ("Waiting for parallel deployments..."
     and "Parallel deployments finished") to make log flow clearer when
     output from both background jobs is interleaved

3. **Documentation**: Add "Requires" section to function header documenting
   the namespace disjointness requirement

These changes improve debuggability and fail-fast behavior without changing
the core parallelization logic.

* fix(ci): eliminate race conditions for parallel deployments

Address 3 critical race conditions identified by Qodo code review:

**1. Shared temp files in helm::merge_values (Bug #1)**
- BEFORE: Used fixed paths /tmp/step-without-plugins.yaml and
  /tmp/step-only-plugins.yaml
- AFTER: Generate unique temp files per invocation via mktemp
- WHY: Concurrent helm::merge_values calls would overwrite each other's
  intermediate files, corrupting merged values

**2. Kubeconfig global state mutation (Bug #2)**
- BEFORE: namespace::configure and apply_yaml_files both ran
  `oc config set-context --current --namespace=...`
- AFTER: Removed all oc config set-context calls; added explicit
  --namespace flag to all oc/kubectl apply commands that lacked it
- WHY: Parallel deployments racing on current context would apply
  resources to nondeterministic namespaces

**3. In-place YAML file edits (Bug #3)**
- BEFORE: apply_yaml_files used sed -i to modify shared YAML templates
  directly in the repo
- AFTER: Copy templates to per-deployment tmpdir, patch copies, apply
  from tmpdir
- WHY: Concurrent sed -i edits would clobber each other and apply
  manifests with wrong namespace fields

All three bugs would have caused nondeterministic deployment failures
when base_deployment and rbac_deployment run in parallel.

Changes:
- .ci/pipelines/lib/helm.sh: mktemp + trap cleanup for intermediate files
- .ci/pipelines/lib/namespace.sh: Remove oc config set-context
- .ci/pipelines/utils.sh: Tmpdir for YAML patches, explicit --namespace
  on all oc apply calls

* fix(ci): address code review findings for parallel deployments

1. postgres-cred.yaml in-place edit race: configure_external_postgres_db
   now patches a temp copy instead of editing the shared template via
   sed_inplace. Same class of bug fixed in apply_yaml_files.

2. Unquoted ${NAME_SPACE} in namespace::configure calls: quote both
   occurrences (lines 553, 721) to prevent word-splitting.

3. mktemp in helm::merge_values overwrite path: move mktemp + trap
   inside the "merge" branch so the "overwrite" path doesn't create
   and immediately discard two unused temp files.

4. Remove redundant "Waiting for parallel deployments" log::section
   that fires right before the blocking wait calls — the bookend
   sections are sufficient.

* fix(ci): replace trap RETURN with explicit cleanup

trap RETURN leaks to the calling function in bash < 4.4, causing
unbound variable errors when step_1_file/tmpdir are expanded in
the caller's scope under set -o nounset.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
@openshift-ci

openshift-ci Bot commented Jun 30, 2026

Copy link
Copy Markdown

@rhdh-rc-test: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-ocp-helm b6d8161 link unknown /test e2e-ocp-helm

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant