Skip to content

Commit f96bf83

Browse files
kimadelinekimadeline
authored
Remove existing positional args when running a single test on pytest (microsoft#6537)
* Make positional arg parsing more flexible * Add news file * Make tests pass * added tests for argumentsHelper.ts * Add tests for getPositionalArguments only * Remove comment * Increase coverage * Fix test * Fix typo
1 parent 360620d commit f96bf83

File tree

5 files changed

+95
-18
lines changed

5 files changed

+95
-18
lines changed

‎news/2 Fixes/5757.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Remove existing positional arguments when running single pytest tests.

‎src/client/testing/common/argumentsHelper.ts‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,29 +44,30 @@ export class ArgumentsHelper implements IArgumentsHelper {
4444
}
4545
}
4646
public getPositionalArguments(args: string[], optionsWithArguments: string[] = [], optionsWithoutArguments: string[] = []): string[] {
47-
let lastIndexOfOption = -1;
47+
const nonPositionalIndexes: number[] = [];
4848
args.forEach((arg, index) => {
4949
if (optionsWithoutArguments.indexOf(arg) !== -1) {
50-
lastIndexOfOption = index;
50+
nonPositionalIndexes.push(index);
5151
return;
5252
} else if (optionsWithArguments.indexOf(arg) !== -1) {
53+
nonPositionalIndexes.push(index);
5354
// Cuz the next item is the value.
54-
lastIndexOfOption = index + 1;
55+
nonPositionalIndexes.push(index + 1);
5556
} else if (optionsWithArguments.findIndex(item => arg.startsWith(`${item}=`)) !== -1) {
56-
lastIndexOfOption = index;
57+
nonPositionalIndexes.push(index);
5758
return;
5859
} else if (arg.startsWith('-')) {
5960
// Ok this is an unknown option, lets treat this as one without values.
6061
this.logger.logWarning(`Unknown command line option passed into args parser for tests '${arg}'. Please report on https://github.com/Microsoft/vscode-python/issues/new`);
61-
lastIndexOfOption = index;
62+
nonPositionalIndexes.push(index);
6263
return;
63-
} else if (args.indexOf('=') > 0) {
64+
} else if (arg.indexOf('=') > 0) {
6465
// Ok this is an unknown option with a value
6566
this.logger.logWarning(`Unknown command line option passed into args parser for tests '${arg}'. Please report on https://github.com/Microsoft/vscode-python/issues/new`);
66-
lastIndexOfOption = index;
67+
nonPositionalIndexes.push(index);
6768
}
6869
});
69-
return args.slice(lastIndexOfOption + 1);
70+
return args.filter((_, index) => nonPositionalIndexes.indexOf(index) === -1);
7071
}
7172
public filterArguments(args: string[], optionsWithArguments: string[] = [], optionsWithoutArguments: string[] = []): string[] {
7273
let ignoreIndex = -1;

‎src/test/common/argumentsHelper.unit.test.ts‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
'use strict';
5+
6+
import { expect } from 'chai';
7+
import { anyString, instance, mock, when } from 'ts-mockito';
8+
import { Logger } from '../../client/common/logger';
9+
import { ILogger } from '../../client/common/types';
10+
import { ServiceContainer } from '../../client/ioc/container';
11+
import { IServiceContainer } from '../../client/ioc/types';
12+
import { ArgumentsHelper } from '../../client/testing/common/argumentsHelper';
13+
14+
suite('ArgumentsHelper tests', () => {
15+
let argumentsHelper: ArgumentsHelper;
16+
17+
setup(() => {
18+
const logger: ILogger = mock(Logger);
19+
when(logger.logWarning(anyString())).thenReturn();
20+
const serviceContainer: IServiceContainer = mock(ServiceContainer);
21+
when(serviceContainer.get<ILogger>(ILogger)).thenReturn(instance(logger));
22+
23+
argumentsHelper = new ArgumentsHelper(instance(serviceContainer));
24+
});
25+
26+
test('getPositionalArguments with both options parameters should return correct positional arguments', () => {
27+
const args = ['arg1', '--foo', 'arg2', '--bar', 'arg3', 'arg4'];
28+
const optionsWithArguments = ['--bar'];
29+
const optionsWithoutArguments = ['--foo'];
30+
const result = argumentsHelper.getPositionalArguments(args, optionsWithArguments, optionsWithoutArguments);
31+
32+
expect(result).to.have.length(3);
33+
expect(result).to.deep.equal(['arg1', 'arg2', 'arg4']);
34+
});
35+
36+
test('getPositionalArguments with optionsWithArguments with inline `option=value` syntax should return correct positional arguments', () => {
37+
const args = ['arg1', '--foo', 'arg2', '--bar=arg3', 'arg4'];
38+
const optionsWithArguments = ['--bar'];
39+
const optionsWithoutArguments = ['--foo'];
40+
const result = argumentsHelper.getPositionalArguments(args, optionsWithArguments, optionsWithoutArguments);
41+
42+
expect(result).to.have.length(3);
43+
expect(result).to.deep.equal(['arg1', 'arg2', 'arg4']);
44+
});
45+
46+
test('getPositionalArguments with unknown arguments with inline `option=value` syntax should return correct positional arguments', () => {
47+
const args = ['arg1', '--foo', 'arg2', 'bar=arg3', 'arg4'];
48+
const optionsWithArguments: string[] = [];
49+
const optionsWithoutArguments = ['--foo'];
50+
const result = argumentsHelper.getPositionalArguments(args, optionsWithArguments, optionsWithoutArguments);
51+
52+
expect(result).to.have.length(3);
53+
expect(result).to.deep.equal(['arg1', 'arg2', 'arg4']);
54+
});
55+
56+
test('getPositionalArguments with no options parameter should be the same as passing empty arrays', () => {
57+
const args = ['arg1', '--foo', 'arg2', '--bar', 'arg3', 'arg4'];
58+
const optionsWithArguments: string[] = [];
59+
const optionsWithoutArguments: string[] = [];
60+
const result = argumentsHelper.getPositionalArguments(args, optionsWithArguments, optionsWithoutArguments);
61+
62+
expect(result).to.deep.equal(argumentsHelper.getPositionalArguments(args));
63+
expect(result).to.have.length(4);
64+
expect(result).to.deep.equal(['arg1', 'arg2', 'arg3', 'arg4']);
65+
});
66+
});

‎src/test/testing/nosetest/nosetest.argsService.unit.test.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ suite('ArgsService: nosetest', () => {
3434

3535
test('Test getting the test folder in nosetest', () => {
3636
const dir = path.join('a', 'b', 'c');
37-
const args = ['anzy', '--one', '--three', dir];
37+
const args = ['--one', '--three', dir];
3838
const testDirs = argumentsService.getTestFolders(args);
3939
expect(testDirs).to.be.lengthOf(1);
4040
expect(testDirs[0]).to.equal(dir);
@@ -44,8 +44,9 @@ suite('ArgsService: nosetest', () => {
4444
const dir2 = path.join('a', 'b', '2');
4545
const args = ['anzy', '--one', '--three', dir, dir2];
4646
const testDirs = argumentsService.getTestFolders(args);
47-
expect(testDirs).to.be.lengthOf(2);
48-
expect(testDirs[0]).to.equal(dir);
49-
expect(testDirs[1]).to.equal(dir2);
47+
expect(testDirs).to.be.lengthOf(3);
48+
expect(testDirs[0]).to.equal('anzy');
49+
expect(testDirs[1]).to.equal(dir);
50+
expect(testDirs[2]).to.equal(dir2);
5051
});
5152
});

‎src/test/testing/pytest/pytest.argsService.unit.test.ts‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,14 @@ suite('ArgsService: pytest', () => {
3434

3535
test('Test getting the test folder in pytest', () => {
3636
const dir = path.join('a', 'b', 'c');
37-
const args = ['anzy', '--one', '--rootdir', dir];
37+
const args = ['--one', '--rootdir', dir];
38+
const testDirs = argumentsService.getTestFolders(args);
39+
expect(testDirs).to.be.lengthOf(1);
40+
expect(testDirs[0]).to.equal(dir);
41+
});
42+
test('Test getting the test folder in pytest (with folder before the arguments)', () => {
43+
const dir = path.join('a', 'b', 'c');
44+
const args = [dir, '--one', '--rootdir'];
3845
const testDirs = argumentsService.getTestFolders(args);
3946
expect(testDirs).to.be.lengthOf(1);
4047
expect(testDirs[0]).to.equal(dir);
@@ -59,15 +66,15 @@ suite('ArgsService: pytest', () => {
5966
});
6067
test('Test getting the test folder in pytest (with single positional dir)', () => {
6168
const dir = path.join('a', 'b', 'c');
62-
const args = ['anzy', '--one', dir];
69+
const args = ['--one', dir];
6370
const testDirs = argumentsService.getTestFolders(args);
6471
expect(testDirs).to.be.lengthOf(1);
6572
expect(testDirs[0]).to.equal(dir);
6673
});
6774
test('Test getting the test folder in pytest (with multiple positional dirs)', () => {
6875
const dir = path.join('a', 'b', 'c');
6976
const dir2 = path.join('a', 'b', '2');
70-
const args = ['anzy', '--one', dir, dir2];
77+
const args = ['--one', dir, dir2];
7178
const testDirs = argumentsService.getTestFolders(args);
7279
expect(testDirs).to.be.lengthOf(2);
7380
expect(testDirs[0]).to.equal(dir);
@@ -78,8 +85,9 @@ suite('ArgsService: pytest', () => {
7885
const dir2 = path.join('a', 'b', '2');
7986
const args = ['anzy', '--one', dir, dir2, path.join(dir, 'one.py')];
8087
const testDirs = argumentsService.getTestFolders(args);
81-
expect(testDirs).to.be.lengthOf(2);
82-
expect(testDirs[0]).to.equal(dir);
83-
expect(testDirs[1]).to.equal(dir2);
88+
expect(testDirs).to.be.lengthOf(3);
89+
expect(testDirs[0]).to.equal('anzy');
90+
expect(testDirs[1]).to.equal(dir);
91+
expect(testDirs[2]).to.equal(dir2);
8492
});
8593
});

0 commit comments

Comments
 (0)