${workspaceFolder} & . not being resolved in settings in some cases

[karrtikr]

Resolving ${workspaceFolder}

Having

     "python.testing.unittestArgs": [
        "-p",
        "*test.py",
        "-s",
        "${workspaceFolder}/src",
    ]

instead of

     "python.testing.unittestArgs": [
        "-p",
        "*test.py",
        "-s",
        "./src",
    ]

doesn't work.

Resolving .

This problem arises when using forward slashes in path.

"python.pythonPath": "./.venv/Scripts/python.exe" doesn't work but "python.pythonPath": ".\\.venv\\Scripts\\python.exe" does.

     "python.testing.unittestArgs": [
        "-p",
        "*test.py",
        "-s",
        "./src",
    ],
    "python.testing.cwd": "./test"

doesn't search for tests in ./test/src directory

[karrtikr]

This problem arises when using forward slashes in path.

For this particular case, we use child_process.exec which uses cmd shells to run ./.venv/Scripts/python.exe which fails, hence we're not able to resolve relative paths with forward slashes. However using full path with forward slashes work. Two possible solutions,

• Use child_process.spawn to run commands which return the result, which apparently doesn't use shell (?) cc @int19h
• Resolve all relative paths to absolute before using them. This is already being done, except for cases where the relative path contains forward slashes on Windows.

vscode-python/src/client/common/configSettings.ts

Lines 641 to 654 in 620b614

	function getAbsolutePath(pathToCheck: string, rootDir: string | undefined): string {
	if (!rootDir) {
	rootDir = __dirname;
	}
	// tslint:disable-next-line:prefer-type-cast no-unsafe-any
	pathToCheck = untildify(pathToCheck) as string;
	if (isTestExecution() && !pathToCheck) {
	return rootDir;
	}
	if (pathToCheck.indexOf(path.sep) === -1) {
	return pathToCheck;
	}
	return path.isAbsolute(pathToCheck) ? pathToCheck : path.resolve(rootDir, pathToCheck);
	}

We can change this line

if (pathToCheck.indexOf(path.sep) === -1) {

to

if (pathToCheck.indexOf('\\') === -1 || pathToCheck.indexOf('/') === -1) {

[int19h]

The real problem here is command line escaping, or lack thereof. Briefly, in cmd.exe, for historical reasons, the following is valid:

del/s/q foo

and is the same as:

del /s /q foo

There are many convoluted rules there that affect this behavior, and it's not really worthwhile trying to figure them all out. We should just avoid APIs like exec() that take command line argument as a single string - those should do appropriate quoting.

So, basically, always either execFile() or spawn() (with shell: true if needed - it defaults to false for both). But exec() should be an extremely rare case - I doubt we have any scenarios where it's necessary. Any scenario that does justify it, also justifies a lengthy code comment explaining why it's needed :)

And shell: true should only be set if needed, because it's expensive to spawn shells, and they introduce more moving bits that can potentially fail. Like, in this case, it would be needed because we're running a batch script on Windows (but perhaps we can avoid that?).

[eleanorjboyd]

Tried this and it is working for me on the rewrite, please reopen if this is not the case and anyone thinks the problem still exists.

Thanks