Skip to content

Commit 5b37f00

Browse files
eleanorjboydCopilot
andcommitted
Refine compact discovery payload review feedback
- Move isAbsolutePath helper to common/platform/fs-paths so it can be shared instead of duplicated inside testDiscoveryHandler. Uses path.posix.isAbsolute || path.win32.isAbsolute so paths produced on either OS (e.g. coming from a Python subprocess) are recognized. - Tighten create_class_node annotation in vscode_pytest from Any to pytest.Class | DescribeBlockType. DescribeBlockType is imported under TYPE_CHECKING so it remains optional at runtime. - Add unit tests for isAbsolutePath covering POSIX absolute, Windows drive-letter, UNC, and relative paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent af97557 commit 5b37f00

4 files changed

Lines changed: 42 additions & 6 deletions

File tree

python_files/vscode_pytest/__init__.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
if TYPE_CHECKING:
2727
from pluggy import Result
28+
from pytest_describe.plugin import DescribeBlock as DescribeBlockType
2829
from typing_extensions import NotRequired
2930

3031
USES_PYTEST_DESCRIBE = False
@@ -873,7 +874,7 @@ def create_session_node(session: pytest.Session) -> TestNode:
873874
}
874875

875876

876-
def create_class_node(class_module: Any) -> TestNode:
877+
def create_class_node(class_module: pytest.Class | DescribeBlockType) -> TestNode:
877878
"""Creates a class node from a pytest class object.
878879
879880
Keyword arguments:

src/client/common/platform/fs-paths.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,19 @@ export function normCasePath(filePath: string): string {
148148
return normCase(nodepath.normalize(filePath));
149149
}
150150

151+
/**
152+
* Returns true if the given path is absolute on either POSIX or Windows.
153+
*
154+
* Node's `path.isAbsolute` is platform-dependent, so a POSIX path like `/foo`
155+
* is not recognized as absolute on Windows, and a Windows path like `C:\foo`
156+
* is not recognized as absolute on POSIX. Use this helper when the path could
157+
* have been produced on a different OS than the one currently running (e.g.
158+
* paths received in a JSON payload from a Python subprocess).
159+
*/
160+
export function isAbsolutePath(value: string): boolean {
161+
return nodepath.posix.isAbsolute(value) || nodepath.win32.isAbsolute(value);
162+
}
163+
151164
export function normCase(s: string): string {
152165
return getOSType() === OSType.Windows ? s.toUpperCase() : s;
153166
}

src/client/testing/testController/common/testDiscoveryHandler.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,7 @@ import { createErrorTestItem } from './testItemUtilities';
1111
import { buildErrorNodeOptions, populateTestTree } from './utils';
1212
import { TestItemIndex } from './testItemIndex';
1313
import { PROJECT_ID_SEPARATOR } from './projectUtils';
14-
15-
function isAbsolutePath(value: string): boolean {
16-
return /^([a-zA-Z]:[\\/]|\\\\)/.test(value) || value.startsWith('/');
17-
}
14+
import { isAbsolutePath } from '../../../common/platform/fs-paths';
1815

1916
function joinWithBase(base: string, relativePath: string): string {
2017
if (!base || isAbsolutePath(relativePath)) {

src/test/common/platform/fs-paths.unit.test.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
import { expect } from 'chai';
55
import * as path from 'path';
66
import * as TypeMoq from 'typemoq';
7-
import { FileSystemPathUtils } from '../../../client/common/platform/fs-paths';
7+
import { FileSystemPathUtils, isAbsolutePath } from '../../../client/common/platform/fs-paths';
88
import { getNamesAndValues } from '../../../client/common/utils/enum';
99
import { OSType } from '../../../client/common/utils/platform';
1010

@@ -112,3 +112,28 @@ suite('FileSystem - Path Utils', () => {
112112
});
113113
});
114114
});
115+
116+
suite('FileSystem - isAbsolutePath', () => {
117+
test('Recognizes POSIX absolute paths', () => {
118+
expect(isAbsolutePath('/foo/bar')).to.equal(true);
119+
expect(isAbsolutePath('/')).to.equal(true);
120+
});
121+
122+
test('Recognizes Windows drive-letter absolute paths', () => {
123+
expect(isAbsolutePath('C:\\foo\\bar')).to.equal(true);
124+
expect(isAbsolutePath('c:/foo/bar')).to.equal(true);
125+
expect(isAbsolutePath('Z:\\')).to.equal(true);
126+
});
127+
128+
test('Recognizes Windows UNC absolute paths', () => {
129+
expect(isAbsolutePath('\\\\server\\share\\foo')).to.equal(true);
130+
});
131+
132+
test('Rejects relative paths', () => {
133+
expect(isAbsolutePath('foo/bar')).to.equal(false);
134+
expect(isAbsolutePath('./foo')).to.equal(false);
135+
expect(isAbsolutePath('../foo')).to.equal(false);
136+
expect(isAbsolutePath('foo\\bar')).to.equal(false);
137+
expect(isAbsolutePath('')).to.equal(false);
138+
});
139+
});

0 commit comments

Comments
 (0)