Add Positron API integration tests and CI workflow - #4298
Conversation
|
Claude finished @jonvanausdeln's task in 1m 13s —— View job Code Review Complete
This is a well-structured PR that adds valuable integration tests for Publisher's Positron API surface. The implementation follows established patterns and the test coverage targets the right code paths that weren't previously testable. HighlightsStrong test design — The tests exercise the real API boundary ( Clean separation — Keeping Positron tests in Good CI integration — The workflow uses date-keyed caching for the Positron download, properly gates on Minor Suggestions
Questions
VerdictLGTM — This is solid work that fills an important gap in test coverage. The suggestions above are all minor polish items that shouldn't block merging. |
dotNomad
left a comment
There was a problem hiding this comment.
Had a few comments / questions for you including this note about a stack trace in the logs:
Looking at the Run Positron API Tests run in the CI I'm seeing this:
Server: Start requested.
Server State: Starting
Server State: Running
rejected promise not handled within 1 second: Error: GitHubLoginFailed
stack trace: Error: GitHubLoginFailed
at Ere._authShowWarnings (/Users/runner/work/publisher/publisher/extensions/vscode/.positron-test/stable/2026.07.1-5/darwin-arm64/Positron.app/Contents/Resources/app/extensions/copilot/dist/extension.js:1697:18535)
at async Ere.getCopilotToken (/Users/runner/work/publisher/publisher/extensions/vscode/.positron-test/stable/2026.07.1-5/darwin-arm64/Positron.app/Contents/Resources/app/extensions/copilot/dist/extension.js:1697:16253)
at async Ege.getCopilotToken (/Users/runner/work/publisher/publisher/extensions/vscode/.positron-test/stable/2026.07.1-5/darwin-arm64/Positron.app/Contents/Resources/app/extensions/copilot/dist/extension.js:1400:7787)
at async Jde.updateCachedToken (/Users/runner/work/publisher/publisher/extensions/vscode/.positron-test/stable/2026.07.1-5/darwin-arm64/Positron.app/Contents/Resources/app/extensions/copilot/dist/extension.js:2954:8746)
An unknown error occurred. Please consult the log for more details.
rejected promise not handled within 1 second: Error: GitHubLoginFailed
stack trace: Error: GitHubLoginFailed
at Ere._authShowWarnings (/Users/runner/work/publisher/publisher/extensions/vscode/.positron-test/stable/2026.07.1-5/darwin-arm64/Positron.app/Contents/Resources/app/extensions/copilot/dist/extension.js:1697:18535)
at async Ere.getCopilotToken (/Users/runner/work/publisher/publisher/extensions/vscode/.positron-test/stable/2026.07.1-5/darwin-arm64/Positron.app/Contents/Resources/app/extensions/copilot/dist/extension.js:1697:16253)
at async Ege.getCopilotToken (/Users/runner/work/publisher/publisher/extensions/vscode/.positron-test/stable/2026.07.1-5/darwin-arm64/Positron.app/Contents/Resources/app/extensions/copilot/dist/extension.js:1400:7787)
at async NS._fetchModels (/Users/runner/work/publisher/publisher/extensions/vscode/.positron-test/stable/2026.07.1-5/darwin-arm64/Positron.app/Contents/Resources/app/extensions/copilot/dist/extension.js:6787:4399)
rejected promise not handled within 1 second: Error: GitHubLoginFailed
stack trace: Error: GitHubLoginFailed
at Ere._authShowWarnings (/Users/runner/work/publisher/publisher/extensions/vscode/.positron-test/stable/2026.07.1-5/darwin-arm64/Positron.app/Contents/Resources/app/extensions/copilot/dist/extension.js:1697:18535)
at async Ere.getCopilotToken (/Users/runner/work/publisher/publisher/extensions/vscode/.positron-test/stable/2026.07.1-5/darwin-arm64/Positron.app/Contents/Resources/app/extensions/copilot/dist/extension.js:1697:16253)
at async Ege.getCopilotToken (/Users/runner/work/publisher/publisher/extensions/vscode/.positron-test/stable/2026.07.1-5/darwin-arm64/Positron.app/Contents/Resources/app/extensions/copilot/dist/extension.js:1400:7787)
at async Jde.updateCachedToken (/Users/runner/work/publisher/publisher/extensions/vscode/.positron-test/stable/2026.07.1-5/darwin-arm64/Positron.app/Contents/Resources/app/extensions/copilot/dist/extension.js:2954:8746)
Looks like this an issue with the Copilot extension inside of Positron, thought it was worth noting for this.
|
|
||
| - The Positron API is reached through the `acquirePositronApi()` global that | ||
| Positron injects into the extension host (typed by | ||
| `src/@types/positron.d.ts`). Publisher's own code feature-detects Positron |
|
|
||
| /** | ||
| * Positron may report runtime paths with a leading `~/`, which Publisher | ||
| * expands before use (https://github.com/posit-dev/positron/issues/12942). |
There was a problem hiding this comment.
It looks like this issue was closed - is this workaround still necessary?
There was a problem hiding this comment.
It was fixed (posit-dev/positron#14813), but the fix is only in dailies right now. This suite runs against the stable channel (currently 2026.07.1), CI is testing against a build that predates the fix, and Publisher's production code (utils/vscode.ts) still expands ~/ to stay compatible with pre-2026.08 builds regardless. This test mirrors what Publisher does, not what the newest Positron does, so the helper stays until the expansion leaves utils/vscode.ts. I've updated the comment to make that lifecycle explicit.
| }); | ||
|
|
||
| const testsRoot = __dirname; | ||
| for (const file of fs.readdirSync(testsRoot)) { |
There was a problem hiding this comment.
Should we use { recursive: true } on the readdirSync so we capture any tests in a sub-directory?
Doesn't affect is now, but could be handy if we make sub-directories in the future.
There was a problem hiding this comment.
Done — switched to { recursive: true }
| const runtime = await api.runtime.getPreferredRuntime(languageId); | ||
| if (runtime) { | ||
| return runtime; | ||
| } |
There was a problem hiding this comment.
Should we throw if we don't get a runtime back from getPreferredRuntime?
lastError doesn't get set if we have a falsey runtime which could make it confusing when reading the logs.
We could also change the assert.ok string to specify if positron reported no preferred runtime.
There was a problem hiding this comment.
I agree, the loop is now an explicit while (Date.now() < deadline), and the failure message distinguishes "getPreferredRuntime threw: …" from "resolved without reporting a preferred runtime", so the empty-result case no longer prints Last error: undefined.
| runs-on: macos-latest | ||
| steps: | ||
| - uses: actions/checkout@v7 | ||
| - uses: actions/setup-node@v6 |
There was a problem hiding this comment.
We just updated to use setup-node@v7, would be an easy up here.
There was a problem hiding this comment.
Done — rebased onto main and bumped.
| cache-dependency-path: "**/package-lock.json" | ||
| # The interpreter-discovery tests need Python and R installed so | ||
| # Positron's runtime discovery can find them. | ||
| - uses: actions/setup-python@v6 |
There was a problem hiding this comment.
Similar note here about using setup-python@v7
There was a problem hiding this comment.
Done — rebased onto main and bumped.
| import Mocha from "mocha"; | ||
|
|
||
| export function run(): Promise<void> { | ||
| const mocha = new Mocha({ |
There was a problem hiding this comment.
I'm curious if @posit-dev/positron-test-electron has any plans to create a Positron package similar to @vscode/test-cli that handles the Mocha bootstrapping. We are using it elsewhere in the Publisher codebase and the setup is a bit simpler.
There was a problem hiding this comment.
Nothing currently, but I just opened an issue capturing it. posit-dev/positron-test-electron#4
Adds a Positron-only integration test suite that runs the Publisher extension inside a real Positron build via @posit-dev/positron-test-electron, covering the extension's actual Positron API usage (acquirePositronApi + runtime.getPreferredRuntime interpreter discovery) that the plain VSCode suite and the mock-based contract tests cannot reach. - src/test/positron/: Mocha entry point plus starter tests for API acquisition, extension activation, and Python/R interpreter discovery against the live API - scripts/run-positron-tests.mjs: launcher (npm run test-positron; POSITRON_CHANNEL=daily supported; macOS only for now, upstream limitation) - esbuild.tests.mjs/.vscode-test.mjs: separate the plain suite (out/test/suite) from the Positron suite (out/test/positron) - .github/workflows/positron-api-tests.yaml: runs on PRs and main (macos-latest, Python 3.12 + R 4.4, cached Positron download) Also raises the scanPythonDependencies integration-test timeout from 30s to 120s (matching the scanRPackages block) — the real-Python scan takes ~25s alone and flakes under parallel suite load. Part of posit-dev/positron#14531
- Disable the bundled Copilot extension in the test launch so it stops spamming the logs with failed GitHub auth attempts on credential-less CI runners - Make waitForPreferredRuntime distinguish "getPreferredRuntime threw" from "resolved without a runtime" in its failure message, and use an explicit while-deadline loop - Clarify that the ~/ expansion mirror stays for compatibility with Positron builds older than the posit-dev/positron#12942 fix - Discover tests recursively so future subdirectories are picked up - Bump setup-node and setup-python to v7, matching main - Note upstream Windows/Linux support status in the README and launcher
404e0f9 to
2a23931
Compare
Good catch — that's Positron's bundled Copilot extension (GitHub.copilot-chat) failing GitHub auth on the credential-less runner. We launch with extensions enabled because runtime discovery lives in the bundled Python/Ark extensions, so Copilot was loading along for the ride. Fixed by disabling just it via --disable-extension GitHub.copilot-chat in the launcher; the logs should be clean now |
dotNomad
left a comment
There was a problem hiding this comment.
Thank you for doing this and for the updates. This is looking great to merge 🎉
Summary
Closes posit-dev/positron#14963
Adds a Positron-only integration test suite that runs the Publisher extension inside a real Positron build via
@posit-dev/positron-test-electron, plus a CI workflow that runs it on every PR and push tomain.Part of the rollout tracked in posit-dev/positron#14531 (pattern established in quarto-dev/quarto#1058).
What's covered
Publisher's real Positron-API surface is interpreter discovery (
src/utils/vscode.ts):acquirePositronApi()feature detection pluspositron.runtime.getPreferredRuntime()with VSCode fallbacks. The starter tests exercise exactly that against the live API — paths unreachable by the plain VSCode suite or the mock-based contract tests:extension.test.ts— Positron injects theacquirePositronApiglobal, the API reports a version, and Publisher activates in the Positron extension host.interpreter-discovery.test.ts— Publisher'sgetPythonInterpreterPath()/getRInterpreterPath()resolve the same interpreter Positron reports as its preferred runtime, covering API acquisition, the retry loop, and~/runtime-path expansion (Provide 2 distinct fields for runtime path: one full path, one for display positron#12942). The tests wait for Positron's async runtime discovery before asserting, so they measure Publisher's behavior rather than discovery timing.Harness
scripts/run-positron-tests.mjs— downloads/caches a Positron build (stable by default,POSITRON_CHANNEL=dailysupported) and runs the Mocha entry point (src/test/positron/index.ts) inside it. Bundled extensions stay enabled because runtime discovery is provided by the bundled Python and Ark extensions.npm run test-positron(macOS only for now — an upstreampositron-test-electronlimitation; Windows/Linux follow once extractable archives are published).esbuild.tests.mjs/.vscode-test.mjs— the plain suite (out/test/suite/) and Positron suite (out/test/positron/) are now separated sonpm testdoesn't pick up Positron-only tests.src/test/positron/README.mddocuments how to run and add tests.CI
.github/workflows/positron-api-tests.yaml(macos-latest, Python 3.12 + R 4.4 for runtime discovery, date-keyed cache of the Positron download, launched throughposit-dev/setup-positron), wired intopull-request.yamlandmain.yaml.Drive-by fix
Raises the
scanPythonDependencies (real Python)integration-test timeout from 30s to 120s, matching thescanRPackagesblock above it — the real-Python scan takes ~25s on its own and flakes under parallel suite load.Follow-up (not in this PR)
Migrating from the hand-rolled
src/@types/positron.d.tsto the published@posit-dev/positronpackage is tracked in #3647.Testing
npm test(vanilla VSCode suite) passes with the new output layouttsc, ESLint, Prettier, and the full Vitest unit suite pass