ci: turn on and fix ui-style and test-vscode stages - #6054
Conversation
… CI was off `ui-style` has been disabled, so `prettier --check .` and the per-package eslint runs have not been enforced on merges for a while and drifted. Reformat the three files prettier flags, and fix the one eslint error in `vscode/extension`: `lineage_settings.spec.ts` imported `FrameLocator` and `Page` straight from `@playwright/test`, which `no-restricted-imports` forbids for `tests/**/*.spec.ts`. Every other spec takes `Page` from `./fixtures`, so re-export `FrameLocator` there too and import both from the fixtures module. No behavioural changes — the reformatting is whitespace only. Signed-off-by: Adegbite Ayoade <tripleaceme@gmail.com>
Both jobs were pinned to `if: false`. Restore the real conditions, adding a `vscode` paths filter so a change under `vscode/**` triggers them, and give `test-vscode` the `needs: changes` it was missing. Prettier also reflows two matrix lists in this file that grew past the 80 column limit while the job was off (the engine list and the dbt-version list); those were already failing `prettier --check` on main. `test-vscode-e2e` is deliberately left disabled — per the note on the issue, its flakiness is tracked separately. Signed-off-by: Adegbite Ayoade <tripleaceme@gmail.com>
|
Looks good to me, thanks for getting to this so fast! -- Trino / Spark failures are separate issue addressed here: #6055 |
|
Thanks for the quick review and merge! Is there anything you'd like me pick up next? I've been digging through the VSCode Extension issues and a few look
Happy to take whichever is most useful, or something else entirely if those are already spoken for or blocked on the extension CI. |
|
Hey @tripleaceme -- we don't have many working on the Extension side of things rn, just make sure to double check the Issues for any assignees or linked PRs. I think #5920 and is a pretty big win if you want to tackle that one! |
|
I'd like to take this one and I think #5642 is the same bug, so I'll cover both in a single PR. |
|
@tripleaceme please comment on the appropriate issues for visibility so they can be assigned |
Description
Closes #6004.
ui-styleandtest-vscodewere both pinned toif: false. This turns them back on and fixes everything that surfaced once they ran.1. Formatting / lint drift. With
ui-styleoff,prettier --check .and the per-package eslint runs stopped being enforced, so a few things drifted onmain:prettier --check .fails on.devcontainer/devcontainer.json,web/client/src/library/components/graph/help.ts,web/client/src/library/components/graph/ModelNode.tsx, and.github/workflows/pr.yamlitself. All reformatted; the changes are whitespace only.eslint src testsfails invscode/extension:tests/lineage_settings.spec.tsimportsFrameLocatorandPagedirectly from@playwright/test, whichno-restricted-importsforbids fortests/**/*.spec.ts. Every other spec takesPagefrom./fixtures, so I followed that convention — re-exportedFrameLocatorfromtests/fixtures.tsand imported both from there.2. Turning the jobs on, following #5994:
vscodeoutput to thechangesjob, filtering onvscode/**.ui-stylenow runs whenclient,vscodeorcichanged, or onmain.test-vscodegets the same treatment plus theneeds: changesit was missing.test-vscode-e2eis deliberately left disabled, per the note on the issue that its flakiness should be tracked separately.Note on the
pr.yamldiffPrettier reflows two matrix lists in
pr.yamlthat grew past the 80-column limit while the job was off — theenginelist (StarRocks, #5832) and thedbt-versionlist (#6028, #5999). Those were already failingprettier --checkonmain, so they have to be fixed here forfmt:checkto go green. I kept them in the CI commit so the formatting-only commit stays reviewable on its own.Test Plan
Ran both job commands locally against a clean
pnpm install:pnpm run cicoversvscode/bus(tsc --noEmit),vscode/extension(eslint+check-types+esbuild+vitest run— 7 tests pass), andweb/common(syncpack+eslint+build). Verified it leaves no modified files behind, which matters becauseweb/common's lint script runs with--fix.I also confirmed against an unmodified
upstream/maincheckout that the twopr.yamlmatrix reflows and the three other unformatted files are pre-existing, not something my edits introduced.Local run was macOS / Node 24; CI uses Ubuntu / Node 22. Conveniently this PR touches both
.github/**andvscode/**, so it should exerciseui-styleandtest-vscodeon itself.Checklist
make styleand fixed any issuespnpm run ci(see Test Plan). No Python changed, somake fast-testis untouched by this PR and I did not run itgit commit -s) per the DCO