Remove test specific code, ensure black supports paths relative to ho…

…me (#2780)

For #2274
For #2678

news/1 Enhancements/2781.md

@@ -0,0 +1 @@
+Ensure `python.condaPath` supports paths relative to `Home`. E.g. `"python.condaPath":"~/anaconda3/bin/conda"`.

news/2 Fixes/2274.md

@@ -0,0 +1 @@
+Ensure `python.formatting.blackPath` supports paths relative to `Home`. E.g. `"python.formatting.blackPath":"~/venv/bin/black"`.

news/3 Code Health/2678.md

@@ -0,0 +1 @@
+Remove test specific code from `configSettings.ts` class.

src/client/common/configSettings.ts

@@ -57,22 +57,10 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
     // tslint:disable-next-line:variable-name
     private _pythonPath = '';
 
-    constructor(workspaceFolder?: Uri, initialize = true) {
+    constructor(workspaceFolder?: Uri) {
         super();
         this.workspaceRoot = workspaceFolder ? workspaceFolder : Uri.file(__dirname);
-        if (initialize) {
-            this.disposables.push(workspace.onDidChangeConfiguration(() => {
-                const currentConfig = workspace.getConfiguration('python', this.workspaceRoot);
-                this.update(currentConfig);
-
-                // If workspace config changes, then we could have a cascading effect of on change events.
-                // Let's defer the change notification.
-                setTimeout(() => this.emit('change'), 1);
-            }));
-
-            const initialConfig = workspace.getConfiguration('python', this.workspaceRoot);
-            this.update(initialConfig);
-        }
+        this.initialize();
     }
     // tslint:disable-next-line:function-name
     public static getInstance(resource?: Uri): PythonSettings {
@@ -128,7 +116,8 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
         // tslint:disable-next-line:no-backbone-get-set-outside-model no-non-null-assertion
         this.venvPath = systemVariables.resolveAny(pythonSettings.get<string>('venvPath'))!;
         this.venvFolders = systemVariables.resolveAny(pythonSettings.get<string[]>('venvFolders'))!;
-        this.condaPath = systemVariables.resolveAny(pythonSettings.get<string>('condaPath'))!;
+        const condaPath = systemVariables.resolveAny(pythonSettings.get<string>('condaPath'))!;
+        this.condaPath = condaPath && condaPath.length > 0 ? getAbsolutePath(condaPath, workspaceRoot) : condaPath;
 
         this.downloadLanguageServer = systemVariables.resolveAny(pythonSettings.get<boolean>('downloadLanguageServer', true))!;
         this.jediEnabled = systemVariables.resolveAny(pythonSettings.get<boolean>('jediEnabled', true))!;
@@ -240,6 +229,7 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
         };
         this.formatting.autopep8Path = getAbsolutePath(systemVariables.resolveAny(this.formatting.autopep8Path), workspaceRoot);
         this.formatting.yapfPath = getAbsolutePath(systemVariables.resolveAny(this.formatting.yapfPath), workspaceRoot);
+        this.formatting.blackPath = getAbsolutePath(systemVariables.resolveAny(this.formatting.blackPath), workspaceRoot);
 
         // tslint:disable-next-line:no-backbone-get-set-outside-model no-non-null-assertion
         const autoCompleteSettings = systemVariables.resolveAny(pythonSettings.get<IAutoCompleteSettings>('autoComplete'))!;
@@ -346,6 +336,19 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
             this._pythonPath = value;
         }
     }
+    protected initialize(): void {
+        this.disposables.push(workspace.onDidChangeConfiguration(() => {
+            const currentConfig = workspace.getConfiguration('python', this.workspaceRoot);
+            this.update(currentConfig);
+
+            // If workspace config changes, then we could have a cascading effect of on change events.
+            // Let's defer the change notification.
+            setTimeout(() => this.emit('change'), 1);
+        }));
+
+        const initialConfig = workspace.getConfiguration('python', this.workspaceRoot);
+        this.update(initialConfig);
+    }
 }
 
 function getAbsolutePath(pathToCheck: string, rootDir: string): string {

src/test/common/configSettings.unit.test.ts

@@ -4,7 +4,10 @@
 'use strict';
 
 import { expect } from 'chai';
+import * as path from 'path';
 import * as TypeMoq from 'typemoq';
+// tslint:disable-next-line:no-require-imports
+import untildify = require('untildify');
 import { WorkspaceConfiguration } from 'vscode';
 import {
     PythonSettings
@@ -19,83 +22,152 @@ import {
     IUnitTestSettings,
     IWorkspaceSymbolSettings
 } from '../../client/common/types';
+import { noop } from '../../utils/misc';
 
 // tslint:disable-next-line:max-func-body-length
 suite('Python Settings', () => {
     let config: TypeMoq.IMock<WorkspaceConfiguration>;
+    let expected: PythonSettings;
+    let settings: PythonSettings;
+    const CustomPythonSettings = class extends PythonSettings {
+        protected initialize() { noop(); }
+    };
 
     setup(() => {
         config = TypeMoq.Mock.ofType<WorkspaceConfiguration>(undefined, TypeMoq.MockBehavior.Strict);
+        expected = new CustomPythonSettings();
+        settings = new CustomPythonSettings();
     });
 
-    function initializeConfig(settings: PythonSettings) {
+    function initializeConfig(sourceSettings: PythonSettings) {
         // string settings
         for (const name of ['pythonPath', 'venvPath', 'condaPath', 'envFile']) {
             config.setup(c => c.get<string>(name))
-                .returns(() => settings[name]);
+                .returns(() => sourceSettings[name]);
         }
-        if (settings.jediEnabled) {
+        if (sourceSettings.jediEnabled) {
             config.setup(c => c.get<string>('jediPath'))
-                .returns(() => settings.jediPath);
+                .returns(() => sourceSettings.jediPath);
         }
         for (const name of ['venvFolders']) {
             config.setup(c => c.get<string[]>(name))
-                .returns(() => settings[name]);
+                .returns(() => sourceSettings[name]);
         }
 
         // boolean settings
         for (const name of ['downloadLanguageServer', 'jediEnabled', 'autoUpdateLanguageServer']) {
             config.setup(c => c.get<boolean>(name, true))
-                .returns(() => settings[name]);
+                .returns(() => sourceSettings[name]);
         }
         for (const name of ['disableInstallationCheck', 'globalModuleInstallation']) {
             config.setup(c => c.get<boolean>(name))
-                .returns(() => settings[name]);
+                .returns(() => sourceSettings[name]);
         }
 
         // number settings
-        if (settings.jediEnabled) {
+        if (sourceSettings.jediEnabled) {
             config.setup(c => c.get<number>('jediMemoryLimit'))
-                .returns(() => settings.jediMemoryLimit);
+                .returns(() => sourceSettings.jediMemoryLimit);
         }
 
         // "any" settings
         // tslint:disable-next-line:no-any
         config.setup(c => c.get<any[]>('devOptions'))
-            .returns(() => settings.devOptions);
+            .returns(() => sourceSettings.devOptions);
 
         // complex settings
         config.setup(c => c.get<ILintingSettings>('linting'))
-            .returns(() => settings.linting);
+            .returns(() => sourceSettings.linting);
         config.setup(c => c.get<IAnalysisSettings>('analysis'))
-            .returns(() => settings.analysis);
+            .returns(() => sourceSettings.analysis);
         config.setup(c => c.get<ISortImportSettings>('sortImports'))
-            .returns(() => settings.sortImports);
+            .returns(() => sourceSettings.sortImports);
         config.setup(c => c.get<IFormattingSettings>('formatting'))
-            .returns(() => settings.formatting);
+            .returns(() => sourceSettings.formatting);
         config.setup(c => c.get<IAutoCompleteSettings>('autoComplete'))
-            .returns(() => settings.autoComplete);
+            .returns(() => sourceSettings.autoComplete);
         config.setup(c => c.get<IWorkspaceSymbolSettings>('workspaceSymbols'))
-            .returns(() => settings.workspaceSymbols);
+            .returns(() => sourceSettings.workspaceSymbols);
         config.setup(c => c.get<IUnitTestSettings>('unitTest'))
-            .returns(() => settings.unitTest);
+            .returns(() => sourceSettings.unitTest);
         config.setup(c => c.get<ITerminalSettings>('terminal'))
-            .returns(() => settings.terminal);
+            .returns(() => sourceSettings.terminal);
     }
 
     test('condaPath updated', () => {
-        const expected = new PythonSettings(undefined, false);
         expected.pythonPath = 'python3';
         expected.condaPath = 'spam';
         initializeConfig(expected);
         config.setup(c => c.get<string>('condaPath'))
             .returns(() => expected.condaPath)
             .verifiable(TypeMoq.Times.once());
 
-        const settings = new PythonSettings(undefined, false);
         settings.update(config.object);
 
         expect(settings.condaPath).to.be.equal(expected.condaPath);
         config.verifyAll();
     });
+
+    test('condaPath (relative to home) updated', () => {
+        expected.pythonPath = 'python3';
+        expected.condaPath = path.join('~', 'anaconda3', 'bin', 'conda');
+        initializeConfig(expected);
+        config.setup(c => c.get<string>('condaPath'))
+            .returns(() => expected.condaPath)
+            .verifiable(TypeMoq.Times.once());
+
+        settings.update(config.object);
+
+        expect(settings.condaPath).to.be.equal(untildify(expected.condaPath));
+        config.verifyAll();
+    });
+
+    test('Formatter Paths and args', () => {
+        expected.pythonPath = 'python3';
+        // tslint:disable-next-line:no-any
+        expected.formatting = {
+            autopep8Args: ['1', '2'], autopep8Path: 'one',
+            blackArgs: ['3', '4'], blackPath: 'two',
+            yapfArgs: ['5', '6'], yapfPath: 'three',
+            provider: ''
+        };
+        expected.formatting.blackPath = 'spam';
+        initializeConfig(expected);
+        config.setup(c => c.get<IFormattingSettings>('formatting'))
+            .returns(() => expected.formatting)
+            .verifiable(TypeMoq.Times.once());
+
+        settings.update(config.object);
+
+        for (const key of Object.keys(expected.formatting)) {
+            expect(settings.formatting[key]).to.be.deep.equal(expected.formatting[key]);
+        }
+        config.verifyAll();
+    });
+    test('Formatter Paths (paths relative to home)', () => {
+        expected.pythonPath = 'python3';
+        // tslint:disable-next-line:no-any
+        expected.formatting = {
+            autopep8Args: [], autopep8Path: path.join('~', 'one'),
+            blackArgs: [], blackPath: path.join('~', 'two'),
+            yapfArgs: [], yapfPath: path.join('~', 'three'),
+            provider: ''
+        };
+        expected.formatting.blackPath = 'spam';
+        initializeConfig(expected);
+        config.setup(c => c.get<IFormattingSettings>('formatting'))
+            .returns(() => expected.formatting)
+            .verifiable(TypeMoq.Times.once());
+
+        settings.update(config.object);
+
+        for (const key of Object.keys(expected.formatting)) {
+            if (!key.endsWith('path')) {
+                continue;
+            }
+            const expectedPath = untildify(expected.formatting[key]);
+            expect(settings.formatting[key]).to.be.equal(expectedPath);
+        }
+        config.verifyAll();
+    });
 });