Skip to content

ci: move the studio CI tests into ci.yaml so that they can be required - #250

Merged
crookedstorm merged 2 commits into
mainfrom
rollup-pr-ci/crookedstorm
Jun 9, 2026
Merged

ci: move the studio CI tests into ci.yaml so that they can be required#250
crookedstorm merged 2 commits into
mainfrom
rollup-pr-ci/crookedstorm

Conversation

@crookedstorm

@crookedstorm crookedstorm commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

This should be a straightforward way to make the studio tests required for studio changes (only).

Summary by CodeRabbit

  • Chores

    • Consolidated CI into a unified pipeline and removed a redundant workflow
    • Added conditional web checks: typecheck, tests, format, lint, dependency verification, and E2E, and included them in the CI aggregate status/gate
  • Documentation

    • Expanded CI documentation to clarify test/check categories and how the merge-gate evaluates required jobs and optional path-filtered jobs

@crookedstorm
crookedstorm requested a review from a team as a code owner June 9, 2026 22:22
Signed-off-by: Brooke Storm <brookes@nvidia.com>
@crookedstorm
crookedstorm force-pushed the rollup-pr-ci/crookedstorm branch from a2f7920 to 54c6ca3 Compare June 9, 2026 22:23
@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Consolidates web CI jobs from studio-ci.yaml into ci.yaml, broadens the changes action to flag any web/** changes, adds six conditional web CI jobs (typecheck/test/format/lint/studio-deps/studio-e2e), updates job conditions, wires them into ci-status as the merge gate, and documents the behavior.

Changes

CI Workflow Consolidation

Layer / File(s) Summary
Expand path filters for web detection
.github/actions/changes/action.yaml
web-studio output description and dorny/paths-filter updated to match web/** instead of specific package subdirs.
Add web CI job definitions
.github/workflows/ci.yaml
Six new conditional jobs: web-typecheck, web-test, web-format, web-lint, web-studio-deps, web-studio-e2e. Triggered on workflow_dispatch or changes.outputs.web-studio == 'true'. PR runs fetch base + pnpm filter for scoped typecheck/test; E2E installs Playwright and uploads report artifact.
Adjust CI job conditions and gating
.github/workflows/ci.yaml
python-unit-test-tools condition changed to !cancelled(); require-nvskills gated to pull_request events.
Wire jobs into ci-status merge gate
.github/workflows/ci.yaml
ci-status needs extended to include the six new web jobs so their results are included in the merge-gate evaluation.
Document ci-status merge-gate behavior
.github/CI_README.md
Expanded ci.yaml README entry describing ci-status as the merge gate, its needs-based evaluation, and how skipped/success allow optional jobs.

Possibly related PRs

Suggested reviewers

  • philipmattingly
  • svvarom
  • steramae-nvidia
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Title accurately describes the main change: consolidating studio CI tests from studio-ci.yaml into ci.yaml to enable them as required checks.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch rollup-pr-ci/crookedstorm

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor
Suite Lines Covered Line Rate Branch Rate
Unit Tests 19062/25122 75.9% 62.5%
Integration Tests 12033/23810 50.5% 25.8%

Signed-off-by: Brooke Storm <brookes@nvidia.com>
@crookedstorm
crookedstorm force-pushed the rollup-pr-ci/crookedstorm branch from d68030e to 8510944 Compare June 9, 2026 22:29
@svvarom
svvarom self-requested a review June 9, 2026 22:33

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 1

🤖 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 @.github/workflows/ci.yaml:
- Around line 399-402: The gating condition for web jobs uses
needs.changes.outputs.web-studio but your change-detection output currently
matches web/** (too broad), so update either the changes job that sets
outputs.web-studio to only be true for studio-specific paths (e.g., matches
web/studio/** or whatever studio path pattern you want) OR change the job
conditionals to use the correct output for general web changes vs studio-only
checks; specifically, fix the expression using needs.changes.outputs.web-studio
(the clause with github.event_name == 'workflow_dispatch' ||
needs.changes.outputs.web-studio == 'true') and all other occurrences
referencing needs.changes.outputs.web-studio (the additional gated web job
conditions) so studio-only jobs read a dedicated web-studio output and general
web jobs use the web output, preventing studio E2E/ci-status from running on
non-studio web changes.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 3fc60b34-bef2-4c77-b7a8-730b564b4c25

📥 Commits

Reviewing files that changed from the base of the PR and between 54c6ca3 and 8510944.

📒 Files selected for processing (1)
  • .github/workflows/ci.yaml

Comment thread .github/workflows/ci.yaml
@crookedstorm
crookedstorm enabled auto-merge June 9, 2026 22:36
@crookedstorm
crookedstorm added this pull request to the merge queue Jun 9, 2026
Merged via the queue into main with commit 5151291 Jun 9, 2026
40 checks passed
@crookedstorm
crookedstorm deleted the rollup-pr-ci/crookedstorm branch June 24, 2026 20:25
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.

3 participants