From f4f883c6bf6503ad00c8664ed161ac17fc79e6d4 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 7 Apr 2023 17:33:44 -0700 Subject: [PATCH 1/6] Fix info needed workflow (#21021) --- .github/workflows/triage-info-needed.yml | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/.github/workflows/triage-info-needed.yml b/.github/workflows/triage-info-needed.yml index 3b4165c74c55..51c5b610f03f 100644 --- a/.github/workflows/triage-info-needed.yml +++ b/.github/workflows/triage-info-needed.yml @@ -14,14 +14,12 @@ jobs: env: KEYWORDS: '["\\?", "please", "kindly", "let me know", "try", "can you", "could you", "would you", "may I", "provide", "let us know", "tell me", "give me", "send me", "what", "when", "where", "why", "how"]' steps: - - name: Checkout code - uses: actions/checkout@v3 - name: Check for author uses: actions/github-script@v6 with: github-token: ${{ secrets.GITHUB_TOKEN }} script: | - const issue = await github.issues.get({ + const issue = await github.rest.issues.get({ owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number @@ -36,7 +34,7 @@ jobs: const shouldAddLabel = isTeamMember && commentAuthor !== issue.data.user.login && isRequestForInfo; if (shouldAddLabel) { - await github.issues.addLabels({ + await github.rest.issues.addLabels({ owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number, @@ -48,14 +46,12 @@ jobs: if: contains(github.event.issue.labels.*.name, 'info-needed') && contains(github.event.issue.labels.*.name, 'triage-needed') runs-on: ubuntu-latest steps: - - name: Checkout code - uses: actions/checkout@v3 - name: Check for author uses: actions/github-script@v6 with: github-token: ${{ secrets.GITHUB_TOKEN }} script: | - const issue = await github.issues.get({ + const issue = await github.rest.issues.get({ owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number @@ -63,7 +59,7 @@ jobs: const commentAuthor = context.payload.comment.user.login; const issueAuthor = issue.data.user.login; if (commentAuthor === issueAuthor) { - await github.issues.removeLabel({ + await github.rest.issues.removeLabel({ owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number, @@ -77,7 +73,7 @@ jobs: } // Loop through all the comments on the issue in reverse order and find the last username that a TRIAGER mentioned // If the comment author is the last mentioned username, remove the "info-needed" label - const comments = await github.issues.listComments({ + const comments = await github.rest.issues.listComments({ owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number @@ -89,7 +85,7 @@ jobs: const matches = comment.body.match(/@\w+/g) || []; const mentionedUsernames = matches.map(match => match.replace('@', '')); if (mentionedUsernames.includes(commentAuthor)) { - await github.issues.removeLabel({ + await github.rest.issues.removeLabel({ owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number, From c64bb0e5403c41b951501f786460fe7b92d65511 Mon Sep 17 00:00:00 2001 From: Eleanor Boyd Date: Mon, 10 Apr 2023 10:00:02 -0700 Subject: [PATCH 2/6] Add Testing for PytestExecutionAdapter (#21019) --- .../pytestExecutionAdapter.unit.test.ts | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 src/test/testing/testController/pytest/pytestExecutionAdapter.unit.test.ts diff --git a/src/test/testing/testController/pytest/pytestExecutionAdapter.unit.test.ts b/src/test/testing/testController/pytest/pytestExecutionAdapter.unit.test.ts new file mode 100644 index 000000000000..ac6c6bd274a4 --- /dev/null +++ b/src/test/testing/testController/pytest/pytestExecutionAdapter.unit.test.ts @@ -0,0 +1,90 @@ +// /* eslint-disable @typescript-eslint/no-explicit-any */ +// // Copyright (c) Microsoft Corporation. All rights reserved. +// // Licensed under the MIT License. +// import * as assert from 'assert'; +// import { Uri } from 'vscode'; +// import * as typeMoq from 'typemoq'; +// import { IConfigurationService } from '../../../../client/common/types'; +// import { DataReceivedEvent, ITestServer } from '../../../../client/testing/testController/common/types'; +// import { IPythonExecutionFactory, IPythonExecutionService } from '../../../../client/common/process/types'; +// import { createDeferred, Deferred } from '../../../../client/common/utils/async'; +// import { PytestTestExecutionAdapter } from '../../../../client/testing/testController/pytest/pytestExecutionAdapter'; + +// suite('pytest test execution adapter', () => { +// let testServer: typeMoq.IMock; +// let configService: IConfigurationService; +// let execFactory = typeMoq.Mock.ofType(); +// let adapter: PytestTestExecutionAdapter; +// let execService: typeMoq.IMock; +// let deferred: Deferred; +// setup(() => { +// testServer = typeMoq.Mock.ofType(); +// testServer.setup((t) => t.getPort()).returns(() => 12345); +// testServer +// .setup((t) => t.onDataReceived(typeMoq.It.isAny(), typeMoq.It.isAny())) +// .returns(() => ({ +// dispose: () => { +// /* no-body */ +// }, +// })); +// configService = ({ +// getSettings: () => ({ +// testing: { pytestArgs: ['.'] }, +// }), +// isTestExecution: () => false, +// } as unknown) as IConfigurationService; +// execFactory = typeMoq.Mock.ofType(); +// execService = typeMoq.Mock.ofType(); +// execFactory +// .setup((x) => x.createActivatedEnvironment(typeMoq.It.isAny())) +// .returns(() => Promise.resolve(execService.object)); +// deferred = createDeferred(); +// execService +// .setup((x) => x.exec(typeMoq.It.isAny(), typeMoq.It.isAny())) +// .returns(() => { +// deferred.resolve(); +// return Promise.resolve({ stdout: '{}' }); +// }); +// execFactory.setup((p) => ((p as unknown) as any).then).returns(() => undefined); +// execService.setup((p) => ((p as unknown) as any).then).returns(() => undefined); +// }); +// test('onDataReceivedHandler should parse only if known UUID', async () => { +// const uri = Uri.file('/my/test/path/'); +// const uuid = 'uuid123'; +// const data = { status: 'success' }; +// testServer.setup((t) => t.createUUID(typeMoq.It.isAny())).returns(() => uuid); +// const eventData: DataReceivedEvent = { +// uuid, +// data: JSON.stringify(data), +// }; + +// adapter = new PytestTestExecutionAdapter(testServer.object, configService); +// const promise = adapter.runTests(uri, [], false); +// await deferred.promise; +// adapter.onDataReceivedHandler(eventData); +// const result = await promise; +// assert.deepStrictEqual(result, data); +// }); +// test('onDataReceivedHandler should not parse if it is unknown UUID', async () => { +// const uri = Uri.file('/my/test/path/'); +// const uuid = 'uuid456'; +// let data = { status: 'error' }; +// testServer.setup((t) => t.createUUID(typeMoq.It.isAny())).returns(() => uuid); +// const wrongUriEventData: DataReceivedEvent = { +// uuid: 'incorrect-uuid456', +// data: JSON.stringify(data), +// }; +// adapter = new PytestTestExecutionAdapter(testServer.object, configService); +// const promise = adapter.runTests(uri, [], false); +// adapter.onDataReceivedHandler(wrongUriEventData); + +// data = { status: 'success' }; +// const correctUriEventData: DataReceivedEvent = { +// uuid, +// data: JSON.stringify(data), +// }; +// adapter.onDataReceivedHandler(correctUriEventData); +// const result = await promise; +// assert.deepStrictEqual(result, data); +// }); +// }); From 983f05aa34226432191cf8b019668d6d16441184 Mon Sep 17 00:00:00 2001 From: Eleanor Boyd Date: Mon, 10 Apr 2023 14:17:36 -0700 Subject: [PATCH 3/6] Add testing for unittest execution python logic (#21022) these tests cover: - parsing execution args for unittest - test run with no test_ids attached - test run with single test_id and test is a success. --------- Co-authored-by: Karthik Nadig --- .../tests/unittestadapter/test_execution.py | 94 +++++++++++++++++++ pythonFiles/unittestadapter/execution.py | 4 +- 2 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 pythonFiles/tests/unittestadapter/test_execution.py diff --git a/pythonFiles/tests/unittestadapter/test_execution.py b/pythonFiles/tests/unittestadapter/test_execution.py new file mode 100644 index 000000000000..14ddefa48d52 --- /dev/null +++ b/pythonFiles/tests/unittestadapter/test_execution.py @@ -0,0 +1,94 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +import os +import pathlib +from typing import List + +import pytest +from unittestadapter.execution import parse_execution_cli_args, run_tests + +TEST_DATA_PATH = pathlib.Path(__file__).parent / ".data" + + +@pytest.mark.parametrize( + "args, expected", + [ + ( + [ + "--port", + "111", + "--uuid", + "fake-uuid", + "--testids", + "test_file.test_class.test_method", + ], + (111, "fake-uuid", ["test_file.test_class.test_method"]), + ), + ( + ["--port", "111", "--uuid", "fake-uuid", "--testids", ""], + (111, "fake-uuid", [""]), + ), + ( + [ + "--port", + "111", + "--uuid", + "fake-uuid", + "--testids", + "test_file.test_class.test_method", + "-v", + "-s", + ], + (111, "fake-uuid", ["test_file.test_class.test_method"]), + ), + ], +) +def test_parse_execution_cli_args(args: List[str], expected: List[str]) -> None: + """The parse_execution_cli_args function should return values for the port, uuid, and testids arguments + when passed as command-line options, and ignore unrecognized arguments. + """ + actual = parse_execution_cli_args(args) + assert actual == expected + + +def test_no_ids_run() -> None: + """This test runs on an empty array of test_ids, therefore it should return + an empty dict for the result. + """ + start_dir: str = os.fspath(TEST_DATA_PATH) + testids = [] + pattern = "discovery_simple*" + actual = run_tests(start_dir, testids, pattern, None, "fake-uuid") + assert actual + assert all(item in actual for item in ("cwd", "status")) + assert actual["status"] == "success" + assert actual["cwd"] == os.fspath(TEST_DATA_PATH) + if "result" in actual: + assert len(actual["result"]) == 0 + else: + raise AssertionError("actual['result'] is None") + + +def test_single_ids_run() -> None: + """This test runs on a single test_id, therefore it should return + a dict with a single key-value pair for the result. + + This single test passes so the outcome should be 'success'. + """ + id = "discovery_simple.DiscoverySimple.test_one" + actual = run_tests( + os.fspath(TEST_DATA_PATH), [id], "discovery_simple*", None, "fake-uuid" + ) + assert actual + assert all(item in actual for item in ("cwd", "status")) + assert actual["status"] == "success" + assert actual["cwd"] == os.fspath(TEST_DATA_PATH) + assert "result" in actual + result = actual["result"] + assert len(result) == 1 + assert id in result + id_result = result[id] + assert id_result is not None + assert "outcome" in id_result + assert id_result["outcome"] == "success" diff --git a/pythonFiles/unittestadapter/execution.py b/pythonFiles/unittestadapter/execution.py index 6075a1480acb..569ad7fdf298 100644 --- a/pythonFiles/unittestadapter/execution.py +++ b/pythonFiles/unittestadapter/execution.py @@ -206,11 +206,11 @@ def run_tests( if error is not None: payload["error"] = error + else: + status = TestExecutionStatus.success payload["status"] = status - # print(f"payload: \n{json.dumps(payload, indent=4)}") - return payload From 92744f7d52d1c3eb4268177270050917650882a9 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 10 Apr 2023 18:49:57 -0700 Subject: [PATCH 4/6] Bump xml2js from 0.4.23 to 0.5.0 (#21032) Bumps [xml2js](https://github.com/Leonidas-from-XIV/node-xml2js) from 0.4.23 to 0.5.0.
Commits

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=xml2js&package-manager=npm_and_yarn&previous-version=0.4.23&new-version=0.5.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/microsoft/vscode-python/network/alerts).
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- package-lock.json | 80 ++++++++++++++++++++++++++++++++++++----------- package.json | 2 +- 2 files changed, 62 insertions(+), 20 deletions(-) diff --git a/package-lock.json b/package-lock.json index f445bf7f42e9..9b537745c9bd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -44,7 +44,7 @@ "vscode-tas-client": "^0.1.63", "which": "^2.0.2", "winreg": "^1.2.4", - "xml2js": "^0.4.19" + "xml2js": "^0.5.0" }, "devDependencies": { "@istanbuljs/nyc-config-typescript": "^1.0.2", @@ -221,6 +221,18 @@ "resolved": "https://registry.npmjs.org/tslib/-/tslib-2.4.1.tgz", "integrity": "sha512-tGyy4dAjRIEwI7BzsB0lynWgOpfqjUdq91XXAlIWD2OwKBH7oCl/GZG/HT4BOHrTlPMOASlMQ7veyTqpmRcrNA==" }, + "node_modules/@azure/core-http/node_modules/xml2js": { + "version": "0.4.23", + "resolved": "https://registry.npmjs.org/xml2js/-/xml2js-0.4.23.tgz", + "integrity": "sha512-ySPiMjM0+pLDftHgXY4By0uswI3SPKLDw/i3UXbnO8M/p28zqexCUoPmQFrYD+/1BzhGJSs2i1ERWKJAtiLrug==", + "dependencies": { + "sax": ">=0.6.0", + "xmlbuilder": "~11.0.0" + }, + "engines": { + "node": ">=4.0.0" + } + }, "node_modules/@azure/core-tracing": { "version": "1.0.0-preview.13", "resolved": "https://registry.npmjs.org/@azure/core-tracing/-/core-tracing-1.0.0-preview.13.tgz", @@ -11900,6 +11912,11 @@ "resolved": "https://registry.npmjs.org/safer-buffer/-/safer-buffer-2.1.2.tgz", "integrity": "sha512-YZo3K82SD7Riyi0E1EQPojLz7kpepnSQI9IyPbHHg1XXXevb5dJI7tpyN2ADxGcQbHG7vcyRHk0cbwqcQriUtg==" }, + "node_modules/sax": { + "version": "1.2.4", + "resolved": "https://registry.npmjs.org/sax/-/sax-1.2.4.tgz", + "integrity": "sha512-NqVDv9TpANUjFm0N8uM5GxL36UgKi9/atZw+x7YFnQ8ckwFGKrl4xX4yWtrey3UJm5nP1kUbnYgLopqWNSRhWw==" + }, "node_modules/schema-utils": { "version": "3.1.1", "resolved": "https://registry.npmjs.org/schema-utils/-/schema-utils-3.1.1.tgz", @@ -14270,6 +14287,19 @@ "node": ">=8.17.0" } }, + "node_modules/vsce/node_modules/xml2js": { + "version": "0.4.23", + "resolved": "https://registry.npmjs.org/xml2js/-/xml2js-0.4.23.tgz", + "integrity": "sha512-ySPiMjM0+pLDftHgXY4By0uswI3SPKLDw/i3UXbnO8M/p28zqexCUoPmQFrYD+/1BzhGJSs2i1ERWKJAtiLrug==", + "dev": true, + "dependencies": { + "sax": ">=0.6.0", + "xmlbuilder": "~11.0.0" + }, + "engines": { + "node": ">=4.0.0" + } + }, "node_modules/vscode-debugadapter": { "version": "1.35.0", "resolved": "https://registry.npmjs.org/vscode-debugadapter/-/vscode-debugadapter-1.35.0.tgz", @@ -14890,9 +14920,9 @@ "dev": true }, "node_modules/xml2js": { - "version": "0.4.23", - "resolved": "https://registry.npmjs.org/xml2js/-/xml2js-0.4.23.tgz", - "integrity": "sha512-ySPiMjM0+pLDftHgXY4By0uswI3SPKLDw/i3UXbnO8M/p28zqexCUoPmQFrYD+/1BzhGJSs2i1ERWKJAtiLrug==", + "version": "0.5.0", + "resolved": "https://registry.npmjs.org/xml2js/-/xml2js-0.5.0.tgz", + "integrity": "sha512-drPFnkQJik/O+uPKpqSgr22mpuFHqKdbS835iAQrUC73L2F5WkboIRd63ai/2Yg6I1jzifPFKH2NTK+cfglkIA==", "dependencies": { "sax": ">=0.6.0", "xmlbuilder": "~11.0.0" @@ -14901,11 +14931,6 @@ "node": ">=4.0.0" } }, - "node_modules/xml2js/node_modules/sax": { - "version": "1.2.4", - "resolved": "https://registry.npmjs.org/sax/-/sax-1.2.4.tgz", - "integrity": "sha512-NqVDv9TpANUjFm0N8uM5GxL36UgKi9/atZw+x7YFnQ8ckwFGKrl4xX4yWtrey3UJm5nP1kUbnYgLopqWNSRhWw==" - }, "node_modules/xmlbuilder": { "version": "11.0.1", "resolved": "https://registry.npmjs.org/xmlbuilder/-/xmlbuilder-11.0.1.tgz", @@ -15249,6 +15274,15 @@ "version": "2.4.1", "resolved": "https://registry.npmjs.org/tslib/-/tslib-2.4.1.tgz", "integrity": "sha512-tGyy4dAjRIEwI7BzsB0lynWgOpfqjUdq91XXAlIWD2OwKBH7oCl/GZG/HT4BOHrTlPMOASlMQ7veyTqpmRcrNA==" + }, + "xml2js": { + "version": "0.4.23", + "resolved": "https://registry.npmjs.org/xml2js/-/xml2js-0.4.23.tgz", + "integrity": "sha512-ySPiMjM0+pLDftHgXY4By0uswI3SPKLDw/i3UXbnO8M/p28zqexCUoPmQFrYD+/1BzhGJSs2i1ERWKJAtiLrug==", + "requires": { + "sax": ">=0.6.0", + "xmlbuilder": "~11.0.0" + } } } }, @@ -24487,6 +24521,11 @@ "resolved": "https://registry.npmjs.org/safer-buffer/-/safer-buffer-2.1.2.tgz", "integrity": "sha512-YZo3K82SD7Riyi0E1EQPojLz7kpepnSQI9IyPbHHg1XXXevb5dJI7tpyN2ADxGcQbHG7vcyRHk0cbwqcQriUtg==" }, + "sax": { + "version": "1.2.4", + "resolved": "https://registry.npmjs.org/sax/-/sax-1.2.4.tgz", + "integrity": "sha512-NqVDv9TpANUjFm0N8uM5GxL36UgKi9/atZw+x7YFnQ8ckwFGKrl4xX4yWtrey3UJm5nP1kUbnYgLopqWNSRhWw==" + }, "schema-utils": { "version": "3.1.1", "resolved": "https://registry.npmjs.org/schema-utils/-/schema-utils-3.1.1.tgz", @@ -26361,6 +26400,16 @@ "requires": { "rimraf": "^3.0.0" } + }, + "xml2js": { + "version": "0.4.23", + "resolved": "https://registry.npmjs.org/xml2js/-/xml2js-0.4.23.tgz", + "integrity": "sha512-ySPiMjM0+pLDftHgXY4By0uswI3SPKLDw/i3UXbnO8M/p28zqexCUoPmQFrYD+/1BzhGJSs2i1ERWKJAtiLrug==", + "dev": true, + "requires": { + "sax": ">=0.6.0", + "xmlbuilder": "~11.0.0" + } } } }, @@ -26828,19 +26877,12 @@ "dev": true }, "xml2js": { - "version": "0.4.23", - "resolved": "https://registry.npmjs.org/xml2js/-/xml2js-0.4.23.tgz", - "integrity": "sha512-ySPiMjM0+pLDftHgXY4By0uswI3SPKLDw/i3UXbnO8M/p28zqexCUoPmQFrYD+/1BzhGJSs2i1ERWKJAtiLrug==", + "version": "0.5.0", + "resolved": "https://registry.npmjs.org/xml2js/-/xml2js-0.5.0.tgz", + "integrity": "sha512-drPFnkQJik/O+uPKpqSgr22mpuFHqKdbS835iAQrUC73L2F5WkboIRd63ai/2Yg6I1jzifPFKH2NTK+cfglkIA==", "requires": { "sax": ">=0.6.0", "xmlbuilder": "~11.0.0" - }, - "dependencies": { - "sax": { - "version": "1.2.4", - "resolved": "https://registry.npmjs.org/sax/-/sax-1.2.4.tgz", - "integrity": "sha512-NqVDv9TpANUjFm0N8uM5GxL36UgKi9/atZw+x7YFnQ8ckwFGKrl4xX4yWtrey3UJm5nP1kUbnYgLopqWNSRhWw==" - } } }, "xmlbuilder": { diff --git a/package.json b/package.json index 57d969221a86..08d56bdf3bc0 100644 --- a/package.json +++ b/package.json @@ -1875,7 +1875,7 @@ "vscode-tas-client": "^0.1.63", "which": "^2.0.2", "winreg": "^1.2.4", - "xml2js": "^0.4.19" + "xml2js": "^0.5.0" }, "devDependencies": { "@istanbuljs/nyc-config-typescript": "^1.0.2", From d61377653e8b6f01c2efebb97ab33860484216e2 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 11 Apr 2023 08:33:11 -0700 Subject: [PATCH 5/6] Fix debugging when using "internalConsole" (#21033) Closes https://github.com/microsoft/vscode-python/issues/20828 Do case-insensitive merge of environment variables, always make sure to return the standard env key for an OS, similar to `process.env`. --- src/client/common/platform/fs-paths.ts | 6 +- src/client/common/variables/environment.ts | 63 ++++++++++++++++--- .../configuration/resolvers/helper.ts | 5 +- .../variables/envVarsService.unit.test.ts | 25 ++++---- src/test/debugger/envVars.test.ts | 7 ++- 5 files changed, 83 insertions(+), 23 deletions(-) diff --git a/src/client/common/platform/fs-paths.ts b/src/client/common/platform/fs-paths.ts index 18a1fea363b7..2d46fca98526 100644 --- a/src/client/common/platform/fs-paths.ts +++ b/src/client/common/platform/fs-paths.ts @@ -145,7 +145,11 @@ export class FileSystemPathUtils implements IFileSystemPathUtils { } export function normCasePath(filePath: string): string { - return getOSType() === OSType.Windows ? nodepath.normalize(filePath).toUpperCase() : nodepath.normalize(filePath); + return normCase(nodepath.normalize(filePath)); +} + +export function normCase(s: string): string { + return getOSType() === OSType.Windows ? s.toUpperCase() : s; } /** diff --git a/src/client/common/variables/environment.ts b/src/client/common/variables/environment.ts index 06d3ef3af414..4b3652e5021f 100644 --- a/src/client/common/variables/environment.ts +++ b/src/client/common/variables/environment.ts @@ -10,6 +10,7 @@ import { EventName } from '../../telemetry/constants'; import { IFileSystem } from '../platform/types'; import { IPathUtils } from '../types'; import { EnvironmentVariables, IEnvironmentVariablesService } from './types'; +import { normCase } from '../platform/fs-paths'; @injectable() export class EnvironmentVariablesService implements IEnvironmentVariablesService { @@ -61,6 +62,9 @@ export class EnvironmentVariablesService implements IEnvironmentVariablesService if (!target) { return; } + const reference = target; + target = normCaseKeys(target); + source = normCaseKeys(source); const settingsNotToMerge = ['PYTHONPATH', this.pathVariable]; Object.keys(source).forEach((setting) => { if (settingsNotToMerge.indexOf(setting) >= 0) { @@ -70,6 +74,8 @@ export class EnvironmentVariablesService implements IEnvironmentVariablesService target[setting] = source[setting]; } }); + restoreKeys(target); + matchTarget(reference, target); } public appendPythonPath(vars: EnvironmentVariables, ...pythonPaths: string[]) { @@ -80,18 +86,24 @@ export class EnvironmentVariablesService implements IEnvironmentVariablesService return this.appendPaths(vars, this.pathVariable, ...paths); } - private get pathVariable(): 'Path' | 'PATH' { + private get pathVariable(): string { if (!this._pathVariable) { this._pathVariable = this.pathUtils.getPathVariableName(); } - return this._pathVariable!; + return normCase(this._pathVariable)!; } - private appendPaths( - vars: EnvironmentVariables, - variableName: 'PATH' | 'Path' | 'PYTHONPATH', - ...pathsToAppend: string[] - ) { + private appendPaths(vars: EnvironmentVariables, variableName: string, ...pathsToAppend: string[]) { + const reference = vars; + vars = normCaseKeys(vars); + variableName = normCase(variableName); + vars = this._appendPaths(vars, variableName, ...pathsToAppend); + restoreKeys(vars); + matchTarget(reference, vars); + return vars; + } + + private _appendPaths(vars: EnvironmentVariables, variableName: string, ...pathsToAppend: string[]) { const valueToAppend = pathsToAppend .filter((item) => typeof item === 'string' && item.trim().length > 0) .map((item) => item.trim()) @@ -183,3 +195,40 @@ function substituteEnvVars( return value.replace(/\\\$/g, '$'); } + +export function normCaseKeys(env: EnvironmentVariables): EnvironmentVariables { + const normalizedEnv: EnvironmentVariables = {}; + Object.keys(env).forEach((key) => { + const normalizedKey = normCase(key); + normalizedEnv[normalizedKey] = env[key]; + }); + return normalizedEnv; +} + +export function restoreKeys(env: EnvironmentVariables) { + const processEnvKeys = Object.keys(process.env); + processEnvKeys.forEach((processEnvKey) => { + const originalKey = normCase(processEnvKey); + if (originalKey !== processEnvKey && env[originalKey] !== undefined) { + env[processEnvKey] = env[originalKey]; + delete env[originalKey]; + } + }); +} + +export function matchTarget(reference: EnvironmentVariables, target: EnvironmentVariables): void { + Object.keys(reference).forEach((key) => { + if (target.hasOwnProperty(key)) { + reference[key] = target[key]; + } else { + delete reference[key]; + } + }); + + // Add any new keys from target to reference + Object.keys(target).forEach((key) => { + if (!reference.hasOwnProperty(key)) { + reference[key] = target[key]; + } + }); +} diff --git a/src/client/debugger/extension/configuration/resolvers/helper.ts b/src/client/debugger/extension/configuration/resolvers/helper.ts index 0ccaa9964054..96cdf65bfac3 100644 --- a/src/client/debugger/extension/configuration/resolvers/helper.ts +++ b/src/client/debugger/extension/configuration/resolvers/helper.ts @@ -48,7 +48,10 @@ export class DebugEnvironmentVariablesHelper implements IDebugEnvironmentVariabl } // Append the PYTHONPATH and PATH variables. - this.envParser.appendPath(env, debugLaunchEnvVars[pathVariableName]); + this.envParser.appendPath( + env, + debugLaunchEnvVars[pathVariableName] ?? debugLaunchEnvVars[pathVariableName.toUpperCase()], + ); this.envParser.appendPythonPath(env, debugLaunchEnvVars.PYTHONPATH); if (typeof env[pathVariableName] === 'string' && env[pathVariableName]!.length > 0) { diff --git a/src/test/common/variables/envVarsService.unit.test.ts b/src/test/common/variables/envVarsService.unit.test.ts index 590803ea135e..8b1c402e634d 100644 --- a/src/test/common/variables/envVarsService.unit.test.ts +++ b/src/test/common/variables/envVarsService.unit.test.ts @@ -10,17 +10,16 @@ import * as TypeMoq from 'typemoq'; import { IFileSystem } from '../../../client/common/platform/types'; import { IPathUtils } from '../../../client/common/types'; import { EnvironmentVariablesService, parseEnvFile } from '../../../client/common/variables/environment'; +import { getSearchPathEnvVarNames } from '../../../client/common/utils/exec'; use(chaiAsPromised); type PathVar = 'Path' | 'PATH'; -const PATHS = [ - 'Path', // Windows - 'PATH', // non-Windows -]; +const PATHS = getSearchPathEnvVarNames(); -suite('Environment Variables Service', () => { +suite('xEnvironment Variables Service', () => { const filename = 'x/y/z/.env'; + const processEnvPath = getSearchPathEnvVarNames()[0]; let pathUtils: TypeMoq.IMock; let fs: TypeMoq.IMock; let variablesService: EnvironmentVariablesService; @@ -208,7 +207,7 @@ PYTHON=${BINDIR}/python3\n\ expect(vars2).to.have.property('TWO', 'TWO', 'Incorrect value'); expect(vars2).to.have.property('THREE', '3', 'Variable not merged'); expect(vars2).to.have.property('PYTHONPATH', 'PYTHONPATH', 'Incorrect value'); - expect(vars2).to.have.property(pathVariable, 'PATH', 'Incorrect value'); + expect(vars2).to.have.property(processEnvPath, 'PATH', 'Incorrect value'); verifyAll(); }); @@ -226,7 +225,7 @@ PYTHON=${BINDIR}/python3\n\ expect(target).to.have.property('TWO', 'TWO', 'Incorrect value'); expect(target).to.have.property('THREE', '3', 'Variable not merged'); expect(target).to.have.property('PYTHONPATH', 'PYTHONPATH', 'Incorrect value'); - expect(target).to.have.property(pathVariable, 'PATH', 'Incorrect value'); + expect(target).to.have.property(processEnvPath, 'PATH', 'Incorrect value'); verifyAll(); }); }); @@ -266,17 +265,17 @@ PYTHON=${BINDIR}/python3\n\ variablesService.appendPath(vars); expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables'); expect(vars).to.have.property('ONE', '1', 'Incorrect value'); - expect(vars).to.have.property(pathVariable, 'PATH', 'Incorrect value'); + expect(vars).to.have.property(processEnvPath, 'PATH', 'Incorrect value'); variablesService.appendPath(vars, ''); expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables'); expect(vars).to.have.property('ONE', '1', 'Incorrect value'); - expect(vars).to.have.property(pathVariable, 'PATH', 'Incorrect value'); + expect(vars).to.have.property(processEnvPath, 'PATH', 'Incorrect value'); variablesService.appendPath(vars, ' ', ''); expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables'); expect(vars).to.have.property('ONE', '1', 'Incorrect value'); - expect(vars).to.have.property(pathVariable, 'PATH', 'Incorrect value'); + expect(vars).to.have.property(processEnvPath, 'PATH', 'Incorrect value'); verifyAll(); }); @@ -291,7 +290,11 @@ PYTHON=${BINDIR}/python3\n\ expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables'); expect(vars).to.have.property('ONE', '1', 'Incorrect value'); - expect(vars).to.have.property(pathVariable, `PATH${path.delimiter}${pathToAppend}`, 'Incorrect value'); + expect(vars).to.have.property( + processEnvPath, + `PATH${path.delimiter}${pathToAppend}`, + 'Incorrect value', + ); verifyAll(); }); }); diff --git a/src/test/debugger/envVars.test.ts b/src/test/debugger/envVars.test.ts index 6aa0dea4d8c6..c043146fe53d 100644 --- a/src/test/debugger/envVars.test.ts +++ b/src/test/debugger/envVars.test.ts @@ -15,6 +15,7 @@ import { ConsoleType, LaunchRequestArguments } from '../../client/debugger/types import { isOs, OSType } from '../common'; import { closeActiveWindows, initialize, initializeTest, IS_MULTI_ROOT_TEST, TEST_DEBUGGER } from '../initialize'; import { UnitTestIocContainer } from '../testing/serviceRegistry'; +import { normCase } from '../../client/common/platform/fs-paths'; use(chaiAsPromised); @@ -109,9 +110,9 @@ suite('Resolving Environment Variables when Debugging', () => { }); async function testJsonEnvVariables(console: ConsoleType, expectedNumberOfVariables: number) { - const prop1 = shortid.generate(); - const prop2 = shortid.generate(); - const prop3 = shortid.generate(); + const prop1 = normCase(shortid.generate()); + const prop2 = normCase(shortid.generate()); + const prop3 = normCase(shortid.generate()); const env: Record = {}; env[prop1] = prop1; env[prop2] = prop2; From f058f5efeab5b18e01170f2d76286cfa255b304f Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 11 Apr 2023 10:24:28 -0700 Subject: [PATCH 6/6] Undo unwanted change in env var tests (#21035) https://github.com/microsoft/vscode-python/pull/21033#discussion_r1163009663 --- src/test/common/variables/envVarsService.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/common/variables/envVarsService.unit.test.ts b/src/test/common/variables/envVarsService.unit.test.ts index 8b1c402e634d..0c978b2f9e86 100644 --- a/src/test/common/variables/envVarsService.unit.test.ts +++ b/src/test/common/variables/envVarsService.unit.test.ts @@ -17,7 +17,7 @@ use(chaiAsPromised); type PathVar = 'Path' | 'PATH'; const PATHS = getSearchPathEnvVarNames(); -suite('xEnvironment Variables Service', () => { +suite('Environment Variables Service', () => { const filename = 'x/y/z/.env'; const processEnvPath = getSearchPathEnvVarNames()[0]; let pathUtils: TypeMoq.IMock;