Skip to content

Commit 2e7d210

Browse files
eleanorjboydseeM
authored andcommitted
support symlinks in parents of the root of the workspace (microsoft/vscode-python#23257)
fixes microsoft/vscode-python#23223
1 parent b608874 commit 2e7d210

File tree

8 files changed

+335
-19
lines changed

8 files changed

+335
-19
lines changed
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
# Copyright (c) Microsoft Corporation. All rights reserved.
2+
# Licensed under the MIT License.
3+
4+
import pathlib
5+
import tempfile
6+
import os
7+
import sys
8+
9+
from .helpers import ( # noqa: E402
10+
TEST_DATA_PATH,
11+
)
12+
13+
script_dir = pathlib.Path(__file__).parent.parent.parent
14+
sys.path.append(os.fspath(script_dir))
15+
from vscode_pytest import has_symlink_parent # noqa: E402
16+
17+
18+
def test_has_symlink_parent_with_symlink():
19+
# Create a temporary directory and a file in it
20+
with tempfile.TemporaryDirectory() as temp_dir:
21+
file_path = pathlib.Path(temp_dir) / "file"
22+
file_path.touch()
23+
24+
# Create a symbolic link to the temporary directory
25+
symlink_path = pathlib.Path(temp_dir) / "symlink"
26+
symlink_path.symlink_to(temp_dir)
27+
28+
# Check that has_symlink_parent correctly identifies the symbolic link
29+
assert has_symlink_parent(symlink_path / "file")
30+
31+
32+
def test_has_symlink_parent_without_symlink():
33+
folder_path = TEST_DATA_PATH / "unittest_folder" / "test_add.py"
34+
# Check that has_symlink_parent correctly identifies that there are no symbolic links
35+
assert not has_symlink_parent(folder_path)

extensions/positron-python/python_files/vscode_pytest/__init__.py

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,13 +79,21 @@ def pytest_load_initial_conftests(early_config, parser, args):
7979
raise VSCodePytestError(
8080
f"The path set in the argument --rootdir={rootdir} does not exist."
8181
)
82-
if (
83-
os.path.islink(rootdir)
84-
and pathlib.Path(os.path.realpath(rootdir)) == pathlib.Path.cwd()
85-
):
82+
83+
# Check if the rootdir is a symlink or a child of a symlink to the current cwd.
84+
isSymlink = False
85+
86+
if os.path.islink(rootdir):
87+
isSymlink = True
8688
print(
87-
f"Plugin info[vscode-pytest]: rootdir argument, {rootdir}, is identified as a symlink to the cwd, {pathlib.Path.cwd()}.",
88-
"Therefore setting symlink path to rootdir argument.",
89+
f"Plugin info[vscode-pytest]: rootdir argument, {rootdir}, is identified as a symlink."
90+
)
91+
elif pathlib.Path(os.path.realpath(rootdir)) != rootdir:
92+
print("Plugin info[vscode-pytest]: Checking if rootdir is a child of a symlink.")
93+
isSymlink = has_symlink_parent(rootdir)
94+
if isSymlink:
95+
print(
96+
f"Plugin info[vscode-pytest]: rootdir argument, {rootdir}, is identified as a symlink or child of a symlink, adjusting pytest paths accordingly.",
8997
)
9098
global SYMLINK_PATH
9199
SYMLINK_PATH = pathlib.Path(rootdir)
@@ -144,6 +152,21 @@ def pytest_exception_interact(node, call, report):
144152
)
145153

146154

155+
def has_symlink_parent(current_path):
156+
"""Recursively checks if any parent directories of the given path are symbolic links."""
157+
# Convert the current path to an absolute Path object
158+
curr_path = pathlib.Path(current_path)
159+
print("Checking for symlink parent starting at current path: ", curr_path)
160+
161+
# Iterate over all parent directories
162+
for parent in curr_path.parents:
163+
# Check if the parent directory is a symlink
164+
if os.path.islink(parent):
165+
print(f"Symlink found at: {parent}")
166+
return True
167+
return False
168+
169+
147170
def get_absolute_test_id(test_id: str, testPath: pathlib.Path) -> str:
148171
"""A function that returns the absolute test id. This is necessary because testIds are relative to the rootdir.
149172
This does not work for our case since testIds when referenced during run time are relative to the instantiation

extensions/positron-python/src/client/testing/testController/common/utils.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the MIT License.
33
import * as net from 'net';
44
import * as path from 'path';
5+
import * as fs from 'fs';
56
import { CancellationToken, Position, TestController, TestItem, Uri, Range, Disposable } from 'vscode';
67
import { Message } from 'vscode-jsonrpc';
78
import { traceError, traceInfo, traceLog, traceVerbose } from '../../../logging';
@@ -502,3 +503,32 @@ export function argKeyExists(args: string[], key: string): boolean {
502503
}
503504
return false;
504505
}
506+
507+
/**
508+
* Checks recursively if any parent directories of the given path are symbolic links.
509+
* @param {string} currentPath - The path to start checking from.
510+
* @returns {Promise<boolean>} - Returns true if any parent directory is a symlink, otherwise false.
511+
*/
512+
export async function hasSymlinkParent(currentPath: string): Promise<boolean> {
513+
try {
514+
// Resolve the path to an absolute path
515+
const absolutePath = path.resolve(currentPath);
516+
// Get the parent directory
517+
const parentDirectory = path.dirname(absolutePath);
518+
// Check if the current directory is the root directory
519+
if (parentDirectory === absolutePath) {
520+
return false;
521+
}
522+
// Check if the parent directory is a symlink
523+
const stats = await fs.promises.lstat(parentDirectory);
524+
if (stats.isSymbolicLink()) {
525+
traceLog(`Symlink found at: ${parentDirectory}`);
526+
return true;
527+
}
528+
// Recurse up the directory tree
529+
return await hasSymlinkParent(parentDirectory);
530+
} catch (error) {
531+
console.error('Error checking symlinks:', error);
532+
return false;
533+
}
534+
}

extensions/positron-python/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
fixLogLinesNoTrailing,
2222
startDiscoveryNamedPipe,
2323
addValueIfKeyNotExist,
24+
hasSymlinkParent,
2425
} from '../common/utils';
2526
import { IEnvironmentVariablesProvider } from '../../../common/variables/types';
2627

@@ -67,9 +68,20 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
6768
const cwd = settings.testing.cwd && settings.testing.cwd.length > 0 ? settings.testing.cwd : uri.fsPath;
6869

6970
// check for symbolic path
70-
const stats = fs.lstatSync(cwd);
71+
const stats = await fs.promises.lstat(cwd);
72+
const resolvedPath = await fs.promises.realpath(cwd);
73+
let isSymbolicLink = false;
7174
if (stats.isSymbolicLink()) {
72-
traceWarn("The cwd is a symbolic link, adding '--rootdir' to pytestArgs only if it doesn't already exist.");
75+
isSymbolicLink = true;
76+
traceWarn('The cwd is a symbolic link.');
77+
} else if (resolvedPath !== cwd) {
78+
traceWarn(
79+
'The cwd resolves to a different path, checking if it has a symbolic link somewhere in its path.',
80+
);
81+
isSymbolicLink = await hasSymlinkParent(cwd);
82+
}
83+
if (isSymbolicLink) {
84+
traceWarn("Symlink found, adding '--rootdir' to pytestArgs only if it doesn't already exist. cwd: ", cwd);
7385
pytestArgs = addValueIfKeyNotExist(pytestArgs, '--rootdir', cwd);
7486
}
7587

extensions/positron-python/src/client/testing/testController/unittest/testExecutionAdapter.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,7 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter {
9090
} finally {
9191
// wait for EOT
9292
await deferredTillEOT.promise;
93-
console.log('deferredTill EOT resolved');
9493
await deferredTillServerClose.promise;
95-
console.log('Server closed await now resolved');
9694
}
9795
const executionPayload: ExecutionTestPayload = {
9896
cwd: uri.fsPath,

extensions/positron-python/src/test/testing/common/testingAdapter.test.ts

Lines changed: 126 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import * as typeMoq from 'typemoq';
66
import * as path from 'path';
77
import * as assert from 'assert';
88
import * as fs from 'fs';
9+
import * as os from 'os';
910
import { PytestTestDiscoveryAdapter } from '../../../client/testing/testController/pytest/pytestDiscoveryAdapter';
1011
import { ITestController, ITestResultResolver } from '../../../client/testing/testController/common/types';
1112
import { IPythonExecutionFactory } from '../../../client/common/process/types';
@@ -62,6 +63,13 @@ suite('End to End Tests: test adapters', () => {
6263
'testTestingRootWkspc',
6364
'symlinkWorkspace',
6465
);
66+
const nestedTarget = path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'testTestingRootWkspc', 'target workspace');
67+
const nestedSymlink = path.join(
68+
EXTENSION_ROOT_DIR_FOR_TESTS,
69+
'src',
70+
'testTestingRootWkspc',
71+
'symlink_parent-folder',
72+
);
6573
suiteSetup(async () => {
6674
serviceContainer = (await initialize()).serviceContainer;
6775

@@ -73,7 +81,14 @@ suite('End to End Tests: test adapters', () => {
7381
if (err) {
7482
traceError(err);
7583
} else {
76-
traceLog('Symlink created successfully for end to end tests.');
84+
traceLog('Symlink created successfully for regular symlink end to end tests.');
85+
}
86+
});
87+
fs.symlink(nestedTarget, nestedSymlink, 'dir', (err) => {
88+
if (err) {
89+
traceError(err);
90+
} else {
91+
traceLog('Symlink created successfully for nested symlink end to end tests.');
7792
}
7893
});
7994
} catch (err) {
@@ -116,11 +131,23 @@ suite('End to End Tests: test adapters', () => {
116131
if (err) {
117132
traceError(err);
118133
} else {
119-
traceLog('Symlink removed successfully after tests.');
134+
traceLog('Symlink removed successfully after tests, rootPathDiscoverySymlink.');
135+
}
136+
});
137+
} else {
138+
traceLog('Symlink was not found to remove after tests, exiting successfully, rootPathDiscoverySymlink.');
139+
}
140+
141+
if (fs.existsSync(nestedSymlink)) {
142+
fs.unlink(nestedSymlink, (err) => {
143+
if (err) {
144+
traceError(err);
145+
} else {
146+
traceLog('Symlink removed successfully after tests, nestedSymlink.');
120147
}
121148
});
122149
} else {
123-
traceLog('Symlink was not found to remove after tests, exiting successfully');
150+
traceLog('Symlink was not found to remove after tests, exiting successfully, nestedSymlink.');
124151
}
125152
});
126153
test('unittest discovery adapter small workspace', async () => {
@@ -256,7 +283,103 @@ suite('End to End Tests: test adapters', () => {
256283
assert.strictEqual(callCount, 1, 'Expected _resolveDiscovery to be called once');
257284
});
258285
});
286+
test('pytest discovery adapter nested symlink', async () => {
287+
if (os.platform() === 'win32') {
288+
console.log('Skipping test for windows');
289+
return;
290+
}
291+
292+
// result resolver and saved data for assertions
293+
let actualData: {
294+
cwd: string;
295+
tests?: unknown;
296+
status: 'success' | 'error';
297+
error?: string[];
298+
};
299+
// set workspace to test workspace folder
300+
const workspacePath = path.join(nestedSymlink, 'custom_sub_folder');
301+
const workspacePathParent = nestedSymlink;
302+
workspaceUri = Uri.parse(workspacePath);
303+
const filePath = path.join(workspacePath, 'test_simple.py');
304+
const stats = fs.lstatSync(workspacePathParent);
305+
306+
// confirm that the path is a symbolic link
307+
assert.ok(stats.isSymbolicLink(), 'The PARENT path is not a symbolic link but must be for this test.');
308+
309+
resultResolver = new PythonResultResolver(testController, pytestProvider, workspaceUri);
310+
let callCount = 0;
311+
resultResolver._resolveDiscovery = async (payload, _token?) => {
312+
traceLog(`resolveDiscovery ${payload}`);
313+
callCount = callCount + 1;
314+
actualData = payload;
315+
return Promise.resolve();
316+
};
317+
// run pytest discovery
318+
const discoveryAdapter = new PytestTestDiscoveryAdapter(
319+
configService,
320+
testOutputChannel.object,
321+
resultResolver,
322+
envVarsService,
323+
);
324+
configService.getSettings(workspaceUri).testing.pytestArgs = [];
325+
326+
await discoveryAdapter.discoverTests(workspaceUri, pythonExecFactory).finally(() => {
327+
// verification after discovery is complete
328+
329+
// 1. Check the status is "success"
330+
assert.strictEqual(
331+
actualData.status,
332+
'success',
333+
`Expected status to be 'success' instead status is ${actualData.status}`,
334+
); // 2. Confirm no errors
335+
assert.strictEqual(actualData.error?.length, 0, "Expected no errors in 'error' field");
336+
// 3. Confirm tests are found
337+
assert.ok(actualData.tests, 'Expected tests to be present');
338+
// 4. Confirm that the cwd returned is the symlink path and the test's path is also using the symlink as the root
339+
if (process.platform === 'win32') {
340+
// covert string to lowercase for windows as the path is case insensitive
341+
traceLog('windows machine detected, converting path to lowercase for comparison');
342+
const a = actualData.cwd.toLowerCase();
343+
const b = filePath.toLowerCase();
344+
const testSimpleActual = (actualData.tests as {
345+
children: {
346+
path: string;
347+
}[];
348+
}).children[0].path.toLowerCase();
349+
const testSimpleExpected = filePath.toLowerCase();
350+
assert.strictEqual(a, b, `Expected cwd to be the symlink path actual: ${a} expected: ${b}`);
351+
assert.strictEqual(
352+
testSimpleActual,
353+
testSimpleExpected,
354+
`Expected test path to be the symlink path actual: ${testSimpleActual} expected: ${testSimpleExpected}`,
355+
);
356+
} else {
357+
assert.strictEqual(
358+
path.join(actualData.cwd),
359+
path.join(workspacePath),
360+
'Expected cwd to be the symlink path, check for non-windows machines',
361+
);
362+
assert.strictEqual(
363+
(actualData.tests as {
364+
children: {
365+
path: string;
366+
}[];
367+
}).children[0].path,
368+
filePath,
369+
'Expected test path to be the symlink path, check for non windows machines',
370+
);
371+
}
372+
373+
// 5. Confirm that resolveDiscovery was called once
374+
assert.strictEqual(callCount, 1, 'Expected _resolveDiscovery to be called once');
375+
});
376+
});
259377
test('pytest discovery adapter small workspace with symlink', async () => {
378+
if (os.platform() === 'win32') {
379+
console.log('Skipping test for windows');
380+
return;
381+
}
382+
260383
// result resolver and saved data for assertions
261384
let actualData: {
262385
cwd: string;

0 commit comments

Comments
 (0)