Removing view test ui and view test output commands (microsoft#16298)

karthiknadig · Jul 26, 2021 · afc31d7 · afc31d7
1 parent 00e6cb2
commit afc31d7
Show file tree

Hide file tree

Showing 13 changed files with 15 additions and 135 deletions.
diff --git a/src/client/common/application/commands.ts b/src/client/common/application/commands.ts
@@ -40,7 +40,6 @@ interface ICommandNameWithoutArgumentTypeMapping {
     [Commands.Exec_Selection_In_Terminal]: [];
     [Commands.Exec_Selection_In_Django_Shell]: [];
     [Commands.Create_Terminal]: [];
-    [Commands.Tests_View_UI]: [];
     [Commands.Tests_Ask_To_Stop_Discovery]: [];
     [Commands.Tests_Ask_To_Stop_Test]: [];
     [Commands.Tests_Discovering]: [];
@@ -95,7 +94,6 @@ export interface ICommandNameArgumentTypeMapping extends ICommandNameWithoutArgu
     [Commands.Sort_Imports]: [undefined, Uri];
     [Commands.Exec_In_Terminal]: [undefined, Uri];
     [Commands.Exec_In_Terminal_Icon]: [undefined, Uri];
-    [Commands.Tests_ViewOutput]: [undefined, CommandSource];
     [Commands.Tests_Select_And_Run_File]: [undefined, CommandSource];
     [Commands.Tests_Run_Current_File]: [undefined, CommandSource];
     [Commands.Tests_Stop]: [undefined, Uri];

diff --git a/src/client/common/constants.ts b/src/client/common/constants.ts
@@ -37,7 +37,6 @@ export namespace Commands {
     export const Exec_In_Terminal_Icon = 'python.execInTerminal-icon';
     export const Exec_Selection_In_Terminal = 'python.execSelectionInTerminal';
     export const Exec_Selection_In_Django_Shell = 'python.execSelectionInDjangoShell';
-    export const Tests_View_UI = 'python.viewTestUI';
     export const Tests_Picker_UI = 'python.selectTestToRun';
     export const Tests_Picker_UI_Debug = 'python.selectTestToDebug';
     export const Tests_Configure = 'python.configureTests';
@@ -94,8 +93,6 @@ export namespace Octicons {
     export const Star = '$(star)';
 }
 
-export const Button_Text_Tests_View_Output = 'View Output';
-
 export namespace Text {
     export const CodeLensRunUnitTest = 'Run Test';
     export const CodeLensDebugUnitTest = 'Debug Test';

diff --git a/src/client/testing/common/managers/baseTestManager.ts b/src/client/testing/common/managers/baseTestManager.ts
@@ -42,7 +42,6 @@ import {
     ITestManager,
     ITestNonPassingMessage,
     ITestResultsService,
-    ITestsHelper,
     ITestsStatusUpdaterService,
     TestDiscoveryOptions,
     Tests,
@@ -254,21 +253,15 @@ export abstract class BaseTestManager implements ITestManager {
                 this.updateStatus(TestStatus.Idle);
                 this.discoverTestsPromise = undefined;
 
-                // have errors in Discovering
-                let haveErrorsInDiscovering = false;
                 tests.testFiles.forEach((file) => {
                     if (file.errorsWhenDiscovering && file.errorsWhenDiscovering.length > 0) {
-                        haveErrorsInDiscovering = true;
                         this.outputChannel.append('_'.repeat(10));
                         this.outputChannel.append(`There was an error in identifying unit tests in ${file.nameToRun}`);
                         this.outputChannel.appendLine('_'.repeat(10));
                         this.outputChannel.appendLine(file.errorsWhenDiscovering);
                     }
                 });
-                if (haveErrorsInDiscovering && !quietMode) {
-                    const testsHelper = this.serviceContainer.get<ITestsHelper>(ITestsHelper);
-                    testsHelper.displayTestErrorMessage('There were some errors in discovering unit tests');
-                }
+
                 this.disposeCancellationToken(CancellationTokenType.testDiscovery);
                 sendTelemetryEvent(EventName.UNITTEST_DISCOVER, undefined, telementryProperties);
                 return tests;
@@ -384,8 +377,6 @@ export abstract class BaseTestManager implements ITestManager {
                 ) {
                     return Promise.reject<Tests>(reason);
                 }
-                const testsHelper = this.serviceContainer.get<ITestsHelper>(ITestsHelper);
-                testsHelper.displayTestErrorMessage('Errors in discovering tests, continuing with tests');
                 return {
                     rootTestFolders: [],
                     testFiles: [],

diff --git a/src/client/testing/common/testUtils.ts b/src/client/testing/common/testUtils.ts
@@ -1,10 +1,8 @@
 import { inject, injectable, named } from 'inversify';
 import * as path from 'path';
 import { Uri, workspace } from 'vscode';
-import { IApplicationShell, ICommandManager } from '../../common/application/types';
-import * as constants from '../../common/constants';
+import { IApplicationShell } from '../../common/application/types';
 import { Product } from '../../common/types';
-import { IServiceContainer } from '../../ioc/types';
 import { ITestingSettings, TestSettingsPropertyNames } from '../configuration/types';
 import { TestProvider } from '../types';
 import { TestFlatteningVisitor } from './testVisitors/flatteningVisitor';
@@ -46,15 +44,9 @@ export function convertFileToPackage(filePath: string): string {
 
 @injectable()
 export class TestsHelper implements ITestsHelper {
-    private readonly appShell: IApplicationShell;
-    private readonly commandManager: ICommandManager;
     constructor(
         @inject(ITestVisitor) @named('TestFlatteningVisitor') private readonly flatteningVisitor: TestFlatteningVisitor,
-        @inject(IServiceContainer) serviceContainer: IServiceContainer,
-    ) {
-        this.appShell = serviceContainer.get<IApplicationShell>(IApplicationShell);
-        this.commandManager = serviceContainer.get<ICommandManager>(ICommandManager);
-    }
+    ) {}
     public parseProviderName(product: UnitTestProduct): TestProvider {
         switch (product) {
             case Product.pytest:
@@ -215,17 +207,6 @@ export class TestsHelper implements ITestsHelper {
             ],
         };
     }
-    public displayTestErrorMessage(message: string) {
-        this.appShell.showErrorMessage(message, constants.Button_Text_Tests_View_Output).then((action) => {
-            if (action === constants.Button_Text_Tests_View_Output) {
-                this.commandManager.executeCommand(
-                    constants.Commands.Tests_ViewOutput,
-                    undefined,
-                    constants.CommandSource.ui,
-                );
-            }
-        });
-    }
     public mergeTests(items: Tests[]): Tests {
         return items.reduce((tests, otherTests, index) => {
             if (index === 0) {

diff --git a/src/client/testing/common/types.ts b/src/client/testing/common/types.ts
@@ -300,7 +300,6 @@ export interface ITestManagementService {
     ): Promise<void>;
     stopTests(resource: Uri): Promise<void>;
     displayStopUI(message: string): Promise<void>;
-    displayUI(cmdSource: CommandSource): Promise<void>;
     displayPickerUI(cmdSource: CommandSource, file: Uri, testFunctions: TestFunction[], debug?: boolean): Promise<void>;
     runTestsImpl(
         cmdSource: CommandSource,
@@ -314,8 +313,6 @@ export interface ITestManagementService {
     selectAndRunTestFile(cmdSource: CommandSource): Promise<void>;
 
     selectAndRunTestMethod(cmdSource: CommandSource, resource: Uri, debug?: boolean): Promise<void>;
-
-    viewOutput(cmdSource: CommandSource): void;
 }
 
 export interface ITestManagerService extends Disposable {
@@ -381,7 +378,6 @@ export interface ITestsHelper {
     getSettingsPropertyNames(product: Product): TestSettingsPropertyNames;
     flattenTestFiles(testFiles: TestFile[], workspaceFolder: string): Tests;
     placeTestFilesIntoFolders(tests: Tests, workspaceFolder: string): void;
-    displayTestErrorMessage(message: string): void;
     shouldRunAllTests(testsToRun?: TestsToRun): boolean;
     mergeTests(items: Tests[]): Tests;
 }

diff --git a/src/client/testing/display/main.ts b/src/client/testing/display/main.ts
@@ -13,7 +13,7 @@ import { IServiceContainer } from '../../ioc/types';
 import { captureTelemetry } from '../../telemetry';
 import { EventName } from '../../telemetry/constants';
 import { CANCELLATION_REASON } from '../common/constants';
-import { ITestsHelper, ITestResultDisplay, Tests } from '../common/types';
+import { ITestResultDisplay, Tests } from '../common/types';
 
 @injectable()
 export class TestResultDisplay implements ITestResultDisplay {
@@ -25,7 +25,6 @@ export class TestResultDisplay implements ITestResultDisplay {
     private progressPrefix!: string;
     private readonly didChange = new EventEmitter<void>();
     private readonly appShell: IApplicationShell;
-    private readonly testsHelper: ITestsHelper;
     private readonly cmdManager: ICommandManager;
     public get onDidChange(): Event<void> {
         return this.didChange.event;
@@ -34,7 +33,6 @@ export class TestResultDisplay implements ITestResultDisplay {
     constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) {
         this.appShell = serviceContainer.get<IApplicationShell>(IApplicationShell);
         this.statusBar = this.appShell.createStatusBarItem(StatusBarAlignment.Left);
-        this.testsHelper = serviceContainer.get<ITestsHelper>(ITestsHelper);
         this.cmdManager = serviceContainer.get<ICommandManager>(ICommandManager);
     }
     public dispose() {
@@ -106,7 +104,6 @@ export class TestResultDisplay implements ITestResultDisplay {
         }
         this.statusBar.tooltip = toolTip.length === 0 ? 'No Tests Ran' : `${toolTip.join(', ')} (Tests)`;
         this.statusBar.text = statusText.length === 0 ? 'No Tests Ran' : statusText.join(' ');
-        this.statusBar.command = constants.Commands.Tests_View_UI;
         this.didChange.fire();
         if (statusText.length === 0 && !debug) {
             this.appShell.showWarningMessage('No tests ran, please check the configuration settings for the tests.');
@@ -116,14 +113,12 @@ export class TestResultDisplay implements ITestResultDisplay {
 
     private updateTestRunWithFailure(reason: any): Promise<any> {
         this.clearProgressTicker();
-        this.statusBar.command = constants.Commands.Tests_View_UI;
         if (reason === CANCELLATION_REASON) {
             this.statusBar.text = '$(zap) Run Tests';
             this.statusBar.tooltip = 'Run Tests';
         } else {
             this.statusBar.text = '$(alert) Tests Failed';
             this.statusBar.tooltip = 'Running Tests Failed';
-            this.testsHelper.displayTestErrorMessage('There was an error in running the tests.');
         }
         return Promise.reject(reason);
     }
@@ -165,7 +160,6 @@ export class TestResultDisplay implements ITestResultDisplay {
         const haveTests = tests && tests.testFunctions.length > 0;
         this.statusBar.text = '$(zap) Run Tests';
         this.statusBar.tooltip = 'Run Tests';
-        this.statusBar.command = constants.Commands.Tests_View_UI;
         this.statusBar.show();
         if (this.didChange) {
             this.didChange.fire();

diff --git a/src/client/testing/display/picker.ts b/src/client/testing/display/picker.ts
@@ -116,7 +116,6 @@ export enum Type {
     RunFile = 4,
     RunClass = 5,
     RunMethod = 6,
-    ViewTestOutput = 7,
     Null = 8,
     SelectAndRunMethod = 9,
     DebugMethod = 10,
@@ -140,36 +139,13 @@ type TestFileItem = QuickPickItem & {
     testFile?: TestFile;
 };
 
-function getSummary(tests?: Tests) {
-    if (!tests || !tests.summary) {
-        return '';
-    }
-    const statusText: string[] = [];
-    if (tests.summary.passed > 0) {
-        statusText.push(`${constants.Octicons.Test_Pass} ${tests.summary.passed} Passed`);
-    }
-    if (tests.summary.failures > 0) {
-        statusText.push(`${constants.Octicons.Test_Fail} ${tests.summary.failures} Failed`);
-    }
-    if (tests.summary.errors > 0) {
-        const plural = tests.summary.errors === 1 ? '' : 's';
-        statusText.push(`${constants.Octicons.Test_Error} ${tests.summary.errors} Error${plural}`);
-    }
-    if (tests.summary.skipped > 0) {
-        statusText.push(`${constants.Octicons.Test_Skip} ${tests.summary.skipped} Skipped`);
-    }
-    return statusText.join(', ').trim();
-}
 function buildItems(tests?: Tests): TestItem[] {
     const items: TestItem[] = [];
     items.push({ description: '', label: 'Run All Tests', type: Type.RunAll });
     items.push({ description: '', label: 'Discover Tests', type: Type.ReDiscover });
     items.push({ description: '', label: 'Run Test Method ...', type: Type.SelectAndRunMethod });
     items.push({ description: '', label: 'Configure Tests', type: Type.Configure });
 
-    const summary = getSummary(tests);
-    items.push({ description: '', label: 'View Test Output', type: Type.ViewTestOutput, detail: summary });
-
     if (tests && tests.summary.failures > 0) {
         items.push({
             description: '',
@@ -310,9 +286,6 @@ export function onItemSelected(
         case Type.ReDiscover: {
             return commandManager.executeCommand(constants.Commands.Tests_Discover, undefined, cmdSource, wkspace);
         }
-        case Type.ViewTestOutput: {
-            return commandManager.executeCommand(constants.Commands.Tests_ViewOutput, undefined, cmdSource);
-        }
         case Type.RunFailed: {
             return commandManager.executeCommand(constants.Commands.Tests_Run_Failed, undefined, cmdSource, wkspace);
         }

diff --git a/src/client/testing/main.ts b/src/client/testing/main.ts
@@ -277,21 +277,7 @@ export class UnitTestManagementService implements ITestManagementService, Dispos
         const testDisplay = this.serviceContainer.get<ITestDisplay>(ITestDisplay);
         testDisplay.displayStopTestUI(testManager.workspaceFolder, message);
     }
-    public async displayUI(cmdSource: constants.CommandSource) {
-        const testManager = await this.getTestManager(true);
-        if (!testManager) {
-            return;
-        }
-
-        const testDisplay = this.serviceContainer.get<ITestDisplay>(ITestDisplay);
-        testDisplay.displayTestUI(cmdSource, testManager.workspaceFolder);
-    }
-    public async displayPickerUI(
-        cmdSource: constants.CommandSource,
-        file: Uri,
-        testFunctions: TestFunction[],
-        debug?: boolean,
-    ) {
+    public async displayPickerUI(cmdSource: CommandSource, file: Uri, testFunctions: TestFunction[], debug?: boolean) {
         const testManager = await this.getTestManager(true, file);
         if (!testManager) {
             return;
@@ -319,11 +305,7 @@ export class UnitTestManagementService implements ITestManagementService, Dispos
         }
         await this.runTestsImpl(cmdSource, resource, { testFunction: testFunctions }, undefined, debug);
     }
-    public viewOutput(_cmdSource: constants.CommandSource) {
-        sendTelemetryEvent(EventName.UNITTEST_VIEW_OUTPUT);
-        this.outputChannel.show();
-    }
-    public async selectAndRunTestMethod(cmdSource: constants.CommandSource, resource: Uri, debug?: boolean) {
+    public async selectAndRunTestMethod(cmdSource: CommandSource, resource: Uri, debug?: boolean) {
         const testManager = await this.getTestManager(true, resource);
         if (!testManager) {
             return;
@@ -509,9 +491,6 @@ export class UnitTestManagementService implements ITestManagementService, Dispos
                     return this.runTestsImpl(cmdSource, resource, testToRun, false, true);
                 },
             ),
-            commandManager.registerCommand(constants.Commands.Tests_View_UI, () =>
-                this.displayUI(constants.CommandSource.commandPalette),
-            ),
             commandManager.registerCommand(
                 constants.Commands.Tests_Picker_UI,
                 (
@@ -543,11 +522,6 @@ export class UnitTestManagementService implements ITestManagementService, Dispos
             commandManager.registerCommand(constants.Commands.Tests_Stop, (_, resource: Uri) =>
                 this.stopTests(resource),
             ),
-            commandManager.registerCommand(
-                constants.Commands.Tests_ViewOutput,
-                (_, cmdSource: constants.CommandSource = constants.CommandSource.commandPalette) =>
-                    this.viewOutput(cmdSource),
-            ),
             commandManager.registerCommand(constants.Commands.Tests_Ask_To_Stop_Discovery, () =>
                 this.displayStopUI('Stop discovering tests'),
             ),

diff --git a/src/test/testing/common/trackEnablement.unit.test.ts b/src/test/testing/common/trackEnablement.unit.test.ts
@@ -9,7 +9,6 @@ import { anything, instance, mock, verify, when } from 'ts-mockito';
 import { IWorkspaceService } from '../../../client/common/application/types';
 import { WorkspaceService } from '../../../client/common/application/workspace';
 import { Product } from '../../../client/common/types';
-import { ServiceContainer } from '../../../client/ioc/container';
 import { EnablementTracker } from '../../../client/testing/common/enablementTracker';
 import { TestConfigSettingsService } from '../../../client/testing/common/services/configSettingService';
 import { TestsHelper } from '../../../client/testing/common/testUtils';
@@ -28,7 +27,7 @@ suite('Unit Tests - Track Enablement', () => {
         sandbox.restore();
         workspaceService = mock(WorkspaceService);
         configService = mock(TestConfigSettingsService);
-        testsHelper = new TestsHelper(instance(mock(TestFlatteningVisitor)), instance(mock(ServiceContainer)));
+        testsHelper = new TestsHelper(instance(mock(TestFlatteningVisitor)));
     });
     teardown(() => {
         sandbox.restore();

diff --git a/src/test/testing/configuration.unit.test.ts b/src/test/testing/configuration.unit.test.ts
@@ -82,7 +82,7 @@ suite('Unit Tests - ConfigurationService', () => {
                 const flattener = typeMoq.Mock.ofType<TestFlatteningVisitor>(undefined, typeMoq.MockBehavior.Strict);
                 serviceContainer
                     .setup((c) => c.get(typeMoq.It.isValue(ITestsHelper)))
-                    .returns(() => new TestsHelper(flattener.object, serviceContainer.object));
+                    .returns(() => new TestsHelper(flattener.object));
                 testConfigService = typeMoq.Mock.ofType(
                     UnitTestConfigurationService,
                     typeMoq.MockBehavior.Loose,