Skip to content

[build] remove duplicated grid ui tests#17468

Merged
titusfortner merged 1 commit into
trunkfrom
grid-ui
May 15, 2026
Merged

[build] remove duplicated grid ui tests#17468
titusfortner merged 1 commit into
trunkfrom
grid-ui

Conversation

@titusfortner

Copy link
Copy Markdown
Member

#16758 added these to run in RBE, so this action is completely duplicate and can be deleted

@qodo-code-review

Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Remove duplicate Grid UI CI workflow

📦 Other

Grey Divider

Walkthroughs

Description
• Remove duplicate Grid UI CI workflow file
• Tests now run in RBE as per PR #16758
• Eliminates redundant GitHub Actions configuration
Diagram
flowchart LR
  A["ci-grid-ui.yml workflow"] -- "deleted (duplicate)" --> B["RBE-based testing"]
Loading

Grey Divider

File Changes

1. .github/workflows/ci-grid-ui.yml ⚙️ Configuration changes +0/-39

Remove duplicate Grid UI workflow file

• Deleted entire workflow file containing Grid UI component tests
• Workflow included Node.js setup, dependency installation, and npm test execution
• Tests are now handled by RBE configuration from PR #16758

.github/workflows/ci-grid-ui.yml


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented May 15, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0)

Grey Divider


Remediation recommended

1. Fork PRs skip UI tests 🐞 Bug ☼ Reliability
Description
After deleting the dedicated Grid UI workflow, forked pull requests modifying
javascript/grid-ui/** will no longer run the //javascript/grid-ui:test component tests because
both CI-RBE and the main CI workflow skip fork PRs. This can let Grid UI changes from forks appear
green (jobs skipped) without any Grid UI validation running.
Code

.github/workflows/ci-grid-ui.yml[L1-12]

-name: CI - Grid UI
-
-on:
-  pull_request:
-    paths:
-      - 'javascript/grid-ui/**'
-  push:
-    branches:
-      - trunk
-    paths:
-      - 'javascript/grid-ui/**'
-  workflow_dispatch:
Evidence
CI-RBE and the main CI workflow both explicitly skip fork PRs, and CI-RBE is the mechanism that runs
bazel test //... (which would include the Grid UI jest test target). With the dedicated workflow
removed, there is no remaining workflow path that executes Grid UI component tests on fork PRs.

.github/workflows/ci-rbe.yml[24-34]
.github/workflows/ci.yml[26-60]
.github/workflows/ci.yml[129-140]
scripts/github-actions/ci-build.sh[18-25]
javascript/grid-ui/BUILD.bazel[140-156]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`.github/workflows/ci-grid-ui.yml` was the only workflow that executed Grid UI component tests on `pull_request` events scoped to `javascript/grid-ui/**` (including forks). With it deleted, fork PRs do not execute `//javascript/grid-ui:test` because existing Bazel-based CI jobs are gated to non-fork PRs.

### Issue Context
- `CI - RBE` runs `./scripts/github-actions/ci-build.sh`, which runs `bazel test ... //...` and would include `//javascript/grid-ui:test`, but the job is skipped for forks.
- `CI` similarly skips its core `check` job for forks; downstream jobs are skipped and `ci-success` can still pass when everything is skipped.

### Fix Focus Areas
- Add a new workflow to restore fork PR coverage for Grid UI changes (ideally Bazel-based, path-filtered, and read-only permissions). For example: trigger on `pull_request` with `paths: ['javascript/grid-ui/**']`, and run `bazel test //javascript/grid-ui:test` (or `bazel test //javascript/grid-ui/...`).
- Ensure the workflow/job runs for forks (e.g., `if: github.event.pull_request.head.repo.fork == true`) to avoid duplicating same-repo coverage provided by CI-RBE.

- .github/workflows/ci-grid-ui.yml[1-80]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@selenium-ci selenium-ci added the B-build Includes scripting, bazel and CI integrations label May 15, 2026
@titusfortner titusfortner merged commit 915bf53 into trunk May 15, 2026
29 checks passed
@titusfortner titusfortner deleted the grid-ui branch May 15, 2026 20:40
This was referenced Jun 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-build Includes scripting, bazel and CI integrations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants