From 7d3ade79bc6d86588c2cf1bcee4361c6494d42f6 Mon Sep 17 00:00:00 2001 From: Jonathan McPherson Date: Thu, 8 Oct 2026 16:33:43 -0700 Subject: [PATCH 1/2] fix duplicate missing packages badge --- .../positronMissingPackages.contribution.ts | 13 +++--- .../missing-packages/missing-packages.test.ts | 44 ++++++++++++++++++- 2 files changed, 50 insertions(+), 7 deletions(-) diff --git a/src/vs/workbench/contrib/positronMissingPackages/browser/positronMissingPackages.contribution.ts b/src/vs/workbench/contrib/positronMissingPackages/browser/positronMissingPackages.contribution.ts index 9fe9cc76765f..d8f831131955 100644 --- a/src/vs/workbench/contrib/positronMissingPackages/browser/positronMissingPackages.contribution.ts +++ b/src/vs/workbench/contrib/positronMissingPackages/browser/positronMissingPackages.contribution.ts @@ -52,15 +52,18 @@ workbenchRegistry.registerWorkbenchContribution(MissingPackagesPrecomputeContrib workbenchRegistry.registerWorkbenchContribution(MissingPackagesContextKeyContribution, LifecyclePhase.Restored); // Editor action bar badge (scenario 2) for scripts whose language has a session -// that supports missing packages, and Quarto documents. The editor action bar -// is disabled for notebooks, so notebooks get a separate mount below. Quarto -// documents are multi-language; the service splits them into per-language code -// chunks and routes each to its session. +// that supports missing packages, and Quarto documents. The Positron notebook +// toolbar also renders this menu, so notebooks are excluded here and get their +// own mount below. Quarto documents are multi-language; the service splits them +// into per-language code chunks and routes each to its session. PositronActionBarWidgetRegistry.registerWidget({ id: 'positronMissingPackages.editorBadge', menuId: MenuId.EditorActionsRight, order: 95, - when: MISSING_PACKAGES_SUPPORTED_KEY, + when: ContextKeyExpr.and( + MISSING_PACKAGES_SUPPORTED_KEY, + ContextKeyExpr.notEquals('activeEditor', POSITRON_NOTEBOOK_EDITOR_ID), + ), selfContained: true, componentFactory: (accessor) => () => React.createElement(MissingPackagesBadgeMount, { accessor }), }); diff --git a/test/e2e/tests/missing-packages/missing-packages.test.ts b/test/e2e/tests/missing-packages/missing-packages.test.ts index ac8b9d83065c..f74bf904e94c 100644 --- a/test/e2e/tests/missing-packages/missing-packages.test.ts +++ b/test/e2e/tests/missing-packages/missing-packages.test.ts @@ -8,11 +8,12 @@ import { join } from 'path'; import { Application } from '../../infra'; import { test as base, expect, tags } from '../_test.setup'; -// Enable the Packages pane (used to verify installs) before the app launches. +// Enable the Packages pane (used to verify installs) and Positron notebooks +// before the app launches. const test = base.extend<{}, {}>({ beforeApp: [ async ({ settingsFile }, use) => { - await settingsFile.append({ 'packages.enabled': true }); + await settingsFile.append({ 'packages.enabled': true, 'positron.notebook.enabled': true }); await use(); }, { scope: 'worker' } @@ -169,4 +170,43 @@ test.describe('Install Missing Packages', { await fs.rm(filePath, { force: true }); }); + + test('Notebook - the toolbar shows a single missing packages badge', { tag: [tags.PYTHON, tags.POSITRON_NOTEBOOKS] }, async function ({ app, python, openFile }) { + const { editors, notebooksPositron } = app.workbench; + const page = app.code.driver.currentPage; + const notebookName = 'missing_packages_badge.ipynb'; + const otherName = 'missing_packages_other.py'; + const notebookPath = join(app.workspacePathOrFolder, notebookName); + const otherPath = join(app.workspacePathOrFolder, otherName); + const badge = page.getByTestId('missing-packages-badge'); + + await test.step('Open a notebook that references a missing package', async () => { + await ensurePackageMissing(app, 'Python'); + await fs.writeFile(otherPath, 'print("other")\n'); + await fs.writeFile(notebookPath, JSON.stringify({ + cells: [{ cell_type: 'code', execution_count: null, metadata: {}, outputs: [], source: [`import ${MISSING_PACKAGE}\n`, 'print("notebook-badge")'] }], + metadata: {}, + nbformat: 4, + nbformat_minor: 5, + })); + await openFile(otherName); + await notebooksPositron.openNotebook(notebookName); + await notebooksPositron.kernel.select('Python'); + }); + + await test.step('The toolbar shows exactly one badge', async () => { + await expect(badge).toBeVisible({ timeout: 30000 }); + await expect(badge).toHaveCount(1); + }); + + await test.step('Switching away and back still shows exactly one badge', async () => { + await editors.clickTab(otherName); + await editors.clickTab(notebookName); + await expect(badge).toBeVisible({ timeout: 30000 }); + await expect(badge).toHaveCount(1); + }); + + await fs.rm(notebookPath, { force: true }); + await fs.rm(otherPath, { force: true }); + }); }); From f74e0aca7e533daeb750a9ba90a0608e278755f1 Mon Sep 17 00:00:00 2001 From: Jonathan McPherson Date: Fri, 9 Oct 2026 15:43:27 -0700 Subject: [PATCH 2/2] e2e test => unit test --- ...missingPackagesBadgeRegistration.vitest.ts | 38 ++++++++++++++++ .../missing-packages/missing-packages.test.ts | 44 +------------------ 2 files changed, 40 insertions(+), 42 deletions(-) create mode 100644 src/vs/workbench/contrib/positronMissingPackages/test/browser/missingPackagesBadgeRegistration.vitest.ts diff --git a/src/vs/workbench/contrib/positronMissingPackages/test/browser/missingPackagesBadgeRegistration.vitest.ts b/src/vs/workbench/contrib/positronMissingPackages/test/browser/missingPackagesBadgeRegistration.vitest.ts new file mode 100644 index 000000000000..fd53f8a81830 --- /dev/null +++ b/src/vs/workbench/contrib/positronMissingPackages/test/browser/missingPackagesBadgeRegistration.vitest.ts @@ -0,0 +1,38 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2026 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ + +/// + +import { MenuId } from '../../../../../platform/actions/common/actions.js'; +import { IContextKeyService } from '../../../../../platform/contextkey/common/contextkey.js'; +import { PositronActionBarWidgetRegistry } from '../../../../../platform/positronActionBar/browser/positronActionBarWidgetRegistry.js'; +import { stubInterface } from '../../../../../test/vitest/stubInterface.js'; +import { POSITRON_NOTEBOOK_EDITOR_ID } from '../../../positronNotebook/common/positronNotebookCommon.js'; +import { MISSING_PACKAGES_SUPPORTED_KEY } from '../../browser/missingPackagesContextKey.js'; +// Registers the badge widgets as a side effect. +import '../../browser/positronMissingPackages.contribution.js'; + +function badgesFor(activeEditor: string): string[] { + const values: Record = { + activeEditor, + [MISSING_PACKAGES_SUPPORTED_KEY.key]: true, + }; + const contextKeyService = stubInterface({ + contextMatchesRules: rules => !rules || rules.evaluate({ getValue: (key: string) => values[key] as T }), + }); + return PositronActionBarWidgetRegistry.getWidgets(MenuId.EditorActionsRight, contextKeyService) + .map(w => w.id) + .filter(id => id.startsWith('positronMissingPackages.')); +} + +describe('missing packages badge registration', () => { + it('shows only the notebook badge in a Positron notebook', () => { + expect(badgesFor(POSITRON_NOTEBOOK_EDITOR_ID)).toEqual(['positronMissingPackages.notebookBadge']); + }); + + it('shows only the editor badge in a text editor', () => { + expect(badgesFor('workbench.editors.files.textFileEditor')).toEqual(['positronMissingPackages.editorBadge']); + }); +}); diff --git a/test/e2e/tests/missing-packages/missing-packages.test.ts b/test/e2e/tests/missing-packages/missing-packages.test.ts index f74bf904e94c..ac8b9d83065c 100644 --- a/test/e2e/tests/missing-packages/missing-packages.test.ts +++ b/test/e2e/tests/missing-packages/missing-packages.test.ts @@ -8,12 +8,11 @@ import { join } from 'path'; import { Application } from '../../infra'; import { test as base, expect, tags } from '../_test.setup'; -// Enable the Packages pane (used to verify installs) and Positron notebooks -// before the app launches. +// Enable the Packages pane (used to verify installs) before the app launches. const test = base.extend<{}, {}>({ beforeApp: [ async ({ settingsFile }, use) => { - await settingsFile.append({ 'packages.enabled': true, 'positron.notebook.enabled': true }); + await settingsFile.append({ 'packages.enabled': true }); await use(); }, { scope: 'worker' } @@ -170,43 +169,4 @@ test.describe('Install Missing Packages', { await fs.rm(filePath, { force: true }); }); - - test('Notebook - the toolbar shows a single missing packages badge', { tag: [tags.PYTHON, tags.POSITRON_NOTEBOOKS] }, async function ({ app, python, openFile }) { - const { editors, notebooksPositron } = app.workbench; - const page = app.code.driver.currentPage; - const notebookName = 'missing_packages_badge.ipynb'; - const otherName = 'missing_packages_other.py'; - const notebookPath = join(app.workspacePathOrFolder, notebookName); - const otherPath = join(app.workspacePathOrFolder, otherName); - const badge = page.getByTestId('missing-packages-badge'); - - await test.step('Open a notebook that references a missing package', async () => { - await ensurePackageMissing(app, 'Python'); - await fs.writeFile(otherPath, 'print("other")\n'); - await fs.writeFile(notebookPath, JSON.stringify({ - cells: [{ cell_type: 'code', execution_count: null, metadata: {}, outputs: [], source: [`import ${MISSING_PACKAGE}\n`, 'print("notebook-badge")'] }], - metadata: {}, - nbformat: 4, - nbformat_minor: 5, - })); - await openFile(otherName); - await notebooksPositron.openNotebook(notebookName); - await notebooksPositron.kernel.select('Python'); - }); - - await test.step('The toolbar shows exactly one badge', async () => { - await expect(badge).toBeVisible({ timeout: 30000 }); - await expect(badge).toHaveCount(1); - }); - - await test.step('Switching away and back still shows exactly one badge', async () => { - await editors.clickTab(otherName); - await editors.clickTab(notebookName); - await expect(badge).toBeVisible({ timeout: 30000 }); - await expect(badge).toHaveCount(1); - }); - - await fs.rm(notebookPath, { force: true }); - await fs.rm(otherPath, { force: true }); - }); });