diff --git a/news/2 Fixes/12429.md b/news/2 Fixes/12429.md new file mode 100644 index 000000000000..4fea2a4f8f5c --- /dev/null +++ b/news/2 Fixes/12429.md @@ -0,0 +1 @@ +Fixed issue when `python.jediEnabled` setting was not removed and `python.languageServer` setting was not updated. diff --git a/src/client/testing/common/updateTestSettings.ts b/src/client/testing/common/updateTestSettings.ts index 1f5c79faf32c..b7b60aad8ad8 100644 --- a/src/client/testing/common/updateTestSettings.ts +++ b/src/client/testing/common/updateTestSettings.ts @@ -31,7 +31,7 @@ export class UpdateTestSettingService implements IExtensionActivationService { const filesToBeFixed = await this.getFilesToBeFixed(resource); await Promise.all(filesToBeFixed.map((file) => this.fixSettingInFile(file))); } - public getSettingsFiles(resource: Resource) { + public getSettingsFiles(resource: Resource): string[] { const settingsFiles: string[] = []; if (this.application.userSettingsFile) { settingsFiles.push(this.application.userSettingsFile); @@ -42,7 +42,7 @@ export class UpdateTestSettingService implements IExtensionActivationService { } return settingsFiles; } - public async getFilesToBeFixed(resource: Resource) { + public async getFilesToBeFixed(resource: Resource): Promise { const files = this.getSettingsFiles(resource); const result = await Promise.all( files.map(async (file) => { @@ -87,10 +87,11 @@ export class UpdateTestSettingService implements IExtensionActivationService { return fileContents; } - public async doesFileNeedToBeFixed(filePath: string) { + public async doesFileNeedToBeFixed(filePath: string): Promise { try { const contents = await this.fs.readFile(filePath); return ( + contents.indexOf('python.jediEnabled') > 0 || contents.indexOf('python.unitTest.') > 0 || contents.indexOf('.pyTest') > 0 || contents.indexOf('.pep8') > 0 @@ -106,13 +107,13 @@ export class UpdateTestSettingService implements IExtensionActivationService { // - `true` or missing then set to `languageServer: Jedi`. // - `false` and `languageServer` is present, do nothing. // - `false` and `languageServer` is NOT present, set `languageServer` to `Microsoft`. - // `jediEnabled` is then removed. + // `jediEnabled` is NOT removed since JSONC parser may also remove comments. const jediEnabledPath = ['python.jediEnabled']; const languageServerPath = ['python.languageServer']; try { - let ast = parseTree(fileContent); - let jediEnabledNode = findNodeAtLocation(ast, jediEnabledPath); + const ast = parseTree(fileContent); + const jediEnabledNode = findNodeAtLocation(ast, jediEnabledPath); const jediEnabled = jediEnabledNode ? getNodeValue(jediEnabledNode) : true; const languageServerNode = findNodeAtLocation(ast, languageServerPath); const formattingOptions: FormattingOptions = { @@ -134,13 +135,6 @@ export class UpdateTestSettingService implements IExtensionActivationService { } fileContent = applyEdits(fileContent, edits); - // Remove jediEnabled - ast = parseTree(fileContent); - jediEnabledNode = findNodeAtLocation(ast, jediEnabledPath); - if (jediEnabledNode) { - edits = modify(fileContent, jediEnabledPath, undefined, { formattingOptions }); - fileContent = applyEdits(fileContent, edits); - } // tslint:disable-next-line:no-empty } catch {} return fileContent; diff --git a/src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts b/src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts index 819b27b9f8cb..d70dd05728ef 100644 --- a/src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts +++ b/src/test/application/diagnostics/checks/updateTestSettings.unit.test.ts @@ -168,6 +168,12 @@ suite('Application Diagnostics - Check Test Settings', () => { assert.ok(!needsToBeFixed); verify(fs.readFile(__filename)).once(); }); + test('Verify `python.jediEnabled` is found in user settings', async () => { + when(fs.readFile(__filename)).thenResolve('"python.jediEnabled": false'); + const needsToBeFixed = await diagnosticService.doesFileNeedToBeFixed(__filename); + assert.ok(needsToBeFixed); + verify(fs.readFile(__filename)).once(); + }); [ { @@ -218,32 +224,32 @@ suite('Application Diagnostics - Check Test Settings', () => { { testTitle: 'jediEnabled: true, no languageServer setting', contents: '{ "python.jediEnabled": true }', - expectedContent: '{"python.languageServer": "Jedi"}' + expectedContent: '{ "python.jediEnabled": true, "python.languageServer": "Jedi"}' }, { testTitle: 'jediEnabled: true, languageServer setting present', contents: '{ "python.jediEnabled": true }', - expectedContent: '{"python.languageServer": "Jedi"}' + expectedContent: '{ "python.jediEnabled": true, "python.languageServer": "Jedi"}' }, { testTitle: 'jediEnabled: false, no languageServer setting', contents: '{ "python.jediEnabled": false }', - expectedContent: '{"python.languageServer": "Microsoft"}' + expectedContent: '{ "python.jediEnabled": false, "python.languageServer": "Microsoft"}' }, { testTitle: 'jediEnabled: false, languageServer is Microsoft', contents: '{ "python.jediEnabled": false, "python.languageServer": "Microsoft" }', - expectedContent: '{"python.languageServer": "Microsoft"}' + expectedContent: '{ "python.jediEnabled": false, "python.languageServer": "Microsoft"}' }, { testTitle: 'jediEnabled: false, languageServer is None', contents: '{ "python.jediEnabled": false, "python.languageServer": "None" }', - expectedContent: '{"python.languageServer": "None"}' + expectedContent: '{ "python.jediEnabled": false, "python.languageServer": "None"}' }, { testTitle: 'jediEnabled: false, languageServer is Jedi', contents: '{ "python.jediEnabled": false, "python.languageServer": "Jedi" }', - expectedContent: '{"python.languageServer": "Jedi"}' + expectedContent: '{ "python.jediEnabled": false, "python.languageServer": "Jedi"}' } ].forEach((item) => { test(item.testTitle, async () => {