diff --git a/.eslintignore b/.eslintignore index ab964706aced..bf4bd2b5097c 100644 --- a/.eslintignore +++ b/.eslintignore @@ -145,7 +145,6 @@ src/test/application/diagnostics/checks/macPythonInterpreter.unit.test.ts src/test/application/diagnostics/checks/pythonPathDeprecated.unit.test.ts src/test/application/diagnostics/checks/pythonInterpreter.unit.test.ts src/test/application/diagnostics/checks/invalidLaunchJsonDebugger.unit.test.ts -src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts src/test/application/diagnostics/checks/powerShellActivation.unit.test.ts src/test/application/diagnostics/checks/invalidPythonPathInDebugger.unit.test.ts src/test/application/diagnostics/checks/envPathVariable.unit.test.ts @@ -206,7 +205,6 @@ src/client/testing/configurationFactory.ts src/client/testing/common/debugLauncher.ts src/client/testing/common/constants.ts src/client/testing/common/testUtils.ts -src/client/testing/common/updateTestSettings.ts src/client/testing/common/socketServer.ts src/client/testing/common/runner.ts diff --git a/news/3 Code Health/14334.md b/news/3 Code Health/14334.md new file mode 100644 index 000000000000..174af1334448 --- /dev/null +++ b/news/3 Code Health/14334.md @@ -0,0 +1 @@ +Remove old settings migrator. diff --git a/src/client/testing/common/updateTestSettings.ts b/src/client/testing/common/updateTestSettings.ts deleted file mode 100644 index 294042dc4d84..000000000000 --- a/src/client/testing/common/updateTestSettings.ts +++ /dev/null @@ -1,100 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; - -import { inject, injectable } from 'inversify'; -import * as path from 'path'; -import { IExtensionActivationService } from '../../activation/types'; -import { IApplicationEnvironment, IWorkspaceService } from '../../common/application/types'; -import '../../common/extensions'; -import { IFileSystem } from '../../common/platform/types'; -import { Resource } from '../../common/types'; -import { swallowExceptions } from '../../common/utils/decorators'; -import { traceDecoratorError, traceError } from '../../logging'; - -// TODO: rename the class since it is not used just for test settings -@injectable() -export class UpdateTestSettingService implements IExtensionActivationService { - public readonly supportedWorkspaceTypes = { untrustedWorkspace: false, virtualWorkspace: false }; - constructor( - @inject(IFileSystem) private readonly fs: IFileSystem, - @inject(IApplicationEnvironment) private readonly application: IApplicationEnvironment, - @inject(IWorkspaceService) private readonly workspace: IWorkspaceService, - ) {} - public async activate(resource: Resource): Promise { - this.updateTestSettings(resource).ignoreErrors(); - } - @traceDecoratorError('Failed to update test settings') - public async updateTestSettings(resource: Resource): Promise { - const filesToBeFixed = await this.getFilesToBeFixed(resource); - await Promise.all(filesToBeFixed.map((file) => this.fixSettingInFile(file))); - } - public getSettingsFiles(resource: Resource): string[] { - const settingsFiles: string[] = []; - if (this.application.userSettingsFile) { - settingsFiles.push(this.application.userSettingsFile); - } - const workspaceFolder = this.workspace.getWorkspaceFolder(resource); - if (workspaceFolder) { - settingsFiles.push(path.join(workspaceFolder.uri.fsPath, '.vscode', 'settings.json')); - } - return settingsFiles; - } - public async getFilesToBeFixed(resource: Resource): Promise { - const files = this.getSettingsFiles(resource); - const result = await Promise.all( - files.map(async (file) => { - const needsFixing = await this.doesFileNeedToBeFixed(file); - return { file, needsFixing }; - }), - ); - return result.filter((item) => item.needsFixing).map((item) => item.file); - } - // fixLanguageServerSetting provided for tests so not all tests have to - // deal with potential whitespace changes. - @swallowExceptions('Failed to update settings.json') - public async fixSettingInFile(filePath: string): Promise { - let fileContents = await this.fs.readFile(filePath); - - const setting = new RegExp('"python\\.unitTest', 'g'); - fileContents = fileContents.replace(setting, '"python.testing'); - - const setting_pytest_enabled = new RegExp('\\.pyTestEnabled"', 'g'); - const setting_pytest_args = new RegExp('\\.pyTestArgs"', 'g'); - const setting_pytest_path = new RegExp('\\.pyTestPath"', 'g'); - fileContents = fileContents.replace(setting_pytest_enabled, '.pytestEnabled"'); - fileContents = fileContents.replace(setting_pytest_args, '.pytestArgs"'); - fileContents = fileContents.replace(setting_pytest_path, '.pytestPath"'); - - const setting_pep8_args = new RegExp('\\.(? { - try { - if (!(await this.fs.fileExists(filePath))) { - return false; - } - - const contents = await this.fs.readFile(filePath); - return ( - contents.indexOf('python.unitTest.') > 0 || - contents.indexOf('.pyTest') > 0 || - contents.indexOf('.pep8') > 0 - ); - } catch (ex) { - traceError('Failed to check if file needs to be fixed', ex); - return false; - } - } -} diff --git a/src/client/testing/serviceRegistry.ts b/src/client/testing/serviceRegistry.ts index c525d4c58abf..6a7b4b5a1640 100644 --- a/src/client/testing/serviceRegistry.ts +++ b/src/client/testing/serviceRegistry.ts @@ -16,7 +16,6 @@ import { ITestsHelper, IUnitTestSocketServer, } from './common/types'; -import { UpdateTestSettingService } from './common/updateTestSettings'; import { UnitTestConfigurationService } from './configuration'; import { TestConfigurationManagerFactory } from './configurationFactory'; import { TestingService, UnitTestManagementService } from './main'; @@ -40,7 +39,6 @@ export function registerTypes(serviceManager: IServiceManager) { ITestConfigurationManagerFactory, TestConfigurationManagerFactory, ); - serviceManager.addSingleton(IExtensionActivationService, UpdateTestSettingService); serviceManager.addSingleton(IExtensionActivationService, UnitTestManagementService); registerTestControllerTypes(serviceManager); diff --git a/src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts b/src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts deleted file mode 100644 index 138a39ac79aa..000000000000 --- a/src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts +++ /dev/null @@ -1,217 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; - -import * as assert from 'assert'; -import { expect } from 'chai'; -import * as path from 'path'; -import * as sinon from 'sinon'; -import { anyString, anything, instance, mock, verify, when } from 'ts-mockito'; -import { Uri } from 'vscode'; -import { ApplicationEnvironment } from '../../../../client/common/application/applicationEnvironment'; -import { IApplicationEnvironment, IWorkspaceService } from '../../../../client/common/application/types'; -import { WorkspaceService } from '../../../../client/common/application/workspace'; -import { PersistentState, PersistentStateFactory } from '../../../../client/common/persistentState'; -import { FileSystem } from '../../../../client/common/platform/fileSystem'; -import { IFileSystem } from '../../../../client/common/platform/types'; -import { IPersistentState } from '../../../../client/common/types'; -import { UpdateTestSettingService } from '../../../../client/testing/common/updateTestSettings'; - -suite('Application Diagnostics - Check Test Settings', () => { - let diagnosticService: UpdateTestSettingService; - let fs: IFileSystem; - let appEnv: IApplicationEnvironment; - let storage: IPersistentState; - let workspace: IWorkspaceService; - const sandbox = sinon.createSandbox(); - setup(() => { - fs = mock(FileSystem); - appEnv = mock(ApplicationEnvironment); - storage = mock(PersistentState) as IPersistentState; - workspace = mock(WorkspaceService); - const stateFactory = mock(PersistentStateFactory); - - when(stateFactory.createGlobalPersistentState('python.unitTest.Settings', anything())).thenReturn( - instance(storage), - ); - - diagnosticService = new UpdateTestSettingService(instance(fs), instance(appEnv), instance(workspace)); - }); - teardown(() => { - sandbox.restore(); - }); - [Uri.file(__filename), undefined].forEach((resource) => { - const resourceTitle = resource ? '(with a resource)' : '(without a resource)'; - - test(`activate method invokes UpdateTestSettings ${resourceTitle}`, async () => { - const updateTestSettings = sandbox.stub(UpdateTestSettingService.prototype, 'updateTestSettings'); - updateTestSettings.resolves(); - diagnosticService = new UpdateTestSettingService(instance(fs), instance(appEnv), instance(workspace)); - - await diagnosticService.activate(resource); - - assert.ok(updateTestSettings.calledOnce); - }); - - test(`activate method invokes UpdateTestSettings and ignores errors raised by UpdateTestSettings ${resourceTitle}`, async () => { - const updateTestSettings = sandbox.stub(UpdateTestSettingService.prototype, 'updateTestSettings'); - updateTestSettings.rejects(new Error('Kaboom')); - diagnosticService = new UpdateTestSettingService(instance(fs), instance(appEnv), instance(workspace)); - - await diagnosticService.activate(resource); - - assert.ok(updateTestSettings.calledOnce); - }); - - test(`When there are no workspaces, then return just the user settings file ${resourceTitle}`, async () => { - when(workspace.getWorkspaceFolder(anything())).thenReturn(); - when(appEnv.userSettingsFile).thenReturn('user.json'); - - const files = diagnosticService.getSettingsFiles(resource); - - assert.deepEqual(files, ['user.json']); - verify(workspace.getWorkspaceFolder(resource)).once(); - }); - test(`When there are no workspaces & no user file, then return an empty array ${resourceTitle}`, async () => { - when(workspace.getWorkspaceFolder(anything())).thenReturn(); - when(appEnv.userSettingsFile).thenReturn(); - - const files = diagnosticService.getSettingsFiles(resource); - - assert.deepEqual(files, []); - verify(workspace.getWorkspaceFolder(resource)).once(); - }); - test(`When there is a workspace folder, then return the user settings file & workspace file ${resourceTitle}`, async function () { - if (!resource) { - return this.skip(); - } - when(workspace.getWorkspaceFolder(resource)).thenReturn({ name: '1', uri: Uri.file('folder1'), index: 0 }); - when(appEnv.userSettingsFile).thenReturn('user.json'); - - const files = diagnosticService.getSettingsFiles(resource); - - assert.deepEqual(files, ['user.json', path.join(Uri.file('folder1').fsPath, '.vscode', 'settings.json')]); - verify(workspace.getWorkspaceFolder(resource)).once(); - }); - test(`When there is a workspace folder & no user file, then workspace file ${resourceTitle}`, async function () { - if (!resource) { - return this.skip(); - } - when(workspace.getWorkspaceFolder(resource)).thenReturn({ name: '1', uri: Uri.file('folder1'), index: 0 }); - when(appEnv.userSettingsFile).thenReturn(); - - const files = diagnosticService.getSettingsFiles(resource); - - assert.deepEqual(files, [path.join(Uri.file('folder1').fsPath, '.vscode', 'settings.json')]); - verify(workspace.getWorkspaceFolder(resource)).once(); - }); - test(`Return an empty array if there are no files ${resourceTitle}`, async () => { - const getSettingsFiles = sandbox.stub(UpdateTestSettingService.prototype, 'getSettingsFiles'); - getSettingsFiles.returns([]); - diagnosticService = new UpdateTestSettingService(instance(fs), instance(appEnv), instance(workspace)); - - const files = await diagnosticService.getFilesToBeFixed(resource); - - expect(files).to.deep.equal([]); - }); - test(`Filter files based on whether they need to be fixed ${resourceTitle}`, async () => { - const getSettingsFiles = sandbox.stub(UpdateTestSettingService.prototype, 'getSettingsFiles'); - const filterFiles = sandbox.stub(UpdateTestSettingService.prototype, 'doesFileNeedToBeFixed'); - filterFiles.callsFake((file) => Promise.resolve(file === 'file_a' || file === 'file_c')); - getSettingsFiles.returns(['file_a', 'file_b', 'file_c', 'file_d']); - - diagnosticService = new UpdateTestSettingService(instance(fs), instance(appEnv), instance(workspace)); - - const files = await diagnosticService.getFilesToBeFixed(resource); - - expect(files).to.deep.equal(['file_a', 'file_c']); - }); - }); - [ - { - testTitle: 'Should fix file if contents contains python.unitTest.', - expectedValue: true, - contents: '{"python.pythonPath":"1234", "python.unitTest.unitTestArgs":[]}', - }, - { - testTitle: 'Should fix file if contents contains python.pyTest.', - expectedValue: true, - contents: '{"python.pythonPath":"1234", "python.pyTestArgs":[]}', - }, - { - testTitle: 'Should fix file if contents contains python.pyTest. & python.unitTest.', - expectedValue: true, - contents: '{"python.pythonPath":"1234", "python.testing.pyTestArgs":[], "python.unitTest.unitTestArgs":[]}', - }, - { - testTitle: 'Should not fix file if contents does not contain python.unitTest. and python.pyTest', - expectedValue: false, - contents: '{"python.pythonPath":"1234", "python.unittest.unitTestArgs":[]}', - }, - ].forEach((item) => { - test(item.testTitle, async () => { - when(fs.fileExists(__filename)).thenResolve(true); - when(fs.readFile(__filename)).thenResolve(item.contents); - - const needsToBeFixed = await diagnosticService.doesFileNeedToBeFixed(__filename); - - expect(needsToBeFixed).to.equal(item.expectedValue); - verify(fs.readFile(__filename)).once(); - }); - }); - test("File should not be fixed if there's an error in reading the file", async () => { - when(fs.fileExists(__filename)).thenResolve(true); - when(fs.readFile(__filename)).thenReject(new Error('Kaboom')); - - const needsToBeFixed = await diagnosticService.doesFileNeedToBeFixed(__filename); - - assert.ok(!needsToBeFixed); - verify(fs.readFile(__filename)).once(); - }); - test('File should not be fixed if file does not exist', async () => { - const needsToBeFixed = await diagnosticService.doesFileNeedToBeFixed(__filename + 'doesnotexist'); - assert.ok(!needsToBeFixed); - }); - - [ - { - testTitle: 'Should replace python.unitTest.', - contents: '{"python.pythonPath":"1234", "python.unitTest.unitTestArgs":[]}', - expectedContents: '{"python.pythonPath":"1234", "python.testing.unitTestArgs":[]}', - }, - { - testTitle: 'Should replace python.unitTest.pyTest.', - contents: - '{"python.pythonPath":"1234", "python.unitTest.pyTestArgs":[], "python.unitTest.pyTestArgs":[], "python.unitTest.pyTestPath":[]}', - expectedContents: - '{"python.pythonPath":"1234", "python.testing.pytestArgs":[], "python.testing.pytestArgs":[], "python.testing.pytestPath":[]}', - }, - { - testTitle: 'Should replace python.testing.pyTest.', - contents: - '{"python.pythonPath":"1234", "python.testing.pyTestArgs":[], "python.testing.pyTestArgs":[], "python.testing.pyTestPath":[]}', - expectedContents: - '{"python.pythonPath":"1234", "python.testing.pytestArgs":[], "python.testing.pytestArgs":[], "python.testing.pytestPath":[]}', - }, - { - testTitle: 'Should not make any changes to the file', - contents: - '{"python.pythonPath":"1234", "python.unittest.unitTestArgs":[], "python.unitTest.pytestArgs":[], "python.testing.pytestArgs":[], "python.testing.pytestPath":[]}', - expectedContents: - '{"python.pythonPath":"1234", "python.unittest.unitTestArgs":[], "python.testing.pytestArgs":[], "python.testing.pytestArgs":[], "python.testing.pytestPath":[]}', - }, - ].forEach((item) => { - test(item.testTitle, async () => { - when(fs.fileExists(__filename)).thenResolve(true); - when(fs.readFile(__filename)).thenResolve(item.contents); - when(fs.writeFile(__filename, anything())).thenResolve(); - - const actualContent = await diagnosticService.fixSettingInFile(__filename); - - verify(fs.readFile(__filename)).once(); - verify(fs.writeFile(__filename, anyString())).once(); - expect(actualContent).to.be.equal(item.expectedContents); - }); - }); -});