Conversation
asserts get stripped under python -O / PYTHONOPTIMIZE, which would let the smoke print PASS even when checks failed. Use RuntimeError so validation runs regardless of interpreter optimization flags.
Selftests don't exercise Chromium and are excluded by .dockerignore. They already run on Ubuntu in ci.yml across Python 3.10 and 3.12. Build-time uv sync + playwright install prove the install path works. The smoke's job is to confirm the headless Chromium fallback runs on RHEL — that's docker/rhel-smoke.py.
The justfile lint recipes referenced 'tests/' which doesn't exist in the repo (vip_tests lives under src/). 'just check' has been failing on a clean checkout because of this. Drop tests/, and add docker/ so the new docker/rhel-smoke.py is covered. Mirror the change in ci.yml's ruff-action invocations.
Connect's user.username is the local part of the email (e.g. "brian.deitte"),
not the full address ("brian.deitte@posit.co"). Strip the domain from
test_username before comparing.
Generated via rsconnect::writeManifest() in an R 4.6.0 container with packages from packagemanager.posit.co/cran/__linux__/noble/latest. The old manifest pinned Rcpp 1.0.14, whose source no longer compiles on R 4.6 because R_ext/PrtUtil.h was removed. Connect deployments to servers with only R 4.6 available now succeed.
The plumber manifest's platform field is now overwritten with the R version Connect reports, matching the shiny/rmarkdown pattern. Drops the static-bundle indirection since plumber was the only entry.
Same root cause as the plumber regen: stale package pins (base64enc 0.1-3, etc.) failed to compile against R 4.6 because legacy R API symbols like SETLENGTH were removed. Regenerated both via rsconnect::writeManifest() in rocker/r-ver:4.6.0 with packages from packagemanager.posit.co/cran/__linux__/noble/latest.
Connect's server_settings/r endpoint lists installations in install order, not by version, so r_versions[0] can return the oldest R on a multi-version server. The regenerated manifests target current packages (R >= 4.1), so deploying against the oldest R fails the package install. Pick the highest version by numeric component instead, in plumber, shiny, rmarkdown, and quarto bundles.
…playwright launch error, replan absent manifest packages
…leanly Remove the never-read `system_packages_by_manager` field from `UninstallPlan` and stop populating it in `build_uninstall_plan`. Wrap `run_install` in a try/except that catches `PlaywrightInstallError` and `PackageQueryError` and prints a clean error to stderr + exits 1 instead of leaking a traceback. Drop pointless wrapper lambdas in favour of direct function references, clarify the `--skip-system` help text to explain the uninstall trade-off, remove the unused `manifest_dir` conftest fixture, and add two new tests (error-surface unit test + install/uninstall round-trip integration test).
Users who want to remove .venv can run `rm -rf .venv` themselves. Trims the public CLI surface and removes the associated plan/runner logic, dataclass field, and test coverage.
Replaces 'uv run playwright install [--with-deps] chromium' across READMEs, Dockerfile, CI workflows, troubleshooting hints, and the auth remediation message. CI workflows use --skip-system to match the prior no-system-step behavior.
Capture playwright install output, drop the two-line BEWARE preamble that fires on unsupported-OS hosts (RHEL family), and replace it with a single vip-attributed summary line. Forwards everything else unchanged.
vip never invoked the sudo removal command itself — it only printed it for the user to run. --system was just a gate on whether to print that command. Always printing it is simpler: users see the full plan on a dry-run, and --yes users see it after the user-space teardown finishes.
Replaces capture_output=True with subprocess.Popen and a thread per pipe, so download progress streams in real time. BEWARE filtering and the vip-attributed summary line carry over unchanged. Tests now mock subprocess.Popen with a StringIO-backed fake.
RHEL/Rocky/Alma/Oracle/CentOS support is now invisible to most users — vip install handles platform detection, package lists, and the sudo-print fallback. The standalone rhel.md was mostly historical context (Playwright fallback details, glibc notes) that doesn't belong in user-facing docs. Replaces the playwright install --with-deps line in the website's Quick start with vip install, and adds a short sentence enumerating supported platforms so RHEL users find vip via search.
Drop the '(content cleanup: skipped)' suffix when no chained cleanup ran — confusing for the common case where vip.toml has no Connect URL. After --yes execution, print the commands users need to also remove vip itself: 'uv tool uninstall posit-vip' and 'uv pip uninstall posit-vip'.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds first-class vip install / vip uninstall commands to provision Playwright Chromium + OS-specific system libraries (including RHEL-family) and to safely reverse user-space artifacts via a per-project .vip-install.json manifest. Also refreshes Connect deploy test fixtures to be less sensitive to runtime installation ordering.
Changes:
- Introduce
vip install(platform detection, system package planning, Playwright Chromium install) andvip uninstall(manifest-driven cleanup + optional chainedvip cleanup). - Update docs, CI workflows, and setup tooling to use
vip installinstead ofplaywright install --with-deps. - Adjust Connect deploy tests/manifests to use latest detected runtime versions and updated dependency manifests.
Reviewed changes
Copilot reviewed 41 out of 44 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| website/src/pages/getting-started.astro | Updates quick-start instructions to use vip install / vip uninstall. |
| uv.lock | Bumps editable package version and refreshes locked deps. |
| src/vip_tests/troubleshooting.toml | Updates troubleshooting remediation command to uv run vip install. |
| src/vip_tests/connect/test_users.py | Makes username assertion robust to email-vs-username formats. |
| src/vip_tests/connect/test_content_deploy.py | Selects newest runtime versions for generated manifests during deploy tests. |
| src/vip_tests/connect/rmarkdown_manifest.json | Updates stored Connect manifest fixture (newer R/platform + package metadata). |
| src/vip_tests/connect/plumber_manifest.json | Updates stored Connect manifest fixture (newer R/platform + package metadata). |
| src/vip/install/init.py | New install/uninstall package namespace. |
| src/vip/install/manifest.py | Implements .vip-install.json schema, load/save, and pending-package helpers. |
| src/vip/install/packages.py | Adds rpm/dpkg-query helpers to detect installed system packages. |
| src/vip/install/platform.py | Adds OS detection and canonical Debian/RHEL Chromium dependency lists. |
| src/vip/install/plan.py | Adds pure builders for install/uninstall plans. |
| src/vip/install/playwright.py | Wraps playwright install chromium, cache detection, and output filtering. |
| src/vip/install/runner.py | Formats and executes install/uninstall plans and persists manifest updates. |
| src/vip/cli.py | Adds vip install and vip uninstall subcommands and wiring. |
| src/vip/auth.py | Updates missing-deps remediation hint to uv run vip install. |
| selftests/test_auth.py | Updates remediation assertions to match new command. |
| selftests/install/conftest.py | Adds install/uninstall selftest fixture module. |
| selftests/install/test_manifest.py | Adds tests for manifest schema IO and validation behavior. |
| selftests/install/test_packages.py | Adds tests for rpm/dpkg query wrappers. |
| selftests/install/test_platform.py | Adds tests for platform detection and Playwright pin review guard. |
| selftests/install/test_plan.py | Adds tests for planning logic (install/uninstall). |
| selftests/install/test_playwright.py | Adds tests for cache detection and BEWARE filtering behavior. |
| selftests/install/test_runner.py | Adds tests for plan formatting/execution and manifest updates. |
| selftests/install/test_cli_install.py | Adds CLI smoke tests for vip install. |
| selftests/install/test_cli_uninstall.py | Adds CLI smoke tests for vip uninstall + chained cleanup wiring. |
| scripts/rhel-smoke.sh | Adds local Docker-based RHEL smoke runner script. |
| plans/connect-ci-testing.md | Updates CI/testing plan doc to use vip install --skip-system. |
| justfile | Updates setup to use vip install and adds RHEL smoke targets; expands ruff paths. |
| docker/rhel9/Dockerfile | Adds RHEL9 smoke image that runs vip install. |
| docker/rhel10/Dockerfile | Adds RHEL10 smoke image that runs vip install. |
| docker/rhel-smoke.py | Adds headless Chromium smoke test used by RHEL images. |
| README.md | Replaces Playwright install instructions and adds uninstall documentation. |
| IMPLEMENTATION_GUIDE.md | Updates setup instructions to reflect vip install. |
| Dockerfile | Uses vip install during image build instead of playwright install --with-deps. |
| AGENTS.md | Documents vip install/vip uninstall behavior and new module map. |
| .gitignore | Ignores .vip-install.json and a new report artifact file. |
| .github/workflows/connect-smoke.yml | Uses vip install --skip-system in CI. |
| .github/workflows/workbench-smoke.yml | Uses vip install --skip-system in CI. |
| .github/workflows/example-report.yml | Uses vip install --skip-system in CI. |
| .github/workflows/ci.yml | Expands ruff check/format paths to include docker/. |
| .github/workflows/rhel-smoke.yml | Adds CI job to build/run RHEL9 & RHEL10 smoke images. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+90
to
+96
| if needs_root and system_step is not None and is_root(): | ||
| _install_system_packages(system_step.manager, system_step.packages) | ||
| for name in system_step.packages: | ||
| manifest.items.append( | ||
| SystemPackageItem(manager=system_step.manager, name=name, installed_at=now) | ||
| ) | ||
|
|
Comment on lines
+104
to
+107
| if filtered: | ||
| print( | ||
| "Installed Playwright Chromium (Ubuntu 24.04 fallback build for unsupported-OS hosts)." | ||
| ) |
Comment on lines
+139
to
+141
| items=items, | ||
| pending_system_packages=list(data.get("pending_system_packages", [])), | ||
| ) |
Comment on lines
+1221
to
+1222
| "Skip the system-package step. VIP will not track those packages in " | ||
| ".vip-install.json, so vip uninstall --system will not propose to remove them. " |
Comment on lines
+21
to
+23
| echo "==> verifying vip uninstall --yes --venv --system runs cleanly" | ||
| docker run --rm --platform linux/amd64 "vip-rhel${version}-smoke" \ | ||
| /bin/sh -c 'uv run vip uninstall --yes --venv --system | tee /tmp/uninst.log; \ |
Comment on lines
+1
to
+35
| """Headless Chromium smoke test for RHEL 9. | ||
|
|
||
| Mirrors microsoft/playwright#40312's manual test: launches Chromium | ||
| headlessly, exercises basic page interaction and JS evaluation, and | ||
| takes a screenshot. Exits non-zero on any failure so docker run | ||
| propagates the result. | ||
| """ | ||
|
|
||
| from playwright.sync_api import sync_playwright | ||
|
|
||
|
|
||
| def main() -> None: | ||
| with sync_playwright() as p: | ||
| browser = p.chromium.launch(headless=True) | ||
| page = browser.new_page() | ||
| page.goto( | ||
| "data:text/html,<title>RHEL9 smoke</title><h1 id=h>ok</h1><input id=i><div id=r></div>" | ||
| ) | ||
| title = page.title() | ||
| if title != "RHEL9 smoke": | ||
| raise RuntimeError(f"unexpected title: {title!r}") | ||
| page.fill("#i", "hello from rhel9") | ||
| page.evaluate( | ||
| "document.getElementById('r').textContent = document.getElementById('i').value" | ||
| ) | ||
| text = page.text_content("#r") | ||
| if text != "hello from rhel9": | ||
| raise RuntimeError(f"unexpected DOM text: {text!r}") | ||
| ua = page.evaluate("navigator.userAgent") | ||
| if "HeadlessChrome" not in ua: | ||
| raise RuntimeError(f"unexpected UA: {ua!r}") | ||
| page.screenshot(path="/tmp/smoke.png") | ||
| browser.close() | ||
| print("PASS: rhel9 headless chromium smoke") | ||
|
|
- runner: clear pending_system_packages on root install path - playwright: drop hard-coded Ubuntu 24.04 from the fallback summary - manifest: validate pending_system_packages is a list of strings - cli: fix --skip-system help text (no more --system on uninstall) - rhel-smoke.sh: drop removed --venv/--system flags - rhel-smoke.py: version-agnostic strings for shared use across RHEL 9/10
The previous wording implied a prior install run, but vip install can hit this branch on the very first invocation (e.g., on macOS where there's no system step, with a Playwright cache already present from another project). Neutral phrasing works for both first-run and re-run cases.
statik
approved these changes
May 1, 2026
📸 Preview Screenshots — PR #24110 screenshots captured from both preview deployments (all pages rendered successfully, no failures). 🌐 Website PreviewPreview URL: https://posit-dev.github.io/vip/pr-preview-site/pr-241/ Home Getting Started Shiny App Tests Feature Matrix Report Example Report (viewport screenshot — page is very long) Example Report Details 📊 Report PreviewPreview URL: https://posit-dev.github.io/vip/pr-preview/pr-241/ Home (viewport screenshot — page is very long) Details Pages captured: 10 / 10
|
|
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I took #227 and #228 together, as they have overlap.