Skip to content

ci: build the e2e virtualenv once per worker and re-enable test-vscode-e2e - #6098

Open
tripleaceme wants to merge 3 commits into
SQLMesh:mainfrom
tripleaceme:fix/vscode-e2e-shared-venv
Open

tripleaceme wants to merge 3 commits into
SQLMesh:mainfrom
tripleaceme:fix/vscode-e2e-shared-venv

Conversation

@tripleaceme

Copy link
Copy Markdown
Contributor

Description

Closes #6071. Builds the test virtualenv once per worker, as agreed on the issue, and turns test-vscode-e2e back on.

Build the virtualenv once per worker

fixtures.ts gains a worker-scoped sharedPythonEnvironment alongside the existing sharedCodeServer. It creates its own directory, installs sqlmesh[lsp,bigquery] plus custom_materializations once, and warms the import. It is not auto, so only tests that ask for it pay for it.

Measured cost that is now paid once per worker rather than once per test:

step cost
uv venv 0.3s
uv pip install -e repo[lsp,bigquery] + custom materializations 18.4s
first import sqlmesh.lsp.main 25.3s

That second figure was not in the diagnosis I posted on the issue, and it is the larger half — Python compiling the dependency tree into __pycache__, inside the LSP handshake the tests wait on. Sharing the virtualenv without warming the import barely moved the needle, so the warm-up is doing as much work here as the install.

Converted: all 3 in configuration.spec.ts, and the 2 environment-injection tests in python_env.spec.ts.

Deliberately not converted, because their own virtualenv is the thing under test: venv_naming.spec.ts (the venv's name is the assertion), tcloud.spec.ts and python_env.spec.ts's tcloud cases (they install a mock tcloud, which in a shared environment would leak into every later test in the worker), and bad_setup.spec.ts (installs without the lsp extra on purpose).

Re-enable the job

Same path filter as the unit job, so Python-only pull requests are not gated on it.

code-server is pinned instead of installed from latest. That was not the cause of the failures — the render selector broke identically on 4.107.1, the newest release when the job was disabled — but leaving it unpinned means the next editor UI change arrives as a mystery failure rather than a visible version bump.

The timeout, and where I would push back on my own change

The per-test timeout goes from 60s to 180s, and I want to be straight that this is the weakest part of the PR.

Removing ~44s of fixed cost was not enough on its own:

at 60s
before 0 of 7 passed
after 1 of 7 passed

At 240s after the change, 9 passed, 0 failed. The per-test durations show why: the first test a worker runs costs 2.5–4.0 minutes; every later test in that worker costs 21–39 seconds. configuration.spec.ts tests 2 and 3 went from timing out to 29–31s, which is the fixture working exactly as intended. The residual is the editor and extension host starting cold on the worker's first page load, which no virtualenv fixture removes.

180s is chosen against measurements from a machine roughly twice as slow as ubuntu-2204-8, and under desktop load. It is the number most worth revisiting once there is real CI data — the first green run on the runner is the only measurement that really counts here, and I would rather set it from that than from my laptop.

One behaviour worth knowing: a test timeout restarts the worker, which destroys the worker fixture and rebuilds the virtualenv for the next test. One slow test therefore makes a whole file look broken. That is an argument for retries staying on.

Test Plan

pnpm run lint                    # ui-style: prettier + eslint + tsc, all workspaces
pnpm run ci (vscode/extension)   # test-vscode: eslint, tsc --noEmit, vitest — 19 tests
pnpm exec playwright test tests/configuration.spec.ts tests/python_env.spec.ts

All numbers above are macOS on a 4-core laptop against code-server 4.137.0 at --workers=2 --retries=0. Nothing here has been run on Linux or on the actual runner.

Checklist

  • I have run make style and fixed any issues
  • I have added tests for my changes (if applicable)
  • All existing tests pass (make fast-test)
  • My commits are signed off (git commit -s) per the DCO

Eleven end-to-end tests fail on the 60 second per-test timeout. Each of
them creates a virtual environment, installs sqlmesh into it from source
and then waits for the language server, all inside the test's own budget.
Measured on a 4 core machine that fixed work costs about 44 seconds: 18
for the install and a further 25 for the first import of sqlmesh, which
is where Python compiles the dependency tree into its bytecode cache.

Move the environment into a worker-scoped fixture with a timeout of its
own, so that it is built and warmed once per worker rather than once per
test. The fixture keeps the environment in a temporary directory of its
own, because the per-test directory is removed after every test.

The tests whose virtual environment is itself the subject of the test
keep building their own. venv_naming.spec.ts asserts on the directory
name, the tcloud tests install a mock tcloud package that must not leak
into the rest of the worker, and bad_setup.spec.ts deliberately installs
without the lsp extra.

Signed-off-by: Adegbite Ayoade <tripleaceme@gmail.com>
Turns the job back on with the same path filter the unit job uses, so
Python-only pull requests are not gated on it.

code-server is pinned rather than installed from latest. That was not
the cause of the failures - the render selector broke identically on
4.107.1, the newest release at the time the job was disabled - but an
unpinned editor means the next UI change lands as a mystery failure
rather than as a visible version bump.

The per-test timeout goes from 60 to 180 seconds. The first test a
worker runs pays for the editor and extension host starting up, which
measured at 2.5-4 minutes locally against 21-39 seconds for every later
test in the same worker, so 60 seconds only ever fit the latter. 180 is
chosen against measurements taken on a machine roughly twice as slow as
the runner, and is the number most worth revisiting once there is real
CI data.

Signed-off-by: Adegbite Ayoade <tripleaceme@gmail.com>
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.

Fix CI: re-enable and fix test-vscode-e2e

1 participant