Skip to content

Commit

Permalink
Trying to diagnose why linting tests are broken
Browse files Browse the repository at this point in the history 
- Fails about 1/3 of runs on Windows- Fails about 1/3 of runs on Windows

- Symptom: lintingEngine::lintOpenPythonFiles returns values *after* command await resolves in lint.tests

- lintOpenPythonFiles returns 3 sets of values, not what I expect (1).

- Haven't yet found a way to await on this properly.
  • Loading branch information
@d3r3kk
d3r3kk committed Sep 18, 2018
1 parent 8d2b9ea commit 8718d97
Show file tree
Hide file tree
Showing 3 changed files with 144 additions and 127 deletions.
5 changes: 4 additions & 1 deletion src/client/linters/lintingEngine.ts
View file
Expand Up @@ -67,7 +67,10 @@ export class LintingEngine implements ILintingEngine {
public async lintOpenPythonFiles(): Promise<vscode.DiagnosticCollection> {
this.diagnosticCollection.clear();
const promises = this.documents.textDocuments.map(async document => this.lintDocument(document, 'auto'));
await Promise.all(promises);
await Promise.all(promises).then((val: void[]) => {
console.log(`[777] Returning ${val.length} items.`);
return val;
});;
return this.diagnosticCollection;
}

Expand Down
2 changes: 1 addition & 1 deletion src/test/index.ts
View file
Expand Up @@ -18,7 +18,7 @@ process.env.IS_MULTI_ROOT_TEST = IS_MULTI_ROOT_TEST.toString();
// If running on CI server and we're running the debugger tests, then ensure we only run debug tests.
// We do this to ensure we only run debugger test, as debugger tests are very flaky on CI.
// So the solution is to run them separately and first on CI.
const grep = IS_CI_SERVER && IS_CI_SERVER_TEST_DEBUGGER ? 'Debug' : undefined;
const grep = 'Linting - General Tests'; // IS_CI_SERVER && IS_CI_SERVER_TEST_DEBUGGER ? 'Debug' : undefined;
const testFilesSuffix = process.env.TEST_FILES_SUFFIX;

// You can directly control Mocha options by uncommenting the following lines.
Expand Down
264 changes: 139 additions & 125 deletions src/test/linters/lint.test.ts
View file
@@ -1,7 +1,7 @@
import * as assert from 'assert';
import * as fs from 'fs-extra';
import * as path from 'path';
import { CancellationTokenSource, ConfigurationTarget, DiagnosticCollection, Uri, window, workspace } from 'vscode';
import { CancellationTokenSource, ConfigurationTarget, DiagnosticCollection, Uri, window, workspace, TextDocument, TextEditor, Diagnostic } from 'vscode';
import { ICommandManager } from '../../client/common/application/types';
import { STANDARD_OUTPUT_CHANNEL } from '../../client/common/constants';
import { Product } from '../../client/common/installer/productInstaller';
Expand All @@ -15,6 +15,7 @@ import { deleteFile, PythonSettingKeys, rootWorkspaceUri } from '../common';
import { closeActiveWindows, initialize, initializeTest, IS_MULTI_ROOT_TEST } from '../initialize';
import { MockOutputChannel } from '../mockClasses';
import { UnitTestIocContainer } from '../unittests/serviceRegistry';
import { Thread } from 'vscode-debugadapter';

const workspaceUri = Uri.file(path.join(__dirname, '..', '..', '..', 'src', 'test'));
const pythoFilesPath = path.join(__dirname, '..', '..', '..', 'src', 'test', 'pythonFiles', 'linting');
Expand Down Expand Up @@ -150,152 +151,165 @@ suite('Linting - General Tests', () => {
return `linting.${linterManager.getLinterInfo(product).enabledSettingName}` as PythonSettingKeys;
}

async function testEnablingDisablingOfLinter(product: Product, enabled: boolean, file?: string) {
const setting = makeSettingKey(product);
const output = ioc.serviceContainer.get<MockOutputChannel>(IOutputChannel, STANDARD_OUTPUT_CHANNEL);
// async function testEnablingDisablingOfLinter(product: Product, enabled: boolean, file?: string) {
// const setting = makeSettingKey(product);
// const output = ioc.serviceContainer.get<MockOutputChannel>(IOutputChannel, STANDARD_OUTPUT_CHANNEL);

await configService.updateSettingAsync(setting, enabled, rootWorkspaceUri,
IS_MULTI_ROOT_TEST ? ConfigurationTarget.WorkspaceFolder : ConfigurationTarget.Workspace);
// await configService.updateSettingAsync(setting, enabled, rootWorkspaceUri,
// IS_MULTI_ROOT_TEST ? ConfigurationTarget.WorkspaceFolder : ConfigurationTarget.Workspace);

file = file ? file : fileToLint;
const document = await workspace.openTextDocument(file);
const cancelToken = new CancellationTokenSource();
// file = file ? file : fileToLint;
// const document = await workspace.openTextDocument(file);
// const cancelToken = new CancellationTokenSource();

await linterManager.setActiveLintersAsync([product]);
await linterManager.enableLintingAsync(enabled);
const linter = linterManager.createLinter(product, output, ioc.serviceContainer);
// await linterManager.setActiveLintersAsync([product]);
// await linterManager.enableLintingAsync(enabled);
// const linter = linterManager.createLinter(product, output, ioc.serviceContainer);

const messages = await linter.lint(document, cancelToken.token);
if (enabled) {
assert.notEqual(messages.length, 0, `No linter errors when linter is enabled, Output - ${output.output}`);
} else {
assert.equal(messages.length, 0, `Errors returned when linter is disabled, Output - ${output.output}`);
}
}
// const messages = await linter.lint(document, cancelToken.token);
// if (enabled) {
// assert.notEqual(messages.length, 0, `No linter errors when linter is enabled, Output - ${output.output}`);
// } else {
// assert.equal(messages.length, 0, `Errors returned when linter is disabled, Output - ${output.output}`);
// }
// }

test('Disable Pylint and test linter', async () => {
await testEnablingDisablingOfLinter(Product.pylint, false);
});
test('Enable Pylint and test linter', async () => {
await testEnablingDisablingOfLinter(Product.pylint, true);
});
test('Disable Pep8 and test linter', async () => {
await testEnablingDisablingOfLinter(Product.pep8, false);
});
test('Enable Pep8 and test linter', async () => {
await testEnablingDisablingOfLinter(Product.pep8, true);
});
test('Disable Flake8 and test linter', async () => {
await testEnablingDisablingOfLinter(Product.flake8, false);
});
test('Enable Flake8 and test linter', async () => {
await testEnablingDisablingOfLinter(Product.flake8, true);
});
test('Disable Prospector and test linter', async () => {
await testEnablingDisablingOfLinter(Product.prospector, false);
});
test('Enable Prospector and test linter', async () => {
await testEnablingDisablingOfLinter(Product.prospector, true);
});
test('Disable Pydocstyle and test linter', async () => {
await testEnablingDisablingOfLinter(Product.pydocstyle, false);
});
test('Enable Pydocstyle and test linter', async () => {
await testEnablingDisablingOfLinter(Product.pydocstyle, true);
});
// test('Disable Pylint and test linter', async () => {
// await testEnablingDisablingOfLinter(Product.pylint, false);
// });
// test('Enable Pylint and test linter', async () => {
// await testEnablingDisablingOfLinter(Product.pylint, true);
// });
// test('Disable Pep8 and test linter', async () => {
// await testEnablingDisablingOfLinter(Product.pep8, false);
// });
// test('Enable Pep8 and test linter', async () => {
// await testEnablingDisablingOfLinter(Product.pep8, true);
// });
// test('Disable Flake8 and test linter', async () => {
// await testEnablingDisablingOfLinter(Product.flake8, false);
// });
// test('Enable Flake8 and test linter', async () => {
// await testEnablingDisablingOfLinter(Product.flake8, true);
// });
// test('Disable Prospector and test linter', async () => {
// await testEnablingDisablingOfLinter(Product.prospector, false);
// });
// test('Enable Prospector and test linter', async () => {
// await testEnablingDisablingOfLinter(Product.prospector, true);
// });
// test('Disable Pydocstyle and test linter', async () => {
// await testEnablingDisablingOfLinter(Product.pydocstyle, false);
// });
// test('Enable Pydocstyle and test linter', async () => {
// await testEnablingDisablingOfLinter(Product.pydocstyle, true);
// });

// tslint:disable-next-line:no-any
async function testLinterMessages(product: Product, pythonFile: string, messagesToBeReceived: ILintMessage[]): Promise<any> {
const outputChannel = ioc.serviceContainer.get<MockOutputChannel>(IOutputChannel, STANDARD_OUTPUT_CHANNEL);
const cancelToken = new CancellationTokenSource();
const document = await workspace.openTextDocument(pythonFile);
// // tslint:disable-next-line:no-any
// async function testLinterMessages(product: Product, pythonFile: string, messagesToBeReceived: ILintMessage[]): Promise<any> {
// const outputChannel = ioc.serviceContainer.get<MockOutputChannel>(IOutputChannel, STANDARD_OUTPUT_CHANNEL);
// const cancelToken = new CancellationTokenSource();
// const document = await workspace.openTextDocument(pythonFile);

await linterManager.setActiveLintersAsync([product], document.uri);
const linter = linterManager.createLinter(product, outputChannel, ioc.serviceContainer);
// await linterManager.setActiveLintersAsync([product], document.uri);
// const linter = linterManager.createLinter(product, outputChannel, ioc.serviceContainer);

const messages = await linter.lint(document, cancelToken.token);
if (messagesToBeReceived.length === 0) {
assert.equal(messages.length, 0, `No errors in linter, Output - ${outputChannel.output}`);
} else {
if (outputChannel.output.indexOf('ENOENT') === -1) {
// Pylint for Python Version 2.7 could return 80 linter messages, where as in 3.5 it might only return 1.
// Looks like pylint stops linting as soon as it comes across any ERRORS.
assert.notEqual(messages.length, 0, `No errors in linter, Output - ${outputChannel.output}`);
}
}
}
test('PyLint', async () => {
await testLinterMessages(Product.pylint, fileToLint, pylintMessagesToBeReturned);
});
test('Flake8', async () => {
await testLinterMessages(Product.flake8, fileToLint, flake8MessagesToBeReturned);
});
test('Pep8', async () => {
await testLinterMessages(Product.pep8, fileToLint, pep8MessagesToBeReturned);
});
test('Pydocstyle', async () => {
await testLinterMessages(Product.pydocstyle, fileToLint, pydocstyleMessagseToBeReturned);
});
test('PyLint with config in root', async () => {
await fs.copy(path.join(pylintConfigPath, '.pylintrc'), path.join(workspaceUri.fsPath, '.pylintrc'));
await testLinterMessages(Product.pylint, path.join(pylintConfigPath, 'file2.py'), []);
});
test('Flake8 with config in root', async () => {
await testLinterMessages(Product.flake8, path.join(flake8ConfigPath, 'file.py'), filteredFlake8MessagesToBeReturned);
});
test('Pep8 with config in root', async () => {
await testLinterMessages(Product.pep8, path.join(pep8ConfigPath, 'file.py'), filteredPep88MessagesToBeReturned);
});
test('Pydocstyle with config in root', async () => {
await configService.updateSettingAsync('linting.pylintUseMinimalCheckers', false, workspaceUri);
await fs.copy(path.join(pydocstyleConfigPath27, '.pydocstyle'), path.join(workspaceUri.fsPath, '.pydocstyle'));
await testLinterMessages(Product.pydocstyle, path.join(pydocstyleConfigPath27, 'file.py'), []);
});
test('PyLint minimal checkers', async () => {
const file = path.join(pythoFilesPath, 'minCheck.py');
await configService.updateSettingAsync('linting.pylintUseMinimalCheckers', true, workspaceUri);
await testEnablingDisablingOfLinter(Product.pylint, false, file);
await configService.updateSettingAsync('linting.pylintUseMinimalCheckers', false, workspaceUri);
await testEnablingDisablingOfLinter(Product.pylint, true, file);
});
// const messages = await linter.lint(document, cancelToken.token);
// if (messagesToBeReceived.length === 0) {
// assert.equal(messages.length, 0, `No errors in linter, Output - ${outputChannel.output}`);
// } else {
// if (outputChannel.output.indexOf('ENOENT') === -1) {
// // Pylint for Python Version 2.7 could return 80 linter messages, where as in 3.5 it might only return 1.
// // Looks like pylint stops linting as soon as it comes across any ERRORS.
// assert.notEqual(messages.length, 0, `No errors in linter, Output - ${outputChannel.output}`);
// }
// }
// }
// test('PyLint', async () => {
// await testLinterMessages(Product.pylint, fileToLint, pylintMessagesToBeReturned);
// });
// test('Flake8', async () => {
// await testLinterMessages(Product.flake8, fileToLint, flake8MessagesToBeReturned);
// });
// test('Pep8', async () => {
// await testLinterMessages(Product.pep8, fileToLint, pep8MessagesToBeReturned);
// });
// test('Pydocstyle', async () => {
// await testLinterMessages(Product.pydocstyle, fileToLint, pydocstyleMessagseToBeReturned);
// });
// test('PyLint with config in root', async () => {
// await fs.copy(path.join(pylintConfigPath, '.pylintrc'), path.join(workspaceUri.fsPath, '.pylintrc'));
// await testLinterMessages(Product.pylint, path.join(pylintConfigPath, 'file2.py'), []);
// });
// test('Flake8 with config in root', async () => {
// await testLinterMessages(Product.flake8, path.join(flake8ConfigPath, 'file.py'), filteredFlake8MessagesToBeReturned);
// });
// test('Pep8 with config in root', async () => {
// await testLinterMessages(Product.pep8, path.join(pep8ConfigPath, 'file.py'), filteredPep88MessagesToBeReturned);
// });
// test('Pydocstyle with config in root', async () => {
// await configService.updateSettingAsync('linting.pylintUseMinimalCheckers', false, workspaceUri);
// await fs.copy(path.join(pydocstyleConfigPath27, '.pydocstyle'), path.join(workspaceUri.fsPath, '.pydocstyle'));
// await testLinterMessages(Product.pydocstyle, path.join(pydocstyleConfigPath27, 'file.py'), []);
// });
// test('PyLint minimal checkers', async () => {
// const file = path.join(pythoFilesPath, 'minCheck.py');
// await configService.updateSettingAsync('linting.pylintUseMinimalCheckers', true, workspaceUri);
// await testEnablingDisablingOfLinter(Product.pylint, false, file);
// await configService.updateSettingAsync('linting.pylintUseMinimalCheckers', false, workspaceUri);
// await testEnablingDisablingOfLinter(Product.pylint, true, file);
// });
// tslint:disable-next-line:no-function-expression
test('Multiple linters', async function () {
// tslint:disable-next-line:no-invalid-this
this.timeout(40000);

await closeActiveWindows();
const document = await workspace.openTextDocument(path.join(pythoFilesPath, 'print.py'));
await window.showTextDocument(document);
await configService.updateSettingAsync('linting.enabled', true, workspaceUri);
await configService.updateSettingAsync('linting.pylintUseMinimalCheckers', false, workspaceUri);
await configService.updateSettingAsync('linting.pylintEnabled', true, workspaceUri);
await configService.updateSettingAsync('linting.flake8Enabled', true, workspaceUri);
await closeActiveWindows().then(() => { console.log('[000] Active windows are closed.')});
const document = await workspace.openTextDocument(path.join(pythoFilesPath, 'print.py'))
.then((td: TextDocument) => { console.log(`[111] opened document ${td.fileName}`); return td; });
await window.showTextDocument(document)
.then((te: TextEditor) => { console.log('[222] Text editor is showing.')});
await configService.updateSettingAsync('linting.enabled', true, workspaceUri)
.then(() => console.log('[333] linting.enabled set to true'));
await configService.updateSettingAsync('linting.pylintUseMinimalCheckers', false, workspaceUri)
.then(() => console.log('[444] linting.pylintUseMinimalCheckers set to false'));
await configService.updateSettingAsync('linting.pylintEnabled', true, workspaceUri)
.then(() => console.log('[555] linting.pylintEnabled set to true'));
await configService.updateSettingAsync('linting.flake8Enabled', true, workspaceUri)
.then(() => console.log('[666] linting.flake8Enabled set to true'));

const commands = ioc.serviceContainer.get<ICommandManager>(ICommandManager);
const collection = await commands.executeCommand('python.runLinting') as DiagnosticCollection;

const pr = await commands.executeCommand('python.runLinting')
.then((value: {}) => { console.log(`[888] Value received is: ${value}`); return value; },
(rejectedReason: any) => { console.log(`[888-B] Rejection message received. Reason: ${rejectedReason}.`)});

const collection = pr as DiagnosticCollection;
console.log(`Collection: '${collection}'. Collection Name: '${collection.name}'. Document: '${document}'. Document URI: '${document.uri}'.)`);

assert.notEqual(collection, undefined, 'python.runLinting did not return valid diagnostics collection.');

const messages = collection!.get(document.uri);
console.log(`Collected ${messages.length} messages.`);
assert.notEqual(messages!.length, 0, 'No diagnostic messages.');
assert.notEqual(messages!.filter(x => x.source === 'pylint').length, 0, 'No pylint messages.');
assert.notEqual(messages!.filter(x => x.source === 'flake8').length, 0, 'No flake8 messages.');
});
// tslint:disable-next-line:no-any
async function testLinterMessageCount(product: Product, pythonFile: string, messageCountToBeReceived: number): Promise<any> {
const outputChannel = ioc.serviceContainer.get<MockOutputChannel>(IOutputChannel, STANDARD_OUTPUT_CHANNEL);
const cancelToken = new CancellationTokenSource();
const document = await workspace.openTextDocument(pythonFile);
// async function testLinterMessageCount(product: Product, pythonFile: string, messageCountToBeReceived: number): Promise<any> {
// const outputChannel = ioc.serviceContainer.get<MockOutputChannel>(IOutputChannel, STANDARD_OUTPUT_CHANNEL);
// const cancelToken = new CancellationTokenSource();
// const document = await workspace.openTextDocument(pythonFile);

await linterManager.setActiveLintersAsync([product], document.uri);
const linter = linterManager.createLinter(product, outputChannel, ioc.serviceContainer);
// await linterManager.setActiveLintersAsync([product], document.uri);
// const linter = linterManager.createLinter(product, outputChannel, ioc.serviceContainer);

const messages = await linter.lint(document, cancelToken.token);
assert.equal(messages.length, messageCountToBeReceived, 'Expected number of lint errors does not match lint error count');
}
test('Three line output counted as one message', async () => {
const maxErrors = 5;
const target = IS_MULTI_ROOT_TEST ? ConfigurationTarget.WorkspaceFolder : ConfigurationTarget.Workspace;
await configService.updateSettingAsync('linting.maxNumberOfProblems', maxErrors, rootWorkspaceUri, target);
await testLinterMessageCount(Product.pylint, threeLineLintsPath, maxErrors);
});
// const messages = await linter.lint(document, cancelToken.token);
// assert.equal(messages.length, messageCountToBeReceived, 'Expected number of lint errors does not match lint error count');
// }
// test('Three line output counted as one message', async () => {
// const maxErrors = 5;
// const target = IS_MULTI_ROOT_TEST ? ConfigurationTarget.WorkspaceFolder : ConfigurationTarget.Workspace;
// await configService.updateSettingAsync('linting.maxNumberOfProblems', maxErrors, rootWorkspaceUri, target);
// await testLinterMessageCount(Product.pylint, threeLineLintsPath, maxErrors);
// });
});

0 comments on commit 8718d97

Please sign in to comment.