diff --git a/extensions/positron-python/python_files/tests/pytestadapter/test_utils.py b/extensions/positron-python/python_files/tests/pytestadapter/test_utils.py new file mode 100644 index 000000000000..9a1a58376ad8 --- /dev/null +++ b/extensions/positron-python/python_files/tests/pytestadapter/test_utils.py @@ -0,0 +1,35 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +import pathlib +import tempfile +import os +import sys + +from .helpers import ( # noqa: E402 + TEST_DATA_PATH, +) + +script_dir = pathlib.Path(__file__).parent.parent.parent +sys.path.append(os.fspath(script_dir)) +from vscode_pytest import has_symlink_parent # noqa: E402 + + +def test_has_symlink_parent_with_symlink(): + # Create a temporary directory and a file in it + with tempfile.TemporaryDirectory() as temp_dir: + file_path = pathlib.Path(temp_dir) / "file" + file_path.touch() + + # Create a symbolic link to the temporary directory + symlink_path = pathlib.Path(temp_dir) / "symlink" + symlink_path.symlink_to(temp_dir) + + # Check that has_symlink_parent correctly identifies the symbolic link + assert has_symlink_parent(symlink_path / "file") + + +def test_has_symlink_parent_without_symlink(): + folder_path = TEST_DATA_PATH / "unittest_folder" / "test_add.py" + # Check that has_symlink_parent correctly identifies that there are no symbolic links + assert not has_symlink_parent(folder_path) diff --git a/extensions/positron-python/python_files/vscode_pytest/__init__.py b/extensions/positron-python/python_files/vscode_pytest/__init__.py index 1a8b81a95835..ed5dc6299c46 100644 --- a/extensions/positron-python/python_files/vscode_pytest/__init__.py +++ b/extensions/positron-python/python_files/vscode_pytest/__init__.py @@ -79,13 +79,21 @@ def pytest_load_initial_conftests(early_config, parser, args): raise VSCodePytestError( f"The path set in the argument --rootdir={rootdir} does not exist." ) - if ( - os.path.islink(rootdir) - and pathlib.Path(os.path.realpath(rootdir)) == pathlib.Path.cwd() - ): + + # Check if the rootdir is a symlink or a child of a symlink to the current cwd. + isSymlink = False + + if os.path.islink(rootdir): + isSymlink = True print( - f"Plugin info[vscode-pytest]: rootdir argument, {rootdir}, is identified as a symlink to the cwd, {pathlib.Path.cwd()}.", - "Therefore setting symlink path to rootdir argument.", + f"Plugin info[vscode-pytest]: rootdir argument, {rootdir}, is identified as a symlink." + ) + elif pathlib.Path(os.path.realpath(rootdir)) != rootdir: + print("Plugin info[vscode-pytest]: Checking if rootdir is a child of a symlink.") + isSymlink = has_symlink_parent(rootdir) + if isSymlink: + print( + f"Plugin info[vscode-pytest]: rootdir argument, {rootdir}, is identified as a symlink or child of a symlink, adjusting pytest paths accordingly.", ) global SYMLINK_PATH SYMLINK_PATH = pathlib.Path(rootdir) @@ -144,6 +152,21 @@ def pytest_exception_interact(node, call, report): ) +def has_symlink_parent(current_path): + """Recursively checks if any parent directories of the given path are symbolic links.""" + # Convert the current path to an absolute Path object + curr_path = pathlib.Path(current_path) + print("Checking for symlink parent starting at current path: ", curr_path) + + # Iterate over all parent directories + for parent in curr_path.parents: + # Check if the parent directory is a symlink + if os.path.islink(parent): + print(f"Symlink found at: {parent}") + return True + return False + + def get_absolute_test_id(test_id: str, testPath: pathlib.Path) -> str: """A function that returns the absolute test id. This is necessary because testIds are relative to the rootdir. This does not work for our case since testIds when referenced during run time are relative to the instantiation diff --git a/extensions/positron-python/src/client/testing/testController/common/utils.ts b/extensions/positron-python/src/client/testing/testController/common/utils.ts index aa7a5d115246..759fb0713de4 100644 --- a/extensions/positron-python/src/client/testing/testController/common/utils.ts +++ b/extensions/positron-python/src/client/testing/testController/common/utils.ts @@ -2,6 +2,7 @@ // Licensed under the MIT License. import * as net from 'net'; import * as path from 'path'; +import * as fs from 'fs'; import { CancellationToken, Position, TestController, TestItem, Uri, Range, Disposable } from 'vscode'; import { Message } from 'vscode-jsonrpc'; import { traceError, traceInfo, traceLog, traceVerbose } from '../../../logging'; @@ -502,3 +503,32 @@ export function argKeyExists(args: string[], key: string): boolean { } return false; } + +/** + * Checks recursively if any parent directories of the given path are symbolic links. + * @param {string} currentPath - The path to start checking from. + * @returns {Promise} - Returns true if any parent directory is a symlink, otherwise false. + */ +export async function hasSymlinkParent(currentPath: string): Promise { + try { + // Resolve the path to an absolute path + const absolutePath = path.resolve(currentPath); + // Get the parent directory + const parentDirectory = path.dirname(absolutePath); + // Check if the current directory is the root directory + if (parentDirectory === absolutePath) { + return false; + } + // Check if the parent directory is a symlink + const stats = await fs.promises.lstat(parentDirectory); + if (stats.isSymbolicLink()) { + traceLog(`Symlink found at: ${parentDirectory}`); + return true; + } + // Recurse up the directory tree + return await hasSymlinkParent(parentDirectory); + } catch (error) { + console.error('Error checking symlinks:', error); + return false; + } +} diff --git a/extensions/positron-python/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts b/extensions/positron-python/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts index eb4fae6b03d2..3d26f77a121b 100644 --- a/extensions/positron-python/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts +++ b/extensions/positron-python/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts @@ -21,6 +21,7 @@ import { fixLogLinesNoTrailing, startDiscoveryNamedPipe, addValueIfKeyNotExist, + hasSymlinkParent, } from '../common/utils'; import { IEnvironmentVariablesProvider } from '../../../common/variables/types'; @@ -67,9 +68,20 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter { const cwd = settings.testing.cwd && settings.testing.cwd.length > 0 ? settings.testing.cwd : uri.fsPath; // check for symbolic path - const stats = fs.lstatSync(cwd); + const stats = await fs.promises.lstat(cwd); + const resolvedPath = await fs.promises.realpath(cwd); + let isSymbolicLink = false; if (stats.isSymbolicLink()) { - traceWarn("The cwd is a symbolic link, adding '--rootdir' to pytestArgs only if it doesn't already exist."); + isSymbolicLink = true; + traceWarn('The cwd is a symbolic link.'); + } else if (resolvedPath !== cwd) { + traceWarn( + 'The cwd resolves to a different path, checking if it has a symbolic link somewhere in its path.', + ); + isSymbolicLink = await hasSymlinkParent(cwd); + } + if (isSymbolicLink) { + traceWarn("Symlink found, adding '--rootdir' to pytestArgs only if it doesn't already exist. cwd: ", cwd); pytestArgs = addValueIfKeyNotExist(pytestArgs, '--rootdir', cwd); } diff --git a/extensions/positron-python/src/client/testing/testController/unittest/testExecutionAdapter.ts b/extensions/positron-python/src/client/testing/testController/unittest/testExecutionAdapter.ts index edcfbfef9b63..4746c3101752 100644 --- a/extensions/positron-python/src/client/testing/testController/unittest/testExecutionAdapter.ts +++ b/extensions/positron-python/src/client/testing/testController/unittest/testExecutionAdapter.ts @@ -90,9 +90,7 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter { } finally { // wait for EOT await deferredTillEOT.promise; - console.log('deferredTill EOT resolved'); await deferredTillServerClose.promise; - console.log('Server closed await now resolved'); } const executionPayload: ExecutionTestPayload = { cwd: uri.fsPath, diff --git a/extensions/positron-python/src/test/testing/common/testingAdapter.test.ts b/extensions/positron-python/src/test/testing/common/testingAdapter.test.ts index 2dc7386c5176..06b8546be18e 100644 --- a/extensions/positron-python/src/test/testing/common/testingAdapter.test.ts +++ b/extensions/positron-python/src/test/testing/common/testingAdapter.test.ts @@ -6,6 +6,7 @@ import * as typeMoq from 'typemoq'; import * as path from 'path'; import * as assert from 'assert'; import * as fs from 'fs'; +import * as os from 'os'; import { PytestTestDiscoveryAdapter } from '../../../client/testing/testController/pytest/pytestDiscoveryAdapter'; import { ITestController, ITestResultResolver } from '../../../client/testing/testController/common/types'; import { IPythonExecutionFactory } from '../../../client/common/process/types'; @@ -62,6 +63,13 @@ suite('End to End Tests: test adapters', () => { 'testTestingRootWkspc', 'symlinkWorkspace', ); + const nestedTarget = path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'testTestingRootWkspc', 'target workspace'); + const nestedSymlink = path.join( + EXTENSION_ROOT_DIR_FOR_TESTS, + 'src', + 'testTestingRootWkspc', + 'symlink_parent-folder', + ); suiteSetup(async () => { serviceContainer = (await initialize()).serviceContainer; @@ -73,7 +81,14 @@ suite('End to End Tests: test adapters', () => { if (err) { traceError(err); } else { - traceLog('Symlink created successfully for end to end tests.'); + traceLog('Symlink created successfully for regular symlink end to end tests.'); + } + }); + fs.symlink(nestedTarget, nestedSymlink, 'dir', (err) => { + if (err) { + traceError(err); + } else { + traceLog('Symlink created successfully for nested symlink end to end tests.'); } }); } catch (err) { @@ -116,11 +131,23 @@ suite('End to End Tests: test adapters', () => { if (err) { traceError(err); } else { - traceLog('Symlink removed successfully after tests.'); + traceLog('Symlink removed successfully after tests, rootPathDiscoverySymlink.'); + } + }); + } else { + traceLog('Symlink was not found to remove after tests, exiting successfully, rootPathDiscoverySymlink.'); + } + + if (fs.existsSync(nestedSymlink)) { + fs.unlink(nestedSymlink, (err) => { + if (err) { + traceError(err); + } else { + traceLog('Symlink removed successfully after tests, nestedSymlink.'); } }); } else { - traceLog('Symlink was not found to remove after tests, exiting successfully'); + traceLog('Symlink was not found to remove after tests, exiting successfully, nestedSymlink.'); } }); test('unittest discovery adapter small workspace', async () => { @@ -256,7 +283,103 @@ suite('End to End Tests: test adapters', () => { assert.strictEqual(callCount, 1, 'Expected _resolveDiscovery to be called once'); }); }); + test('pytest discovery adapter nested symlink', async () => { + if (os.platform() === 'win32') { + console.log('Skipping test for windows'); + return; + } + + // result resolver and saved data for assertions + let actualData: { + cwd: string; + tests?: unknown; + status: 'success' | 'error'; + error?: string[]; + }; + // set workspace to test workspace folder + const workspacePath = path.join(nestedSymlink, 'custom_sub_folder'); + const workspacePathParent = nestedSymlink; + workspaceUri = Uri.parse(workspacePath); + const filePath = path.join(workspacePath, 'test_simple.py'); + const stats = fs.lstatSync(workspacePathParent); + + // confirm that the path is a symbolic link + assert.ok(stats.isSymbolicLink(), 'The PARENT path is not a symbolic link but must be for this test.'); + + resultResolver = new PythonResultResolver(testController, pytestProvider, workspaceUri); + let callCount = 0; + resultResolver._resolveDiscovery = async (payload, _token?) => { + traceLog(`resolveDiscovery ${payload}`); + callCount = callCount + 1; + actualData = payload; + return Promise.resolve(); + }; + // run pytest discovery + const discoveryAdapter = new PytestTestDiscoveryAdapter( + configService, + testOutputChannel.object, + resultResolver, + envVarsService, + ); + configService.getSettings(workspaceUri).testing.pytestArgs = []; + + await discoveryAdapter.discoverTests(workspaceUri, pythonExecFactory).finally(() => { + // verification after discovery is complete + + // 1. Check the status is "success" + assert.strictEqual( + actualData.status, + 'success', + `Expected status to be 'success' instead status is ${actualData.status}`, + ); // 2. Confirm no errors + assert.strictEqual(actualData.error?.length, 0, "Expected no errors in 'error' field"); + // 3. Confirm tests are found + assert.ok(actualData.tests, 'Expected tests to be present'); + // 4. Confirm that the cwd returned is the symlink path and the test's path is also using the symlink as the root + if (process.platform === 'win32') { + // covert string to lowercase for windows as the path is case insensitive + traceLog('windows machine detected, converting path to lowercase for comparison'); + const a = actualData.cwd.toLowerCase(); + const b = filePath.toLowerCase(); + const testSimpleActual = (actualData.tests as { + children: { + path: string; + }[]; + }).children[0].path.toLowerCase(); + const testSimpleExpected = filePath.toLowerCase(); + assert.strictEqual(a, b, `Expected cwd to be the symlink path actual: ${a} expected: ${b}`); + assert.strictEqual( + testSimpleActual, + testSimpleExpected, + `Expected test path to be the symlink path actual: ${testSimpleActual} expected: ${testSimpleExpected}`, + ); + } else { + assert.strictEqual( + path.join(actualData.cwd), + path.join(workspacePath), + 'Expected cwd to be the symlink path, check for non-windows machines', + ); + assert.strictEqual( + (actualData.tests as { + children: { + path: string; + }[]; + }).children[0].path, + filePath, + 'Expected test path to be the symlink path, check for non windows machines', + ); + } + + // 5. Confirm that resolveDiscovery was called once + assert.strictEqual(callCount, 1, 'Expected _resolveDiscovery to be called once'); + }); + }); test('pytest discovery adapter small workspace with symlink', async () => { + if (os.platform() === 'win32') { + console.log('Skipping test for windows'); + return; + } + // result resolver and saved data for assertions let actualData: { cwd: string; diff --git a/extensions/positron-python/src/test/testing/testController/pytest/pytestDiscoveryAdapter.unit.test.ts b/extensions/positron-python/src/test/testing/testController/pytest/pytestDiscoveryAdapter.unit.test.ts index ab41ef12b726..d1587f59bd2d 100644 --- a/extensions/positron-python/src/test/testing/testController/pytest/pytestDiscoveryAdapter.unit.test.ts +++ b/extensions/positron-python/src/test/testing/testController/pytest/pytestDiscoveryAdapter.unit.test.ts @@ -103,7 +103,15 @@ suite('pytest test discovery adapter', () => { return Promise.resolve(execService.object); }); - sinon.stub(fs, 'lstatSync').returns({ isFile: () => true, isSymbolicLink: () => false } as fs.Stats); + sinon.stub(fs.promises, 'lstat').callsFake( + async () => + ({ + isFile: () => true, + isSymbolicLink: () => false, + } as fs.Stats), + ); + sinon.stub(fs.promises, 'realpath').callsFake(async (pathEntered) => pathEntered.toString()); + adapter = new PytestTestDiscoveryAdapter(configService, outputChannel.object); adapter.discoverTests(uri, execFactory.object); // add in await and trigger @@ -143,7 +151,14 @@ suite('pytest test discovery adapter', () => { }), } as unknown) as IConfigurationService; - sinon.stub(fs, 'lstatSync').returns({ isFile: () => true, isSymbolicLink: () => false } as fs.Stats); + sinon.stub(fs.promises, 'lstat').callsFake( + async () => + ({ + isFile: () => true, + isSymbolicLink: () => false, + } as fs.Stats), + ); + sinon.stub(fs.promises, 'realpath').callsFake(async (pathEntered) => pathEntered.toString()); // set up exec mock deferred = createDeferred(); @@ -180,10 +195,82 @@ suite('pytest test discovery adapter', () => { ); }); test('Test discovery adds cwd to pytest args when path is symlink', async () => { - sinon.stub(fs, 'lstatSync').returns({ - isFile: () => true, - isSymbolicLink: () => true, - } as fs.Stats); + sinon.stub(fs.promises, 'lstat').callsFake( + async () => + ({ + isFile: () => true, + isSymbolicLink: () => true, + } as fs.Stats), + ); + sinon.stub(fs.promises, 'realpath').callsFake(async (pathEntered) => pathEntered.toString()); + + // set up a config service with different pytest args + const configServiceNew: IConfigurationService = ({ + getSettings: () => ({ + testing: { + pytestArgs: ['.', 'abc', 'xyz'], + cwd: expectedPath, + }, + }), + } as unknown) as IConfigurationService; + + // set up exec mock + deferred = createDeferred(); + execFactory = typeMoq.Mock.ofType(); + execFactory + .setup((x) => x.createActivatedEnvironment(typeMoq.It.isAny())) + .returns(() => { + deferred.resolve(); + return Promise.resolve(execService.object); + }); + + adapter = new PytestTestDiscoveryAdapter(configServiceNew, outputChannel.object); + adapter.discoverTests(uri, execFactory.object); + // add in await and trigger + await deferred.promise; + await deferred2.promise; + mockProc.trigger('close'); + + // verification + const expectedArgs = [ + '-m', + 'pytest', + '-p', + 'vscode_pytest', + '--collect-only', + '.', + 'abc', + 'xyz', + `--rootdir=${expectedPath}`, + ]; + execService.verify( + (x) => + x.execObservable( + expectedArgs, + typeMoq.It.is((options) => { + assert.deepEqual(options.env, expectedExtraVariables); + assert.equal(options.cwd, expectedPath); + assert.equal(options.throwOnStdErr, true); + return true; + }), + ), + typeMoq.Times.once(), + ); + }); + test('Test discovery adds cwd to pytest args when path parent is symlink', async () => { + let counter = 0; + sinon.stub(fs.promises, 'lstat').callsFake( + async () => + ({ + isFile: () => true, + isSymbolicLink: () => { + counter = counter + 1; + return counter > 2; + }, + } as fs.Stats), + ); + + sinon.stub(fs.promises, 'realpath').callsFake(async () => 'diff value'); // set up a config service with different pytest args const configServiceNew: IConfigurationService = ({ diff --git a/extensions/positron-python/src/testTestingRootWkspc/target workspace/custom_sub_folder/test_simple.py b/extensions/positron-python/src/testTestingRootWkspc/target workspace/custom_sub_folder/test_simple.py new file mode 100644 index 000000000000..179d6420c76f --- /dev/null +++ b/extensions/positron-python/src/testTestingRootWkspc/target workspace/custom_sub_folder/test_simple.py @@ -0,0 +1,8 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. +import unittest + + +class SimpleClass(unittest.TestCase): + def test_simple_unit(self): + assert True