-
Notifications
You must be signed in to change notification settings - Fork 454
Fix "Downloading .NET..." status bar indicator not clearing after download completes #2607
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
fb22288
30a1291
bd5327a
d515f43
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -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) | ||||||||
| { | ||||||||
|
Comment on lines
+86
to
+87
|
||||||||
| if (installId) | |
| { | |
| if (installId) { |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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); | ||
| }); |
Uh oh!
There was an error while loading. Please reload this page.