From fb222880e0505cf9d9ea953a05a518d89704989e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Mar 2026 16:03:00 +0000 Subject: [PATCH 1/3] Initial plan From 30a129136db2d4f49faa94f1279b72f6daa3c89c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Mar 2026 16:21:18 +0000 Subject: [PATCH 2/3] Fix StatusBarObserver to track concurrent downloads and add unit tests Co-authored-by: nagilson <23152278+nagilson@users.noreply.github.com> --- .../src/EventStream/StatusBarObserver.ts | 40 ++++- .../src/test/unit/StatusBarObserver.test.ts | 142 ++++++++++++++++++ 2 files changed, 181 insertions(+), 1 deletion(-) create mode 100644 vscode-dotnet-runtime-library/src/test/unit/StatusBarObserver.test.ts diff --git a/vscode-dotnet-runtime-library/src/EventStream/StatusBarObserver.ts b/vscode-dotnet-runtime-library/src/EventStream/StatusBarObserver.ts index bcb2a82af5..1d18d0b8ab 100644 --- a/vscode-dotnet-runtime-library/src/EventStream/StatusBarObserver.ts +++ b/vscode-dotnet-runtime-library/src/EventStream/StatusBarObserver.ts @@ -4,6 +4,12 @@ *--------------------------------------------------------------------------------------------*/ import * as vscode from 'vscode'; +import { + DotnetAcquisitionCompleted, + DotnetAcquisitionFinalError, + DotnetAcquisitionStarted, + DotnetInstallExpectedAbort, +} from './EventStreamEvents'; import { EventType } from './EventType'; import { IEvent } from './IEvent'; import { IEventStreamObserver } from './IEventStreamObserver'; @@ -14,17 +20,38 @@ enum StatusBarColors { } export class StatusBarObserver implements IEventStreamObserver { + private readonly inProgressDownloads: string[] = []; + constructor(private readonly statusBarItem: vscode.StatusBarItem, private readonly showLogCommand: string) { } public post(event: IEvent): void { switch (event.type) { case EventType.DotnetAcquisitionStart: + const acquisitionStarted = event as DotnetAcquisitionStarted; + this.inProgressDownloads.push(acquisitionStarted.install.installId); this.setAndShowStatusBar('$(cloud-download) Downloading .NET...', this.showLogCommand, '', 'Downloading .NET...'); break; case EventType.DotnetAcquisitionCompleted: + const acquisitionCompleted = event as DotnetAcquisitionCompleted; + this.removeFromInProgress(acquisitionCompleted.install.installId); + if (this.inProgressDownloads.length === 0) { + this.resetAndHideStatusBar(); + } + break; case EventType.DotnetInstallExpectedAbort: - this.resetAndHideStatusBar(); + const abortEvent = event as DotnetInstallExpectedAbort; + this.removeFromInProgress(abortEvent.install?.installId); + if (this.inProgressDownloads.length === 0) { + this.resetAndHideStatusBar(); + } + break; + case EventType.DotnetAcquisitionFinalError: + const finalError = event as DotnetAcquisitionFinalError; + this.removeFromInProgress(finalError.install?.installId); + if (this.inProgressDownloads.length === 0) { + this.resetAndHideStatusBar(); + } break; case EventType.DotnetAcquisitionError: this.setAndShowStatusBar('$(alert) Error acquiring .NET!', this.showLogCommand, StatusBarColors.Red, 'Error acquiring .NET'); @@ -52,4 +79,15 @@ export class StatusBarObserver implements IEventStreamObserver { this.statusBarItem.tooltip = undefined; this.statusBarItem.hide(); } + + private removeFromInProgress(installId: string | null | undefined): void { + if (installId) + { + const index = this.inProgressDownloads.indexOf(installId); + if (index >= 0) + { + this.inProgressDownloads.splice(index, 1); + } + } + } } diff --git a/vscode-dotnet-runtime-library/src/test/unit/StatusBarObserver.test.ts b/vscode-dotnet-runtime-library/src/test/unit/StatusBarObserver.test.ts new file mode 100644 index 0000000000..0acbde951e --- /dev/null +++ b/vscode-dotnet-runtime-library/src/test/unit/StatusBarObserver.test.ts @@ -0,0 +1,142 @@ +/*--------------------------------------------------------------------------------------------- +* Licensed to the .NET Foundation under one or more agreements. +* The .NET Foundation licenses this file to you under the MIT license. +*--------------------------------------------------------------------------------------------*/ +import * as chai from 'chai'; +import { DotnetInstall } from '../../Acquisition/DotnetInstall'; +import { + DotnetAcquisitionCompleted, + DotnetAcquisitionFinalError, + DotnetAcquisitionStarted, + DotnetInstallCancelledByUserError, + EventBasedError, +} from '../../EventStream/EventStreamEvents'; +import { StatusBarObserver } from '../../EventStream/StatusBarObserver'; + +const assert = chai.assert; +const defaultTimeoutTime = 5000; + +class MockStatusBarItem +{ + public text = ''; + public command: string | undefined = undefined; + public color: string | undefined = undefined; + public tooltip: string | undefined = undefined; + public isVisible = false; + + public show(): void + { + this.isVisible = true; + } + + public hide(): void + { + this.isVisible = false; + } +} + +const makeInstall = (id: string): DotnetInstall => ({ + version: id, + isGlobal: false, + architecture: 'x64', + installId: id, + installMode: 'runtime', +}); + +suite('StatusBarObserver Unit Tests', function () +{ + const showLogCommand = 'dotnet.showLog'; + let mockStatusBarItem: MockStatusBarItem; + let observer: StatusBarObserver; + + setup(() => + { + mockStatusBarItem = new MockStatusBarItem(); + // Cast to any to avoid needing the full vscode.StatusBarItem interface + // eslint-disable-next-line @typescript-eslint/no-explicit-any + observer = new StatusBarObserver(mockStatusBarItem as any, showLogCommand); + }); + + test('Status bar shows when acquisition starts', () => + { + const install = makeInstall('8.0~x64'); + observer.post(new DotnetAcquisitionStarted(install, 'test-ext')); + + assert.isTrue(mockStatusBarItem.isVisible, 'Status bar should be visible after acquisition starts'); + assert.include(mockStatusBarItem.text, 'Downloading .NET', 'Status bar should display downloading text'); + }).timeout(defaultTimeoutTime); + + test('Status bar hides when single acquisition completes', () => + { + const install = makeInstall('8.0~x64'); + observer.post(new DotnetAcquisitionStarted(install, 'test-ext')); + observer.post(new DotnetAcquisitionCompleted(install, '/path/to/dotnet', '8.0')); + + assert.isFalse(mockStatusBarItem.isVisible, 'Status bar should be hidden after acquisition completes'); + }).timeout(defaultTimeoutTime); + + test('Status bar stays visible when one of multiple concurrent downloads completes', () => + { + const installA = makeInstall('8.0~x64'); + const installB = makeInstall('9.0~x64'); + + observer.post(new DotnetAcquisitionStarted(installA, 'test-ext')); + observer.post(new DotnetAcquisitionStarted(installB, 'test-ext')); + + // Complete the first download - status bar should remain visible because B is still in progress + observer.post(new DotnetAcquisitionCompleted(installA, '/path/to/dotnet', '8.0')); + + assert.isTrue(mockStatusBarItem.isVisible, 'Status bar should remain visible while other downloads are in progress'); + }).timeout(defaultTimeoutTime); + + test('Status bar hides only after all concurrent downloads complete', () => + { + const installA = makeInstall('8.0~x64'); + const installB = makeInstall('9.0~x64'); + + observer.post(new DotnetAcquisitionStarted(installA, 'test-ext')); + observer.post(new DotnetAcquisitionStarted(installB, 'test-ext')); + + observer.post(new DotnetAcquisitionCompleted(installA, '/path/to/dotnet', '8.0')); + assert.isTrue(mockStatusBarItem.isVisible, 'Status bar should remain visible after first of two downloads completes'); + + observer.post(new DotnetAcquisitionCompleted(installB, '/path/to/dotnet', '9.0')); + assert.isFalse(mockStatusBarItem.isVisible, 'Status bar should hide after all downloads complete'); + }).timeout(defaultTimeoutTime); + + test('Status bar hides when acquisition is aborted', () => + { + const install = makeInstall('8.0~x64'); + observer.post(new DotnetAcquisitionStarted(install, 'test-ext')); + + const abortError = new EventBasedError('TestAbort', 'Installation cancelled'); + observer.post(new DotnetInstallCancelledByUserError(abortError, install)); + + assert.isFalse(mockStatusBarItem.isVisible, 'Status bar should hide when acquisition is aborted'); + }).timeout(defaultTimeoutTime); + + test('Status bar hides when final error occurs for single download', () => + { + const install = makeInstall('8.0~x64'); + observer.post(new DotnetAcquisitionStarted(install, 'test-ext')); + + const finalError = new DotnetAcquisitionFinalError(new EventBasedError('TestError', 'Download failed'), 'TestEvent', install); + observer.post(finalError); + + assert.isFalse(mockStatusBarItem.isVisible, 'Status bar should hide when acquisition fails with final error'); + }).timeout(defaultTimeoutTime); + + test('Status bar stays visible when one of multiple concurrent downloads fails with final error', () => + { + const installA = makeInstall('8.0~x64'); + const installB = makeInstall('9.0~x64'); + + observer.post(new DotnetAcquisitionStarted(installA, 'test-ext')); + observer.post(new DotnetAcquisitionStarted(installB, 'test-ext')); + + const finalError = new DotnetAcquisitionFinalError(new EventBasedError('TestError', 'Download failed'), 'TestEvent', installA); + observer.post(finalError); + + assert.isTrue(mockStatusBarItem.isVisible, 'Status bar should remain visible when one download fails but another is still in progress'); + }).timeout(defaultTimeoutTime); +}); From d515f43c188ecbdeb57a5609bf5bbb5eb9cc9cfd Mon Sep 17 00:00:00 2001 From: Noah Gilson Date: Wed, 8 Apr 2026 16:14:32 -0700 Subject: [PATCH 3/3] Use Set for inProgressDownloads to handle duplicate installId requests Address review feedback: two extensions (e.g. C# and C# Dev Kit) can request the same installId simultaneously. In the global SDK path, DotnetAcquisitionStarted fires before the inner installer lock, so duplicate start events are possible with only one completion event following. Changing from string[] to Set makes tracking idempotent - duplicate starts for the same installId are naturally deduplicated, so a single completion correctly clears the tracking and hides the status bar. Added tests for: - Duplicate installId starts with single completion - Completion for unknown installId (graceful no-op) - Mixed completion/error with duplicate starts Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/EventStream/StatusBarObserver.ts | 18 +++---- .../src/test/unit/StatusBarObserver.test.ts | 51 +++++++++++++++++++ 2 files changed, 59 insertions(+), 10 deletions(-) diff --git a/vscode-dotnet-runtime-library/src/EventStream/StatusBarObserver.ts b/vscode-dotnet-runtime-library/src/EventStream/StatusBarObserver.ts index 1d18d0b8ab..58fb003843 100644 --- a/vscode-dotnet-runtime-library/src/EventStream/StatusBarObserver.ts +++ b/vscode-dotnet-runtime-library/src/EventStream/StatusBarObserver.ts @@ -20,7 +20,9 @@ enum StatusBarColors { } export class StatusBarObserver implements IEventStreamObserver { - private readonly inProgressDownloads: string[] = []; + // Use a Set so duplicate start events for the same installId (possible in the global SDK path, + // where DotnetAcquisitionStarted fires before the inner installer lock) are naturally deduplicated. + private readonly inProgressDownloads: Set = new Set(); constructor(private readonly statusBarItem: vscode.StatusBarItem, private readonly showLogCommand: string) { } @@ -29,27 +31,27 @@ export class StatusBarObserver implements IEventStreamObserver { switch (event.type) { case EventType.DotnetAcquisitionStart: const acquisitionStarted = event as DotnetAcquisitionStarted; - this.inProgressDownloads.push(acquisitionStarted.install.installId); + this.inProgressDownloads.add(acquisitionStarted.install.installId); this.setAndShowStatusBar('$(cloud-download) Downloading .NET...', this.showLogCommand, '', 'Downloading .NET...'); break; case EventType.DotnetAcquisitionCompleted: const acquisitionCompleted = event as DotnetAcquisitionCompleted; this.removeFromInProgress(acquisitionCompleted.install.installId); - if (this.inProgressDownloads.length === 0) { + if (this.inProgressDownloads.size === 0) { this.resetAndHideStatusBar(); } break; case EventType.DotnetInstallExpectedAbort: const abortEvent = event as DotnetInstallExpectedAbort; this.removeFromInProgress(abortEvent.install?.installId); - if (this.inProgressDownloads.length === 0) { + if (this.inProgressDownloads.size === 0) { this.resetAndHideStatusBar(); } break; case EventType.DotnetAcquisitionFinalError: const finalError = event as DotnetAcquisitionFinalError; this.removeFromInProgress(finalError.install?.installId); - if (this.inProgressDownloads.length === 0) { + if (this.inProgressDownloads.size === 0) { this.resetAndHideStatusBar(); } break; @@ -83,11 +85,7 @@ export class StatusBarObserver implements IEventStreamObserver { private removeFromInProgress(installId: string | null | undefined): void { if (installId) { - const index = this.inProgressDownloads.indexOf(installId); - if (index >= 0) - { - this.inProgressDownloads.splice(index, 1); - } + this.inProgressDownloads.delete(installId); } } } diff --git a/vscode-dotnet-runtime-library/src/test/unit/StatusBarObserver.test.ts b/vscode-dotnet-runtime-library/src/test/unit/StatusBarObserver.test.ts index 0acbde951e..6dceb66a89 100644 --- a/vscode-dotnet-runtime-library/src/test/unit/StatusBarObserver.test.ts +++ b/vscode-dotnet-runtime-library/src/test/unit/StatusBarObserver.test.ts @@ -139,4 +139,55 @@ suite('StatusBarObserver Unit Tests', function () assert.isTrue(mockStatusBarItem.isVisible, 'Status bar should remain visible when one download fails but another is still in progress'); }).timeout(defaultTimeoutTime); + + test('Status bar hides correctly when same installId is started twice (duplicate request)', () => + { + // Simulates the global SDK path where DotnetAcquisitionStarted fires before the inner lock, + // so two extensions requesting the same version can each trigger a start event. + const install = makeInstall('8.0~x64'); + observer.post(new DotnetAcquisitionStarted(install, 'ext-csharp')); + observer.post(new DotnetAcquisitionStarted(install, 'ext-csharpdk')); + + assert.isTrue(mockStatusBarItem.isVisible, 'Status bar should be visible after duplicate starts'); + + // Only one completion fires (the actual install) + observer.post(new DotnetAcquisitionCompleted(install, '/path/to/dotnet', '8.0')); + + assert.isFalse(mockStatusBarItem.isVisible, 'Status bar should hide after completion even with duplicate starts'); + }).timeout(defaultTimeoutTime); + + test('Status bar handles completion for unknown installId gracefully', () => + { + const installA = makeInstall('8.0~x64'); + const installB = makeInstall('9.0~x64'); + + // Only start A + observer.post(new DotnetAcquisitionStarted(installA, 'test-ext')); + + // Complete B (never started) - should not crash or affect status bar + observer.post(new DotnetAcquisitionCompleted(installB, '/path/to/dotnet', '9.0')); + + assert.isTrue(mockStatusBarItem.isVisible, 'Status bar should remain visible since A is still in progress'); + }).timeout(defaultTimeoutTime); + + test('Status bar handles duplicate start with mixed completion and error', () => + { + const install = makeInstall('8.0~x64'); + const otherInstall = makeInstall('9.0~x64'); + + observer.post(new DotnetAcquisitionStarted(install, 'ext-a')); + observer.post(new DotnetAcquisitionStarted(install, 'ext-b')); + observer.post(new DotnetAcquisitionStarted(otherInstall, 'ext-c')); + + // Error on the duplicate - should remove it from tracking + const finalError = new DotnetAcquisitionFinalError(new EventBasedError('TestError', 'Failed'), 'TestEvent', install); + observer.post(finalError); + + assert.isTrue(mockStatusBarItem.isVisible, 'Status bar should remain visible since 9.0 is still in progress'); + + // Complete the other install + observer.post(new DotnetAcquisitionCompleted(otherInstall, '/path/to/dotnet', '9.0')); + + assert.isFalse(mockStatusBarItem.isVisible, 'Status bar should hide after all unique installs complete'); + }).timeout(defaultTimeoutTime); });