From 128d3798571afdb977ef4ad246db20a30449455d Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 7 Aug 2019 15:50:45 -0700 Subject: [PATCH 01/16] Added tests --- src/client/linters/pylint.ts | 2 +- .../applicationDiagnostics.unit.test.ts | 4 +- src/test/linters/pylint.unit.test.ts | 621 ++++++++++++++++++ 3 files changed, 624 insertions(+), 3 deletions(-) create mode 100644 src/test/linters/pylint.unit.test.ts diff --git a/src/client/linters/pylint.ts b/src/client/linters/pylint.ts index 250b8a182647..7b8aa79f3253 100644 --- a/src/client/linters/pylint.ts +++ b/src/client/linters/pylint.ts @@ -76,7 +76,7 @@ export class Pylint extends BaseLinter { ]; const messages = await this.run(minArgs.concat(args), document, cancellation, REGEX); messages.forEach(msg => { - msg.severity = this.parseMessagesSeverity(msg.type, this.pythonSettings.linting.pylintCategorySeverity); + msg.severity = this.parseMessagesSeverity(msg.type, settings.linting.pylintCategorySeverity); }); return messages; diff --git a/src/test/application/diagnostics/applicationDiagnostics.unit.test.ts b/src/test/application/diagnostics/applicationDiagnostics.unit.test.ts index dba79fc92019..d78db406b1fa 100644 --- a/src/test/application/diagnostics/applicationDiagnostics.unit.test.ts +++ b/src/test/application/diagnostics/applicationDiagnostics.unit.test.ts @@ -34,8 +34,8 @@ suite('Application Diagnostics - ApplicationDiagnostics', () => { const oldValueOfVSC_PYTHON_CI_TEST = process.env.VSC_PYTHON_CI_TEST; setup(() => { - process.env.VSC_PYTHON_UNIT_TEST = undefined; - process.env.VSC_PYTHON_CI_TEST = undefined; + delete process.env.VSC_PYTHON_UNIT_TEST; + delete process.env.VSC_PYTHON_CI_TEST; serviceContainer = typemoq.Mock.ofType(); envHealthCheck = typemoq.Mock.ofType(); envHealthCheck.setup(service => service.runInBackground).returns(() => true); diff --git a/src/test/linters/pylint.unit.test.ts b/src/test/linters/pylint.unit.test.ts new file mode 100644 index 000000000000..728cb45365fd --- /dev/null +++ b/src/test/linters/pylint.unit.test.ts @@ -0,0 +1,621 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +// tslint:disable:no-any +// tslint:disable: max-classes-per-file + +import { assert, expect } from 'chai'; +import * as os from 'os'; +import * as path from 'path'; +import * as sinon from 'sinon'; +import { mock } from 'ts-mockito'; +import * as TypeMoq from 'typemoq'; +import * as vscode from 'vscode'; +import { IWorkspaceService } from '../../client/common/application/types'; +import { IFileSystem, IPlatformService } from '../../client/common/platform/types'; +import { IConfigurationService, IOutputChannel } from '../../client/common/types'; +import { IServiceContainer } from '../../client/ioc/types'; +import { Pylint } from '../../client/linters/pylint'; +import { ILinterInfo, ILinterManager, ILintMessage, LintMessageSeverity } from '../../client/linters/types'; + +// tslint:disable-next-line:max-func-body-length +suite('Pylint - Function hasConfigurationFile()', () => { + const folder = '/user/a/b/c/d'; + const oldValueOfPYLINTRC = process.env.PYLINTRC; + const pylintrcFiles = ['pylintrc', '.pylintrc']; + const pylintrc = 'pylintrc'; + const dotPylintrc = '.pylintrc'; + let fileSystem: TypeMoq.IMock; + let platformService: TypeMoq.IMock; + let serviceContainer: TypeMoq.IMock; + + setup(() => { + serviceContainer = TypeMoq.Mock.ofType(); + serviceContainer + .setup(c => c.get(TypeMoq.It.isValue(IFileSystem))) + .returns(() => fileSystem.object); + serviceContainer + .setup(c => c.get(TypeMoq.It.isValue(IPlatformService))) + .returns(() => platformService.object); + fileSystem = TypeMoq.Mock.ofType(); + fileSystem + .setup(x => x.arePathsSame(TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString())) + .returns((a, b) => a === b); + + platformService = TypeMoq.Mock.ofType(); + }); + + teardown(() => { + if (oldValueOfPYLINTRC === undefined) { + delete process.env.PYLINTRC; + } else { + process.env.PYLINTRC = oldValueOfPYLINTRC; + } + }); + + pylintrcFiles.forEach(pylintrcFile => { + test(`If ${pylintrcFile} exists in the current working directory, return true`, async () => { + fileSystem + .setup(x => x.fileExists(path.join(folder, pylintrc))) + .returns(() => Promise.resolve(pylintrc === pylintrcFile)) + .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join(folder, pylintrcFile))) + .returns(() => Promise.resolve(dotPylintrc === pylintrcFile)) + .verifiable(TypeMoq.Times.once()); + const hasConfig = await Pylint.hasConfigurationFile(fileSystem.object, folder, platformService.object); + expect(hasConfig).to.equal(true, 'Should return true'); + fileSystem.verifyAll(); + }); + + test(`If the current working directory is in a Python module, Pylint searches up the hierarchy of Python modules until it finds a ${pylintrcFile} file. And if ${pylintrcFile} exists, return true`, async () => { + const basePath = '/user/a/b/c/d'; + fileSystem + .setup(x => x.fileExists(path.join('/user/a/b/c/d', '__init__.py'))) + .returns(() => Promise.resolve(true)) + .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join('/user/a/b/c/d', pylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.atLeastOnce()); + fileSystem + .setup(x => x.fileExists(path.join('/user/a/b/c/d', dotPylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.atLeastOnce()); + + fileSystem + .setup(x => x.fileExists(path.join('/user/a/b/c', '__init__.py'))) + .returns(() => Promise.resolve(true)) + .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join('/user/a/b/c', pylintrc))) + .returns(() => Promise.resolve(pylintrc === pylintrcFile)); + fileSystem + .setup(x => x.fileExists(path.join('/user/a/b/c', dotPylintrc))) + .returns(() => Promise.resolve(dotPylintrc === pylintrcFile)); + const hasConfig = await Pylint.hasConfigurationFile(fileSystem.object, basePath, platformService.object); + expect(hasConfig).to.equal(true, 'Should return true'); + fileSystem.verifyAll(); + platformService.verifyAll(); + }); + + test(`If ${pylintrcFile} exists in the home directory, return true`, async () => { + const home = os.homedir(); + fileSystem + .setup(x => x.fileExists(path.join(folder, pylintrc))) + .returns(() => Promise.resolve(false)); + fileSystem + .setup(x => x.fileExists(path.join(folder, dotPylintrc))) + .returns(() => Promise.resolve(false)); + fileSystem + .setup(x => x.fileExists(path.join(folder, '__init__.py'))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join(home, '.config', pylintrc))) + .returns(() => Promise.resolve(pylintrc === pylintrcFile)); + fileSystem + .setup(x => x.fileExists(path.join(home, dotPylintrc))) + .returns(() => Promise.resolve(dotPylintrc === pylintrcFile)); + const hasConfig = await Pylint.hasConfigurationFile(fileSystem.object, folder, platformService.object); + expect(hasConfig).to.equal(true, 'Should return true'); + fileSystem.verifyAll(); + platformService.verifyAll(); + }); + }); + + test('If /etc/pylintrc exists in non-Windows platform, return true', async () => { + const home = os.homedir(); + fileSystem + .setup(x => x.fileExists(path.join(folder, pylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join(folder, dotPylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join(folder, '__init__.py'))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join(home, '.config', pylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join(home, dotPylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + platformService + .setup(x => x.isWindows) + .returns(() => false); + fileSystem + .setup(x => x.fileExists(path.join('/etc', pylintrc))) + .returns(() => Promise.resolve(true)); + const hasConfig = await Pylint.hasConfigurationFile(fileSystem.object, folder, platformService.object); + expect(hasConfig).to.equal(true, 'Should return true'); + fileSystem.verifyAll(); + platformService.verifyAll(); + }); + + test('If none of pylintrc configuration files does not exist anywhere, return false', async () => { + const home = os.homedir(); + fileSystem + .setup(x => x.fileExists(path.join(folder, pylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join(folder, dotPylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join(folder, '__init__.py'))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join(home, '.config', pylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join(home, dotPylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + platformService + .setup(x => x.isWindows) + .returns(() => false) + .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join('/etc', pylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + const hasConfig = await Pylint.hasConfigurationFile(fileSystem.object, folder, platformService.object); + expect(hasConfig).to.equal(false, 'Should return false'); + fileSystem.verifyAll(); + platformService.verifyAll(); + }); + + test('If process env PYLINTRC contains path to pylintrc, return true', async () => { + process.env.PYLINTRC = 'path/to/pylintrc'; + const hasConfig = await Pylint.hasConfigurationFile(fileSystem.object, folder, platformService.object); + expect(hasConfig).to.equal(true, 'Should return true'); + }); +}); + +// tslint:disable-next-line:max-func-body-length +suite('Pylint - Function hasConfigrationFileInWorkspace()', () => { + const pylintrc = 'pylintrc'; + const dotPylintrc = '.pylintrc'; + let fileSystem: TypeMoq.IMock; + let platformService: TypeMoq.IMock; + let serviceContainer: TypeMoq.IMock; + + setup(() => { + serviceContainer = TypeMoq.Mock.ofType(); + serviceContainer + .setup(c => c.get(TypeMoq.It.isValue(IFileSystem))) + .returns(() => fileSystem.object); + serviceContainer + .setup(c => c.get(TypeMoq.It.isValue(IPlatformService))) + .returns(() => platformService.object); + fileSystem = TypeMoq.Mock.ofType(); + fileSystem + .setup(x => x.arePathsSame(TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString())) + .returns((a, b) => a === b); + + platformService = TypeMoq.Mock.ofType(); + }); + + test('If none of pylint rc files exist upto the workspace root, return false', async () => { + const folder = '/user/a/b/c'; + const root = '/user/a'; + + fileSystem + .setup(x => x.fileExists(path.join('/user/a/b/c', pylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join('/user/a/b/c', dotPylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + + fileSystem + .setup(x => x.fileExists(path.join('/user/a/b', pylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join('/user/a/b', dotPylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + + fileSystem + .setup(x => x.fileExists(path.join('/user/a', pylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.never()); + fileSystem + .setup(x => x.fileExists(path.join('/user/a', dotPylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.never()); + + fileSystem + .setup(x => x.fileExists(path.join('/user', dotPylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.never()); + fileSystem + .setup(x => x.fileExists(path.join('/user', dotPylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.never()); + + const hasConfig = await Pylint.hasConfigrationFileInWorkspace(fileSystem.object, folder, root); + expect(hasConfig).to.equal(false, 'Should return false'); + fileSystem.verifyAll(); + }); + + test('If pylintrc file exist while going upto the workspace root, return true', async () => { + const folder = '/user/a/b/c'; + const root = '/user/a'; + + fileSystem + .setup(x => x.fileExists(path.join('/user/a/b/c', pylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join('/user/a/b/c', dotPylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + + fileSystem + .setup(x => x.fileExists(path.join('/user/a/b', pylintrc))) + // pylintrc file exists + .returns(() => Promise.resolve(true)) + .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join('/user/a/b', dotPylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.never()); + + fileSystem + .setup(x => x.fileExists(path.join('/user/a', pylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.never()); + fileSystem + .setup(x => x.fileExists(path.join('/user/a', dotPylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.never()); + + fileSystem + .setup(x => x.fileExists(path.join('/user', dotPylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.never()); + fileSystem + .setup(x => x.fileExists(path.join('/user', dotPylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.never()); + + const hasConfig = await Pylint.hasConfigrationFileInWorkspace(fileSystem.object, folder, root); + expect(hasConfig).to.equal(true, 'Should return true'); + fileSystem.verifyAll(); + }); + + test('If dotpylintrc file exist while going upto the workspace root, return true', async () => { + const folder = '/user/a/b/c'; + const root = '/user/a'; + + fileSystem + .setup(x => x.fileExists(path.join('/user/a/b/c', pylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join('/user/a/b/c', dotPylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + + fileSystem + .setup(x => x.fileExists(path.join('/user/a/b', pylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join('/user/a/b', dotPylintrc))) + // .pylintrc file exists + .returns(() => Promise.resolve(true)) + .verifiable(TypeMoq.Times.once()); + + fileSystem + .setup(x => x.fileExists(path.join('/user/a', pylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.never()); + fileSystem + .setup(x => x.fileExists(path.join('/user/a', dotPylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.never()); + + fileSystem + .setup(x => x.fileExists(path.join('/user', dotPylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.never()); + fileSystem + .setup(x => x.fileExists(path.join('/user', dotPylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.never()); + + const hasConfig = await Pylint.hasConfigrationFileInWorkspace(fileSystem.object, folder, root); + expect(hasConfig).to.equal(true, 'Should return true'); + fileSystem.verifyAll(); + }); +}); + +// tslint:disable-next-line:max-func-body-length +suite('Pylint - Function runLinter()', () => { + let fileSystem: TypeMoq.IMock; + let serviceContainer: TypeMoq.IMock; + let workspaceService: TypeMoq.IMock; + let configService: TypeMoq.IMock; + let manager: TypeMoq.IMock; + let output: TypeMoq.IMock; + let _info: TypeMoq.IMock; + let platformService: TypeMoq.IMock; + let run: sinon.SinonStub; + let parseMessagesSeverity: sinon.SinonStub; + const minArgs = [ + '--disable=all', + '--enable=F' + + ',unreachable,duplicate-key,unnecessary-semicolon' + + ',global-variable-not-assigned,unused-variable' + + ',unused-wildcard-import,binary-op-exception' + + ',bad-format-string,anomalous-backslash-in-string' + + ',bad-open-mode' + + ',E0001,E0011,E0012,E0100,E0101,E0102,E0103,E0104,E0105,E0107' + + ',E0108,E0110,E0111,E0112,E0113,E0114,E0115,E0116,E0117,E0118' + + ',E0202,E0203,E0211,E0213,E0236,E0237,E0238,E0239,E0240,E0241' + + ',E0301,E0302,E0303,E0401,E0402,E0601,E0602,E0603,E0604,E0611' + + ',E0632,E0633,E0701,E0702,E0703,E0704,E0710,E0711,E0712,E1003' + + ',E1101,E1102,E1111,E1120,E1121,E1123,E1124,E1125,E1126,E1127' + + ',E1128,E1129,E1130,E1131,E1132,E1133,E1134,E1135,E1136,E1137' + + ',E1138,E1139,E1200,E1201,E1205,E1206,E1300,E1301,E1302,E1303' + + ',E1304,E1305,E1306,E1310,E1700,E1701' + ]; + const doc = { + uri: vscode.Uri.file('path/to/doc') + }; + const args = [ + '--msg-template=\'{line},{column},{category},{symbol}:{msg}\'', + '--reports=n', + '--output-format=text', + doc.uri.fsPath + ]; + const original_hasConfigrationFileInWorkspace = Pylint.hasConfigrationFileInWorkspace; + const original_hasConfigurationFile = Pylint.hasConfigurationFile; + + class PylintTest extends Pylint { + public async run(_args: string[], _document: vscode.TextDocument, _cancellation: vscode.CancellationToken, _regEx: string): Promise { + return []; + } + public parseMessagesSeverity(_error: string, _categorySeverity: any): LintMessageSeverity { + return 'Severity' as any; + } + public get info(): ILinterInfo { + return _info.object; + } + // tslint:disable-next-line: no-unnecessary-override + public async runLinter(document: vscode.TextDocument, cancellation: vscode.CancellationToken): Promise { + return super.runLinter(document, cancellation); + } + public getWorkspaceRootPath(_document: vscode.TextDocument): string { + return 'path/to/workspaceRoot'; + } + } + + setup(() => { + platformService = TypeMoq.Mock.ofType(); + _info = TypeMoq.Mock.ofType(); + output = TypeMoq.Mock.ofType(); + serviceContainer = TypeMoq.Mock.ofType(); + workspaceService = TypeMoq.Mock.ofType(); + configService = TypeMoq.Mock.ofType(); + manager = TypeMoq.Mock.ofType(); + serviceContainer + .setup(c => c.get(TypeMoq.It.isValue(ILinterManager))) + .returns(() => manager.object); + serviceContainer + .setup(c => c.get(TypeMoq.It.isValue(IConfigurationService))) + .returns(() => configService.object); + serviceContainer + .setup(c => c.get(TypeMoq.It.isValue(IWorkspaceService))) + .returns(() => workspaceService.object); + serviceContainer + .setup(c => c.get(TypeMoq.It.isValue(IFileSystem))) + .returns(() => fileSystem.object); + serviceContainer + .setup(c => c.get(TypeMoq.It.isValue(IPlatformService))) + .returns(() => platformService.object); + fileSystem = TypeMoq.Mock.ofType(); + fileSystem + .setup(x => x.arePathsSame(TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString())) + .returns((a, b) => a === b); + manager + .setup(m => m.getLinterInfo(TypeMoq.It.isAny())) + .returns(() => undefined as any); + }); + + teardown(() => { + Pylint.hasConfigrationFileInWorkspace = original_hasConfigrationFileInWorkspace; + Pylint.hasConfigurationFile = original_hasConfigurationFile; + sinon.restore(); + }); + + test('Use minimal checkers if a) setting to use minimal checkers is true, b) there are no custom arguments and c) there is no pylintrc file next to the file or at the workspace root and above', async () => { + const settings = { + linting: { + pylintUseMinimalCheckers: true + } + }; + configService + .setup(c => c.getSettings(doc.uri)) + .returns(() => settings as any); + _info + .setup(info => info.linterArgs(doc.uri)) + .returns(() => []); + Pylint.hasConfigrationFileInWorkspace = () => Promise.resolve(false); + Pylint.hasConfigurationFile = () => Promise.resolve(false); + run = sinon.stub(PylintTest.prototype, 'run'); + run.callsFake(() => Promise.resolve([])); + parseMessagesSeverity = sinon.stub(PylintTest.prototype, 'parseMessagesSeverity'); + parseMessagesSeverity.callsFake(() => 'Severity'); + const pylint = new PylintTest(output.object, serviceContainer.object); + await pylint.runLinter(doc as any, mock(vscode.CancellationTokenSource).token); + assert.deepEqual(run.args[0][0], minArgs.concat(args)); + assert.ok(parseMessagesSeverity.notCalled); + assert.ok(run.calledOnce); + }); + + test('Use minimal checkers if setting to use minimal checkers is false', async () => { + const settings = { + linting: { + pylintUseMinimalCheckers: false + } + }; + configService + .setup(c => c.getSettings(doc.uri)) + .returns(() => settings as any); + _info + .setup(info => info.linterArgs(doc.uri)) + .returns(() => []); + Pylint.hasConfigrationFileInWorkspace = () => Promise.resolve(false); + Pylint.hasConfigurationFile = () => Promise.resolve(false); + run = sinon.stub(PylintTest.prototype, 'run'); + run.callsFake(() => Promise.resolve([])); + parseMessagesSeverity = sinon.stub(PylintTest.prototype, 'parseMessagesSeverity'); + parseMessagesSeverity.callsFake(() => 'Severity'); + const pylint = new PylintTest(output.object, serviceContainer.object); + await pylint.runLinter(doc as any, mock(vscode.CancellationTokenSource).token); + assert.deepEqual(run.args[0][0], args); + assert.ok(parseMessagesSeverity.notCalled); + assert.ok(run.calledOnce); + }); + + test('Do not use minimal checkers if there are custom arguments', async () => { + const settings = { + linting: { + pylintUseMinimalCheckers: true + } + }; + configService + .setup(c => c.getSettings(doc.uri)) + .returns(() => settings as any); + _info + .setup(info => info.linterArgs(doc.uri)) + .returns(() => ['customArg1', 'customArg2']); + Pylint.hasConfigrationFileInWorkspace = () => Promise.resolve(false); + Pylint.hasConfigurationFile = () => Promise.resolve(false); + run = sinon.stub(PylintTest.prototype, 'run'); + run.callsFake(() => Promise.resolve([])); + parseMessagesSeverity = sinon.stub(PylintTest.prototype, 'parseMessagesSeverity'); + parseMessagesSeverity.callsFake(() => 'Severity'); + const pylint = new PylintTest(output.object, serviceContainer.object); + await pylint.runLinter(doc as any, mock(vscode.CancellationTokenSource).token); + assert.deepEqual(run.args[0][0], args); + assert.ok(parseMessagesSeverity.notCalled); + assert.ok(run.calledOnce); + }); + + test('Do not use minimal checkers if there is pylintrc file next to the file or above up to and including the workspace root (Method hasConfigrationFileInWorkspace() returns true)', async () => { + const settings = { + linting: { + pylintUseMinimalCheckers: true + } + }; + configService + .setup(c => c.getSettings(doc.uri)) + .returns(() => settings as any); + _info + .setup(info => info.linterArgs(doc.uri)) + .returns(() => []); + Pylint.hasConfigrationFileInWorkspace = () => Promise.resolve(true); // This implies method hasConfigrationFileInWorkspace() returns true + Pylint.hasConfigurationFile = () => Promise.resolve(false); + run = sinon.stub(PylintTest.prototype, 'run'); + run.callsFake(() => Promise.resolve([])); + parseMessagesSeverity = sinon.stub(PylintTest.prototype, 'parseMessagesSeverity'); + parseMessagesSeverity.callsFake(() => 'Severity'); + const pylint = new PylintTest(output.object, serviceContainer.object); + await pylint.runLinter(doc as any, mock(vscode.CancellationTokenSource).token); + assert.deepEqual(run.args[0][0], args); + assert.ok(parseMessagesSeverity.notCalled); + assert.ok(run.calledOnce); + }); + + test('Do not use minimal checkers if there is pylintrc file exists in process or at the workspace root and above (Method hasConfigurationFile() returns true)', async () => { + const settings = { + linting: { + pylintUseMinimalCheckers: true + } + }; + configService + .setup(c => c.getSettings(doc.uri)) + .returns(() => settings as any); + _info + .setup(info => info.linterArgs(doc.uri)) + .returns(() => []); + Pylint.hasConfigrationFileInWorkspace = () => Promise.resolve(false); + Pylint.hasConfigurationFile = () => Promise.resolve(true); // This implies method hasConfigurationFile() returns true + run = sinon.stub(PylintTest.prototype, 'run'); + run.callsFake(() => Promise.resolve([])); + parseMessagesSeverity = sinon.stub(PylintTest.prototype, 'parseMessagesSeverity'); + parseMessagesSeverity.callsFake(() => 'Severity'); + const pylint = new PylintTest(output.object, serviceContainer.object); + await pylint.runLinter(doc as any, mock(vscode.CancellationTokenSource).token); + assert.deepEqual(run.args[0][0], args); + assert.ok(parseMessagesSeverity.notCalled); + assert.ok(run.calledOnce); + }); + + test('Message returned by runLinter() is as expected', async () => { + const message = [{ + type: 'messageType' + }]; + const expectedResult = [{ + type: 'messageType', + severity: 'LintMessageSeverity' + }]; + const settings = { + linting: { + pylintUseMinimalCheckers: true + } + }; + configService + .setup(c => c.getSettings(doc.uri)) + .returns(() => settings as any); + _info + .setup(info => info.linterArgs(doc.uri)) + .returns(() => []); + Pylint.hasConfigrationFileInWorkspace = () => Promise.resolve(false); + Pylint.hasConfigurationFile = () => Promise.resolve(false); + run = sinon.stub(PylintTest.prototype, 'run'); + run.callsFake(() => Promise.resolve(message as any)); + parseMessagesSeverity = sinon.stub(PylintTest.prototype, 'parseMessagesSeverity'); + parseMessagesSeverity.callsFake(() => 'LintMessageSeverity'); + const pylint = new PylintTest(output.object, serviceContainer.object); + const result = await pylint.runLinter(doc as any, mock(vscode.CancellationTokenSource).token); + assert.deepEqual(result, expectedResult as any); + assert.ok(parseMessagesSeverity.calledOnce); + assert.ok(run.calledOnce); + }); +}); From d0b49d5f19be3f8d5bcdf1c42dd81a7cf065a55b Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 13 Aug 2019 10:19:24 -0700 Subject: [PATCH 02/16] Update src/test/linters/pylint.unit.test.ts Co-Authored-By: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com> --- src/test/linters/pylint.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/linters/pylint.unit.test.ts b/src/test/linters/pylint.unit.test.ts index 728cb45365fd..536b86b4c062 100644 --- a/src/test/linters/pylint.unit.test.ts +++ b/src/test/linters/pylint.unit.test.ts @@ -60,7 +60,7 @@ suite('Pylint - Function hasConfigurationFile()', () => { .returns(() => Promise.resolve(pylintrc === pylintrcFile)) .verifiable(TypeMoq.Times.once()); fileSystem - .setup(x => x.fileExists(path.join(folder, pylintrcFile))) + .setup(x => x.fileExists(path.join(folder, dotPylintrc))) .returns(() => Promise.resolve(dotPylintrc === pylintrcFile)) .verifiable(TypeMoq.Times.once()); const hasConfig = await Pylint.hasConfigurationFile(fileSystem.object, folder, platformService.object); From b105d0decae8ba4143ba167e41abf67a40575b82 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 13 Aug 2019 10:19:43 -0700 Subject: [PATCH 03/16] Update src/test/linters/pylint.unit.test.ts Co-Authored-By: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com> --- src/test/linters/pylint.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/linters/pylint.unit.test.ts b/src/test/linters/pylint.unit.test.ts index 536b86b4c062..b70f1573dda3 100644 --- a/src/test/linters/pylint.unit.test.ts +++ b/src/test/linters/pylint.unit.test.ts @@ -158,7 +158,7 @@ suite('Pylint - Function hasConfigurationFile()', () => { platformService.verifyAll(); }); - test('If none of pylintrc configuration files does not exist anywhere, return false', async () => { + test('If none of the pylintrc configuration files exist anywhere, return false', async () => { const home = os.homedir(); fileSystem .setup(x => x.fileExists(path.join(folder, pylintrc))) From 64bf845c656cdfb054c21f5d5e44a7288a071c95 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 13 Aug 2019 10:19:54 -0700 Subject: [PATCH 04/16] Update src/test/linters/pylint.unit.test.ts Co-Authored-By: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com> --- src/test/linters/pylint.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/linters/pylint.unit.test.ts b/src/test/linters/pylint.unit.test.ts index b70f1573dda3..5b4193f15627 100644 --- a/src/test/linters/pylint.unit.test.ts +++ b/src/test/linters/pylint.unit.test.ts @@ -537,7 +537,7 @@ suite('Pylint - Function runLinter()', () => { assert.ok(run.calledOnce); }); - test('Do not use minimal checkers if there is pylintrc file next to the file or above up to and including the workspace root (Method hasConfigrationFileInWorkspace() returns true)', async () => { + test('Do not use minimal checkers if there is a pylintrc file in the current working directory or when traversing the workspace up to its root (hasConfigrationFileInWorkspace() returns true)', async () => { const settings = { linting: { pylintUseMinimalCheckers: true From 3587efdd11d86c52169165ee9c870b690119eaca Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 13 Aug 2019 10:20:02 -0700 Subject: [PATCH 05/16] Update src/test/linters/pylint.unit.test.ts Co-Authored-By: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com> --- src/test/linters/pylint.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/linters/pylint.unit.test.ts b/src/test/linters/pylint.unit.test.ts index 5b4193f15627..e14130f78d8a 100644 --- a/src/test/linters/pylint.unit.test.ts +++ b/src/test/linters/pylint.unit.test.ts @@ -562,7 +562,7 @@ suite('Pylint - Function runLinter()', () => { assert.ok(run.calledOnce); }); - test('Do not use minimal checkers if there is pylintrc file exists in process or at the workspace root and above (Method hasConfigurationFile() returns true)', async () => { + test('Do not use minimal checkers if a pylintrc file exists in the process, in the current working directory or up in the hierarchy tree (hasConfigurationFile() returns true)', async () => { const settings = { linting: { pylintUseMinimalCheckers: true From cf4424843ebcab3c79c82d5faf22afd1e6e82107 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 13 Aug 2019 10:22:49 -0700 Subject: [PATCH 06/16] Update src/test/linters/pylint.unit.test.ts Co-Authored-By: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com> --- src/test/linters/pylint.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/linters/pylint.unit.test.ts b/src/test/linters/pylint.unit.test.ts index e14130f78d8a..cfce2bd9fa4d 100644 --- a/src/test/linters/pylint.unit.test.ts +++ b/src/test/linters/pylint.unit.test.ts @@ -225,7 +225,7 @@ suite('Pylint - Function hasConfigrationFileInWorkspace()', () => { platformService = TypeMoq.Mock.ofType(); }); - test('If none of pylint rc files exist upto the workspace root, return false', async () => { + test('If none of the pylintrc files exist up to the workspace root, return false', async () => { const folder = '/user/a/b/c'; const root = '/user/a'; From dcba22325b21f277a84dd30360ee4839635e7161 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 13 Aug 2019 10:23:12 -0700 Subject: [PATCH 07/16] Update src/test/linters/pylint.unit.test.ts Co-Authored-By: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com> --- src/test/linters/pylint.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/linters/pylint.unit.test.ts b/src/test/linters/pylint.unit.test.ts index cfce2bd9fa4d..438fd8c8cf0e 100644 --- a/src/test/linters/pylint.unit.test.ts +++ b/src/test/linters/pylint.unit.test.ts @@ -270,7 +270,7 @@ suite('Pylint - Function hasConfigrationFileInWorkspace()', () => { fileSystem.verifyAll(); }); - test('If pylintrc file exist while going upto the workspace root, return true', async () => { + test('If the pylintrc file exists while traversing up to the workspace root, return true', async () => { const folder = '/user/a/b/c'; const root = '/user/a'; From d8e2c6db8a05dab1de0e843da528bbbb26c128a4 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 13 Aug 2019 10:24:10 -0700 Subject: [PATCH 08/16] Update src/test/linters/pylint.unit.test.ts Co-Authored-By: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com> --- src/test/linters/pylint.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/linters/pylint.unit.test.ts b/src/test/linters/pylint.unit.test.ts index 438fd8c8cf0e..2c6533b27efe 100644 --- a/src/test/linters/pylint.unit.test.ts +++ b/src/test/linters/pylint.unit.test.ts @@ -194,7 +194,7 @@ suite('Pylint - Function hasConfigurationFile()', () => { platformService.verifyAll(); }); - test('If process env PYLINTRC contains path to pylintrc, return true', async () => { + test(If process.env.PYLINTRC contains the path to pylintrc, return true', async () => { process.env.PYLINTRC = 'path/to/pylintrc'; const hasConfig = await Pylint.hasConfigurationFile(fileSystem.object, folder, platformService.object); expect(hasConfig).to.equal(true, 'Should return true'); From 4734d6ffc3d6a2c839b085faef4fc1a7a32c34e1 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 13 Aug 2019 10:24:39 -0700 Subject: [PATCH 09/16] Update src/test/linters/pylint.unit.test.ts Co-Authored-By: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com> --- src/test/linters/pylint.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/linters/pylint.unit.test.ts b/src/test/linters/pylint.unit.test.ts index 2c6533b27efe..ac0d7d3b2c25 100644 --- a/src/test/linters/pylint.unit.test.ts +++ b/src/test/linters/pylint.unit.test.ts @@ -195,7 +195,7 @@ suite('Pylint - Function hasConfigurationFile()', () => { }); test(If process.env.PYLINTRC contains the path to pylintrc, return true', async () => { - process.env.PYLINTRC = 'path/to/pylintrc'; + process.env.PYLINTRC = path.join('path', 'to', 'pylintrc'); const hasConfig = await Pylint.hasConfigurationFile(fileSystem.object, folder, platformService.object); expect(hasConfig).to.equal(true, 'Should return true'); }); From 0fe1b4fe56dba561df8bbde7be5f92a7ae8a5780 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 13 Aug 2019 11:11:16 -0700 Subject: [PATCH 10/16] Code reviews --- src/test/linters/pylint.unit.test.ts | 205 +++++++++++---------------- 1 file changed, 79 insertions(+), 126 deletions(-) diff --git a/src/test/linters/pylint.unit.test.ts b/src/test/linters/pylint.unit.test.ts index ac0d7d3b2c25..c52cbd902304 100644 --- a/src/test/linters/pylint.unit.test.ts +++ b/src/test/linters/pylint.unit.test.ts @@ -12,6 +12,7 @@ import { mock } from 'ts-mockito'; import * as TypeMoq from 'typemoq'; import * as vscode from 'vscode'; import { IWorkspaceService } from '../../client/common/application/types'; +import { PlatformService } from '../../client/common/platform/platformService'; import { IFileSystem, IPlatformService } from '../../client/common/platform/types'; import { IConfigurationService, IOutputChannel } from '../../client/common/types'; import { IServiceContainer } from '../../client/ioc/types'; @@ -57,19 +58,15 @@ suite('Pylint - Function hasConfigurationFile()', () => { test(`If ${pylintrcFile} exists in the current working directory, return true`, async () => { fileSystem .setup(x => x.fileExists(path.join(folder, pylintrc))) - .returns(() => Promise.resolve(pylintrc === pylintrcFile)) - .verifiable(TypeMoq.Times.once()); + .returns(() => Promise.resolve(pylintrc === pylintrcFile)); fileSystem .setup(x => x.fileExists(path.join(folder, dotPylintrc))) - .returns(() => Promise.resolve(dotPylintrc === pylintrcFile)) - .verifiable(TypeMoq.Times.once()); + .returns(() => Promise.resolve(dotPylintrc === pylintrcFile)); const hasConfig = await Pylint.hasConfigurationFile(fileSystem.object, folder, platformService.object); expect(hasConfig).to.equal(true, 'Should return true'); - fileSystem.verifyAll(); }); test(`If the current working directory is in a Python module, Pylint searches up the hierarchy of Python modules until it finds a ${pylintrcFile} file. And if ${pylintrcFile} exists, return true`, async () => { - const basePath = '/user/a/b/c/d'; fileSystem .setup(x => x.fileExists(path.join('/user/a/b/c/d', '__init__.py'))) .returns(() => Promise.resolve(true)) @@ -93,7 +90,7 @@ suite('Pylint - Function hasConfigurationFile()', () => { fileSystem .setup(x => x.fileExists(path.join('/user/a/b/c', dotPylintrc))) .returns(() => Promise.resolve(dotPylintrc === pylintrcFile)); - const hasConfig = await Pylint.hasConfigurationFile(fileSystem.object, basePath, platformService.object); + const hasConfig = await Pylint.hasConfigurationFile(fileSystem.object, folder, platformService.object); expect(hasConfig).to.equal(true, 'Should return true'); fileSystem.verifyAll(); platformService.verifyAll(); @@ -124,39 +121,41 @@ suite('Pylint - Function hasConfigurationFile()', () => { }); }); - test('If /etc/pylintrc exists in non-Windows platform, return true', async () => { - const home = os.homedir(); - fileSystem - .setup(x => x.fileExists(path.join(folder, pylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - fileSystem - .setup(x => x.fileExists(path.join(folder, dotPylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - fileSystem - .setup(x => x.fileExists(path.join(folder, '__init__.py'))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - fileSystem - .setup(x => x.fileExists(path.join(home, '.config', pylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - fileSystem - .setup(x => x.fileExists(path.join(home, dotPylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - platformService - .setup(x => x.isWindows) - .returns(() => false); - fileSystem - .setup(x => x.fileExists(path.join('/etc', pylintrc))) - .returns(() => Promise.resolve(true)); - const hasConfig = await Pylint.hasConfigurationFile(fileSystem.object, folder, platformService.object); - expect(hasConfig).to.equal(true, 'Should return true'); - fileSystem.verifyAll(); - platformService.verifyAll(); - }); + if (new PlatformService().isWindows === false) { + test('If /etc/pylintrc exists in non-Windows platform, return true', async () => { + const home = os.homedir(); + fileSystem + .setup(x => x.fileExists(path.join(folder, pylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join(folder, dotPylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join(folder, '__init__.py'))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join(home, '.config', pylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join(home, dotPylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + platformService + .setup(x => x.isWindows) + .returns(() => false); + fileSystem + .setup(x => x.fileExists(path.join('/etc', pylintrc))) + .returns(() => Promise.resolve(true)); + const hasConfig = await Pylint.hasConfigurationFile(fileSystem.object, folder, platformService.object); + expect(hasConfig).to.equal(true, 'Should return true'); + fileSystem.verifyAll(); + platformService.verifyAll(); + }); + } test('If none of the pylintrc configuration files exist anywhere, return false', async () => { const home = os.homedir(); @@ -194,7 +193,7 @@ suite('Pylint - Function hasConfigurationFile()', () => { platformService.verifyAll(); }); - test(If process.env.PYLINTRC contains the path to pylintrc, return true', async () => { + test('If process.env.PYLINTRC contains the path to pylintrc, return true', async () => { process.env.PYLINTRC = path.join('path', 'to', 'pylintrc'); const hasConfig = await Pylint.hasConfigurationFile(fileSystem.object, folder, platformService.object); expect(hasConfig).to.equal(true, 'Should return true'); @@ -202,7 +201,7 @@ suite('Pylint - Function hasConfigurationFile()', () => { }); // tslint:disable-next-line:max-func-body-length -suite('Pylint - Function hasConfigrationFileInWorkspace()', () => { +suite('Pylint - Function hasConfigurationFileInWorkspace()', () => { const pylintrc = 'pylintrc'; const dotPylintrc = '.pylintrc'; let fileSystem: TypeMoq.IMock; @@ -270,96 +269,50 @@ suite('Pylint - Function hasConfigrationFileInWorkspace()', () => { fileSystem.verifyAll(); }); - test('If the pylintrc file exists while traversing up to the workspace root, return true', async () => { - const folder = '/user/a/b/c'; - const root = '/user/a'; - - fileSystem - .setup(x => x.fileExists(path.join('/user/a/b/c', pylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - fileSystem - .setup(x => x.fileExists(path.join('/user/a/b/c', dotPylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - - fileSystem - .setup(x => x.fileExists(path.join('/user/a/b', pylintrc))) - // pylintrc file exists - .returns(() => Promise.resolve(true)) - .verifiable(TypeMoq.Times.once()); - fileSystem - .setup(x => x.fileExists(path.join('/user/a/b', dotPylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.never()); + [pylintrc, dotPylintrc].forEach(pylintrcFile => { + test(`If ${pylintrcFile} exists while traversing up to the workspace root, return true`, async () => { + const folder = '/user/a/b/c'; + const root = '/user/a'; - fileSystem - .setup(x => x.fileExists(path.join('/user/a', pylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.never()); - fileSystem - .setup(x => x.fileExists(path.join('/user/a', dotPylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.never()); - - fileSystem - .setup(x => x.fileExists(path.join('/user', dotPylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.never()); - fileSystem - .setup(x => x.fileExists(path.join('/user', dotPylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.never()); - - const hasConfig = await Pylint.hasConfigrationFileInWorkspace(fileSystem.object, folder, root); - expect(hasConfig).to.equal(true, 'Should return true'); - fileSystem.verifyAll(); - }); - - test('If dotpylintrc file exist while going upto the workspace root, return true', async () => { - const folder = '/user/a/b/c'; - const root = '/user/a'; + fileSystem + .setup(x => x.fileExists(path.join('/user/a/b/c', pylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join('/user/a/b/c', dotPylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); - fileSystem - .setup(x => x.fileExists(path.join('/user/a/b/c', pylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - fileSystem - .setup(x => x.fileExists(path.join('/user/a/b/c', dotPylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join('/user/a/b', pylintrc))) + .returns(() => Promise.resolve(pylintrc === pylintrcFile)); + fileSystem + .setup(x => x.fileExists(path.join('/user/a/b', dotPylintrc))) + .returns(() => Promise.resolve(dotPylintrc === pylintrcFile)); - fileSystem - .setup(x => x.fileExists(path.join('/user/a/b', pylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - fileSystem - .setup(x => x.fileExists(path.join('/user/a/b', dotPylintrc))) - // .pylintrc file exists - .returns(() => Promise.resolve(true)) - .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join('/user/a', pylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.never()); + fileSystem + .setup(x => x.fileExists(path.join('/user/a', dotPylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.never()); - fileSystem - .setup(x => x.fileExists(path.join('/user/a', pylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.never()); - fileSystem - .setup(x => x.fileExists(path.join('/user/a', dotPylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.never()); + fileSystem + .setup(x => x.fileExists(path.join('/user', dotPylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.never()); + fileSystem + .setup(x => x.fileExists(path.join('/user', dotPylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.never()); - fileSystem - .setup(x => x.fileExists(path.join('/user', dotPylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.never()); - fileSystem - .setup(x => x.fileExists(path.join('/user', dotPylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.never()); + const hasConfig = await Pylint.hasConfigrationFileInWorkspace(fileSystem.object, folder, root); + expect(hasConfig).to.equal(true, 'Should return true'); + fileSystem.verifyAll(); + }); - const hasConfig = await Pylint.hasConfigrationFileInWorkspace(fileSystem.object, folder, root); - expect(hasConfig).to.equal(true, 'Should return true'); - fileSystem.verifyAll(); }); }); From 0d7f9cc2efb13380a7e88717558f61b7a6f082e3 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 13 Aug 2019 12:30:04 -0700 Subject: [PATCH 11/16] Make folder platform independent --- src/test/linters/pylint.unit.test.ts | 54 ++++++++++++++-------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/src/test/linters/pylint.unit.test.ts b/src/test/linters/pylint.unit.test.ts index c52cbd902304..78172d9ec75f 100644 --- a/src/test/linters/pylint.unit.test.ts +++ b/src/test/linters/pylint.unit.test.ts @@ -21,7 +21,7 @@ import { ILinterInfo, ILinterManager, ILintMessage, LintMessageSeverity } from ' // tslint:disable-next-line:max-func-body-length suite('Pylint - Function hasConfigurationFile()', () => { - const folder = '/user/a/b/c/d'; + const folder = path.join('user', 'a', 'b', 'c', 'd'); const oldValueOfPYLINTRC = process.env.PYLINTRC; const pylintrcFiles = ['pylintrc', '.pylintrc']; const pylintrc = 'pylintrc'; @@ -68,27 +68,27 @@ suite('Pylint - Function hasConfigurationFile()', () => { test(`If the current working directory is in a Python module, Pylint searches up the hierarchy of Python modules until it finds a ${pylintrcFile} file. And if ${pylintrcFile} exists, return true`, async () => { fileSystem - .setup(x => x.fileExists(path.join('/user/a/b/c/d', '__init__.py'))) + .setup(x => x.fileExists(path.join(path.join('user', 'a', 'b', 'c', 'd'), '__init__.py'))) .returns(() => Promise.resolve(true)) .verifiable(TypeMoq.Times.once()); fileSystem - .setup(x => x.fileExists(path.join('/user/a/b/c/d', pylintrc))) + .setup(x => x.fileExists(path.join(path.join('user', 'a', 'b', 'c', 'd'), pylintrc))) .returns(() => Promise.resolve(false)) .verifiable(TypeMoq.Times.atLeastOnce()); fileSystem - .setup(x => x.fileExists(path.join('/user/a/b/c/d', dotPylintrc))) + .setup(x => x.fileExists(path.join(path.join('user', 'a', 'b', 'c', 'd'), dotPylintrc))) .returns(() => Promise.resolve(false)) .verifiable(TypeMoq.Times.atLeastOnce()); fileSystem - .setup(x => x.fileExists(path.join('/user/a/b/c', '__init__.py'))) + .setup(x => x.fileExists(path.join(path.join('user', 'a', 'b', 'c'), '__init__.py'))) .returns(() => Promise.resolve(true)) .verifiable(TypeMoq.Times.once()); fileSystem - .setup(x => x.fileExists(path.join('/user/a/b/c', pylintrc))) + .setup(x => x.fileExists(path.join(path.join('user', 'a', 'b', 'c'), pylintrc))) .returns(() => Promise.resolve(pylintrc === pylintrcFile)); fileSystem - .setup(x => x.fileExists(path.join('/user/a/b/c', dotPylintrc))) + .setup(x => x.fileExists(path.join(path.join('user', 'a', 'b', 'c'), dotPylintrc))) .returns(() => Promise.resolve(dotPylintrc === pylintrcFile)); const hasConfig = await Pylint.hasConfigurationFile(fileSystem.object, folder, platformService.object); expect(hasConfig).to.equal(true, 'Should return true'); @@ -225,42 +225,42 @@ suite('Pylint - Function hasConfigurationFileInWorkspace()', () => { }); test('If none of the pylintrc files exist up to the workspace root, return false', async () => { - const folder = '/user/a/b/c'; - const root = '/user/a'; + const folder = path.join('user', 'a', 'b', 'c'); + const root = path.join('user', 'a'); fileSystem - .setup(x => x.fileExists(path.join('/user/a/b/c', pylintrc))) + .setup(x => x.fileExists(path.join(path.join('user', 'a', 'b', 'c'), pylintrc))) .returns(() => Promise.resolve(false)) .verifiable(TypeMoq.Times.once()); fileSystem - .setup(x => x.fileExists(path.join('/user/a/b/c', dotPylintrc))) + .setup(x => x.fileExists(path.join(path.join('user', 'a', 'b', 'c'), dotPylintrc))) .returns(() => Promise.resolve(false)) .verifiable(TypeMoq.Times.once()); fileSystem - .setup(x => x.fileExists(path.join('/user/a/b', pylintrc))) + .setup(x => x.fileExists(path.join(path.join('user', 'a', 'b'), pylintrc))) .returns(() => Promise.resolve(false)) .verifiable(TypeMoq.Times.once()); fileSystem - .setup(x => x.fileExists(path.join('/user/a/b', dotPylintrc))) + .setup(x => x.fileExists(path.join(path.join('user', 'a', 'b'), dotPylintrc))) .returns(() => Promise.resolve(false)) .verifiable(TypeMoq.Times.once()); fileSystem - .setup(x => x.fileExists(path.join('/user/a', pylintrc))) + .setup(x => x.fileExists(path.join(path.join('user', 'a'), pylintrc))) .returns(() => Promise.resolve(false)) .verifiable(TypeMoq.Times.never()); fileSystem - .setup(x => x.fileExists(path.join('/user/a', dotPylintrc))) + .setup(x => x.fileExists(path.join(path.join('user', 'a'), dotPylintrc))) .returns(() => Promise.resolve(false)) .verifiable(TypeMoq.Times.never()); fileSystem - .setup(x => x.fileExists(path.join('/user', dotPylintrc))) + .setup(x => x.fileExists(path.join(path.join('user'), dotPylintrc))) .returns(() => Promise.resolve(false)) .verifiable(TypeMoq.Times.never()); fileSystem - .setup(x => x.fileExists(path.join('/user', dotPylintrc))) + .setup(x => x.fileExists(path.join(path.join('user'), dotPylintrc))) .returns(() => Promise.resolve(false)) .verifiable(TypeMoq.Times.never()); @@ -271,40 +271,40 @@ suite('Pylint - Function hasConfigurationFileInWorkspace()', () => { [pylintrc, dotPylintrc].forEach(pylintrcFile => { test(`If ${pylintrcFile} exists while traversing up to the workspace root, return true`, async () => { - const folder = '/user/a/b/c'; - const root = '/user/a'; + const folder = path.join('user', 'a', 'b', 'c'); + const root = path.join('user', 'a'); fileSystem - .setup(x => x.fileExists(path.join('/user/a/b/c', pylintrc))) + .setup(x => x.fileExists(path.join(path.join('user', 'a', 'b', 'c'), pylintrc))) .returns(() => Promise.resolve(false)) .verifiable(TypeMoq.Times.once()); fileSystem - .setup(x => x.fileExists(path.join('/user/a/b/c', dotPylintrc))) + .setup(x => x.fileExists(path.join(path.join('user', 'a', 'b', 'c'), dotPylintrc))) .returns(() => Promise.resolve(false)) .verifiable(TypeMoq.Times.once()); fileSystem - .setup(x => x.fileExists(path.join('/user/a/b', pylintrc))) + .setup(x => x.fileExists(path.join(path.join('user', 'a', 'b'), pylintrc))) .returns(() => Promise.resolve(pylintrc === pylintrcFile)); fileSystem - .setup(x => x.fileExists(path.join('/user/a/b', dotPylintrc))) + .setup(x => x.fileExists(path.join(path.join('user', 'a', 'b'), dotPylintrc))) .returns(() => Promise.resolve(dotPylintrc === pylintrcFile)); fileSystem - .setup(x => x.fileExists(path.join('/user/a', pylintrc))) + .setup(x => x.fileExists(path.join(path.join('user', 'a'), pylintrc))) .returns(() => Promise.resolve(false)) .verifiable(TypeMoq.Times.never()); fileSystem - .setup(x => x.fileExists(path.join('/user/a', dotPylintrc))) + .setup(x => x.fileExists(path.join(path.join('user', 'a'), dotPylintrc))) .returns(() => Promise.resolve(false)) .verifiable(TypeMoq.Times.never()); fileSystem - .setup(x => x.fileExists(path.join('/user', dotPylintrc))) + .setup(x => x.fileExists(path.join(path.join('user'), dotPylintrc))) .returns(() => Promise.resolve(false)) .verifiable(TypeMoq.Times.never()); fileSystem - .setup(x => x.fileExists(path.join('/user', dotPylintrc))) + .setup(x => x.fileExists(path.join(path.join('user'), dotPylintrc))) .returns(() => Promise.resolve(false)) .verifiable(TypeMoq.Times.never()); From 189a07409383406fbe0a78448faeaf52e4870784 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 13 Aug 2019 14:08:05 -0700 Subject: [PATCH 12/16] Update src/test/linters/pylint.unit.test.ts --- src/test/linters/pylint.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/linters/pylint.unit.test.ts b/src/test/linters/pylint.unit.test.ts index 78172d9ec75f..87ead3746efb 100644 --- a/src/test/linters/pylint.unit.test.ts +++ b/src/test/linters/pylint.unit.test.ts @@ -440,7 +440,7 @@ suite('Pylint - Function runLinter()', () => { assert.ok(run.calledOnce); }); - test('Use minimal checkers if setting to use minimal checkers is false', async () => { + test('Do not use minimal checkers if setting to use minimal checkers is false', async () => { const settings = { linting: { pylintUseMinimalCheckers: false From 3890101a8af60f9f96feb64b7618182ce4d4d794 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 13 Aug 2019 14:28:41 -0700 Subject: [PATCH 13/16] Update src/test/linters/pylint.unit.test.ts --- src/test/linters/pylint.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/linters/pylint.unit.test.ts b/src/test/linters/pylint.unit.test.ts index 87ead3746efb..75fdd20d40a8 100644 --- a/src/test/linters/pylint.unit.test.ts +++ b/src/test/linters/pylint.unit.test.ts @@ -256,7 +256,7 @@ suite('Pylint - Function hasConfigurationFileInWorkspace()', () => { .verifiable(TypeMoq.Times.never()); fileSystem - .setup(x => x.fileExists(path.join(path.join('user'), dotPylintrc))) + .setup(x => x.fileExists(path.join(path.join('user'), pylintrc))) .returns(() => Promise.resolve(false)) .verifiable(TypeMoq.Times.never()); fileSystem From 82e3f0240654aeccf59bc69357027622ae6d6801 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 13 Aug 2019 15:43:44 -0700 Subject: [PATCH 14/16] Fix DRY --- src/test/linters/pylint.unit.test.ts | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/test/linters/pylint.unit.test.ts b/src/test/linters/pylint.unit.test.ts index 75fdd20d40a8..540671a31078 100644 --- a/src/test/linters/pylint.unit.test.ts +++ b/src/test/linters/pylint.unit.test.ts @@ -264,6 +264,34 @@ suite('Pylint - Function hasConfigurationFileInWorkspace()', () => { .returns(() => Promise.resolve(false)) .verifiable(TypeMoq.Times.never()); + const rootPathItems = ['user', 'a']; + const folderPathItems = ['b', 'c']; // full folder path will be prefixed by root path + let rootPath = ''; + rootPathItems.forEach(item => { + rootPath = path.join(rootPath, item); + fileSystem + .setup(x => x.fileExists(path.join(rootPath, pylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.never()); + fileSystem + .setup(x => x.fileExists(path.join(rootPath, dotPylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.never()); + }); + let relativeFolderPath = ''; + folderPathItems.forEach(item => { + relativeFolderPath = path.join(relativeFolderPath, item); + const absoluteFolderPath = path.join(rootPath, relativeFolderPath); + fileSystem + .setup(x => x.fileExists(path.join(absoluteFolderPath, pylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join(absoluteFolderPath, dotPylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + }); + const hasConfig = await Pylint.hasConfigrationFileInWorkspace(fileSystem.object, folder, root); expect(hasConfig).to.equal(false, 'Should return false'); fileSystem.verifyAll(); From a69dc726bc3e2bdbc4267579fea85520999efcef Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 13 Aug 2019 16:31:15 -0700 Subject: [PATCH 15/16] Oops --- src/test/linters/pylint.unit.test.ts | 36 ---------------------------- 1 file changed, 36 deletions(-) diff --git a/src/test/linters/pylint.unit.test.ts b/src/test/linters/pylint.unit.test.ts index 540671a31078..ea1a1902bcb4 100644 --- a/src/test/linters/pylint.unit.test.ts +++ b/src/test/linters/pylint.unit.test.ts @@ -228,42 +228,6 @@ suite('Pylint - Function hasConfigurationFileInWorkspace()', () => { const folder = path.join('user', 'a', 'b', 'c'); const root = path.join('user', 'a'); - fileSystem - .setup(x => x.fileExists(path.join(path.join('user', 'a', 'b', 'c'), pylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - fileSystem - .setup(x => x.fileExists(path.join(path.join('user', 'a', 'b', 'c'), dotPylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - - fileSystem - .setup(x => x.fileExists(path.join(path.join('user', 'a', 'b'), pylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - fileSystem - .setup(x => x.fileExists(path.join(path.join('user', 'a', 'b'), dotPylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - - fileSystem - .setup(x => x.fileExists(path.join(path.join('user', 'a'), pylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.never()); - fileSystem - .setup(x => x.fileExists(path.join(path.join('user', 'a'), dotPylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.never()); - - fileSystem - .setup(x => x.fileExists(path.join(path.join('user'), pylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.never()); - fileSystem - .setup(x => x.fileExists(path.join(path.join('user'), dotPylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.never()); - const rootPathItems = ['user', 'a']; const folderPathItems = ['b', 'c']; // full folder path will be prefixed by root path let rootPath = ''; From 16057d8128aa7337b0c13d9cb762d1e78570854e Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 14 Aug 2019 10:15:09 -0700 Subject: [PATCH 16/16] Skip tests --- src/test/linters/pylint.unit.test.ts | 72 ++++++++++++++-------------- 1 file changed, 37 insertions(+), 35 deletions(-) diff --git a/src/test/linters/pylint.unit.test.ts b/src/test/linters/pylint.unit.test.ts index ea1a1902bcb4..0fcc9ab2313a 100644 --- a/src/test/linters/pylint.unit.test.ts +++ b/src/test/linters/pylint.unit.test.ts @@ -121,41 +121,43 @@ suite('Pylint - Function hasConfigurationFile()', () => { }); }); - if (new PlatformService().isWindows === false) { - test('If /etc/pylintrc exists in non-Windows platform, return true', async () => { - const home = os.homedir(); - fileSystem - .setup(x => x.fileExists(path.join(folder, pylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - fileSystem - .setup(x => x.fileExists(path.join(folder, dotPylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - fileSystem - .setup(x => x.fileExists(path.join(folder, '__init__.py'))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - fileSystem - .setup(x => x.fileExists(path.join(home, '.config', pylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - fileSystem - .setup(x => x.fileExists(path.join(home, dotPylintrc))) - .returns(() => Promise.resolve(false)) - .verifiable(TypeMoq.Times.once()); - platformService - .setup(x => x.isWindows) - .returns(() => false); - fileSystem - .setup(x => x.fileExists(path.join('/etc', pylintrc))) - .returns(() => Promise.resolve(true)); - const hasConfig = await Pylint.hasConfigurationFile(fileSystem.object, folder, platformService.object); - expect(hasConfig).to.equal(true, 'Should return true'); - fileSystem.verifyAll(); - platformService.verifyAll(); - }); - } + test('If /etc/pylintrc exists in non-Windows platform, return true', async function () { + if (new PlatformService().isWindows) { + // tslint:disable-next-line:no-invalid-this + return this.skip(); + } + const home = os.homedir(); + fileSystem + .setup(x => x.fileExists(path.join(folder, pylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join(folder, dotPylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join(folder, '__init__.py'))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join(home, '.config', pylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + fileSystem + .setup(x => x.fileExists(path.join(home, dotPylintrc))) + .returns(() => Promise.resolve(false)) + .verifiable(TypeMoq.Times.once()); + platformService + .setup(x => x.isWindows) + .returns(() => false); + fileSystem + .setup(x => x.fileExists(path.join('/etc', pylintrc))) + .returns(() => Promise.resolve(true)); + const hasConfig = await Pylint.hasConfigurationFile(fileSystem.object, folder, platformService.object); + expect(hasConfig).to.equal(true, 'Should return true'); + fileSystem.verifyAll(); + platformService.verifyAll(); + }); test('If none of the pylintrc configuration files exist anywhere, return false', async () => { const home = os.homedir();