From 064ef6315510f3a1deee544521afba6c61afc324 Mon Sep 17 00:00:00 2001 From: Roger Zhang Date: Tue, 5 Nov 2024 16:25:50 -0800 Subject: [PATCH 01/12] test --- packages/core/src/shared/env/resolveEnv.ts | 2 +- .../core/src/shared/sam/localLambdaRunner.ts | 18 ++++++++---------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/packages/core/src/shared/env/resolveEnv.ts b/packages/core/src/shared/env/resolveEnv.ts index 7413c9aa832..bee3e8e4864 100644 --- a/packages/core/src/shared/env/resolveEnv.ts +++ b/packages/core/src/shared/env/resolveEnv.ts @@ -139,7 +139,7 @@ export async function mergeResolvedShellPath(env: IProcessEnvironment): Promise< * - we hit a timeout of `MAX_SHELL_RESOLVE_TIME` * - any other error from spawning a shell to figure out the environment */ -export async function getResolvedShellEnv(env?: IProcessEnvironment): Promise { +export async function getResolvedShellEnv(env?: IProcessEnvironment): Promise { if (!env) { env = process.env } diff --git a/packages/core/src/shared/sam/localLambdaRunner.ts b/packages/core/src/shared/sam/localLambdaRunner.ts index 84547d02c6f..54c19eb2cde 100644 --- a/packages/core/src/shared/sam/localLambdaRunner.ts +++ b/packages/core/src/shared/sam/localLambdaRunner.ts @@ -212,10 +212,7 @@ async function invokeLambdaHandler( return config .samLocalInvokeCommand!.invoke({ options: { - env: await getSpawnEnv({ - ...process.env, - ...env, - }), + env: env, }, command: samCommand, args: samArgs, @@ -236,7 +233,7 @@ async function invokeLambdaHandler( templatePath: config.templatePath, eventPath: config.eventPayloadFile, environmentVariablePath: config.envFile, - environmentVariables: await getSpawnEnv(env), + environmentVariables: env, invoker: config.samLocalInvokeCommand!, dockerNetwork: config.sam?.dockerNetwork, debugPort: debugPort, @@ -290,15 +287,16 @@ export async function runLambdaFunction( getLogger().info(localize('AWS.output.sam.local.startRun', 'Preparing to run locally: {0}', config.handlerName)) } - const envVars = { + const envVars = await getSpawnEnv({ + ...process.env, ...(config.aws?.region ? { AWS_DEFAULT_REGION: config.aws.region } : {}), - } + }) const settings = SamCliSettings.instance const timer = new Timeout(settings.getLocalInvokeTimeout()) // TODO: refactor things to not mutate the config - config.templatePath = await buildLambdaHandler(timer, await getSpawnEnv(envVars), config, settings) + config.templatePath = await buildLambdaHandler(timer, envVars, config, settings) await onAfterBuild() timer.refresh() @@ -315,7 +313,7 @@ export async function runLambdaFunction( // SAM CLI and any API requests are executed in parallel // A failure from either is a failure for the whole invocation - const [process] = await Promise.all([invokeLambdaHandler(timer, envVars, config, settings), apiRequest]).catch( + const [processConst] = await Promise.all([invokeLambdaHandler(timer, envVars, config, settings), apiRequest]).catch( (err) => { timer.cancel() throw err @@ -329,7 +327,7 @@ export async function runLambdaFunction( const terminationListener = vscode.debug.onDidTerminateDebugSession((session) => { const config = session.configuration as SamLaunchRequestArgs if (config.invokeTarget?.target === 'api') { - stopApi(process, config) + stopApi(processConst, config) } }) From bf779de3c788628e6352b0b53f3416c948ed645a Mon Sep 17 00:00:00 2001 From: Roger Zhang Date: Tue, 5 Nov 2024 16:45:02 -0800 Subject: [PATCH 02/12] tt --- packages/core/src/shared/sam/localLambdaRunner.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/core/src/shared/sam/localLambdaRunner.ts b/packages/core/src/shared/sam/localLambdaRunner.ts index 54c19eb2cde..4a4c8378346 100644 --- a/packages/core/src/shared/sam/localLambdaRunner.ts +++ b/packages/core/src/shared/sam/localLambdaRunner.ts @@ -30,7 +30,7 @@ import { showMessageWithCancel } from '../utilities/messages' import { ToolkitError, UnknownError } from '../errors' import { SamCliError } from './cli/samCliInvokerUtils' import fs from '../fs/fs' -import { getSpawnEnv } from '../env/resolveEnv' +import { asEnvironmentVariables } from '../../auth/credentials/utils' const localize = nls.loadMessageBundle() @@ -287,10 +287,11 @@ export async function runLambdaFunction( getLogger().info(localize('AWS.output.sam.local.startRun', 'Preparing to run locally: {0}', config.handlerName)) } - const envVars = await getSpawnEnv({ + const envVars = { ...process.env, + ...(config.awsCredentials ? asEnvironmentVariables(config.awsCredentials) : {}), ...(config.aws?.region ? { AWS_DEFAULT_REGION: config.aws.region } : {}), - }) + } const settings = SamCliSettings.instance const timer = new Timeout(settings.getLocalInvokeTimeout()) From 3d43c776e5e61769d17e65e2837c05f8ec042314 Mon Sep 17 00:00:00 2001 From: Roger Zhang Date: Tue, 5 Nov 2024 16:49:54 -0800 Subject: [PATCH 03/12] add spawn --- packages/core/src/shared/sam/localLambdaRunner.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/core/src/shared/sam/localLambdaRunner.ts b/packages/core/src/shared/sam/localLambdaRunner.ts index 4a4c8378346..11ce556dd39 100644 --- a/packages/core/src/shared/sam/localLambdaRunner.ts +++ b/packages/core/src/shared/sam/localLambdaRunner.ts @@ -31,6 +31,7 @@ import { ToolkitError, UnknownError } from '../errors' import { SamCliError } from './cli/samCliInvokerUtils' import fs from '../fs/fs' import { asEnvironmentVariables } from '../../auth/credentials/utils' +import { getSpawnEnv } from '../env/resolveEnv' const localize = nls.loadMessageBundle() @@ -287,11 +288,11 @@ export async function runLambdaFunction( getLogger().info(localize('AWS.output.sam.local.startRun', 'Preparing to run locally: {0}', config.handlerName)) } - const envVars = { + const envVars = await getSpawnEnv({ ...process.env, ...(config.awsCredentials ? asEnvironmentVariables(config.awsCredentials) : {}), ...(config.aws?.region ? { AWS_DEFAULT_REGION: config.aws.region } : {}), - } + }) const settings = SamCliSettings.instance const timer = new Timeout(settings.getLocalInvokeTimeout()) From 9520e47b510b118789a55500961cc41ea4073e40 Mon Sep 17 00:00:00 2001 From: Roger Zhang Date: Wed, 6 Nov 2024 09:53:32 -0800 Subject: [PATCH 04/12] save --- packages/core/src/shared/env/resolveEnv.ts | 4 ++-- .../core/src/shared/sam/localLambdaRunner.ts | 17 ++++++++--------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/packages/core/src/shared/env/resolveEnv.ts b/packages/core/src/shared/env/resolveEnv.ts index bee3e8e4864..f8710233d05 100644 --- a/packages/core/src/shared/env/resolveEnv.ts +++ b/packages/core/src/shared/env/resolveEnv.ts @@ -114,7 +114,7 @@ export async function getSpawnEnv( export async function mergeResolvedShellPath(env: IProcessEnvironment): Promise { const shellEnv = await getResolvedShellEnv(env) // resolve failed or doesn't need to resolve - if (!shellEnv) { + if (!shellEnv || Object.keys(shellEnv).length === 0) { return env } try { @@ -139,7 +139,7 @@ export async function mergeResolvedShellPath(env: IProcessEnvironment): Promise< * - we hit a timeout of `MAX_SHELL_RESOLVE_TIME` * - any other error from spawning a shell to figure out the environment */ -export async function getResolvedShellEnv(env?: IProcessEnvironment): Promise { +export async function getResolvedShellEnv(env?: IProcessEnvironment): Promise { if (!env) { env = process.env } diff --git a/packages/core/src/shared/sam/localLambdaRunner.ts b/packages/core/src/shared/sam/localLambdaRunner.ts index 11ce556dd39..c65bf373be0 100644 --- a/packages/core/src/shared/sam/localLambdaRunner.ts +++ b/packages/core/src/shared/sam/localLambdaRunner.ts @@ -30,7 +30,6 @@ import { showMessageWithCancel } from '../utilities/messages' import { ToolkitError, UnknownError } from '../errors' import { SamCliError } from './cli/samCliInvokerUtils' import fs from '../fs/fs' -import { asEnvironmentVariables } from '../../auth/credentials/utils' import { getSpawnEnv } from '../env/resolveEnv' const localize = nls.loadMessageBundle() @@ -290,7 +289,6 @@ export async function runLambdaFunction( const envVars = await getSpawnEnv({ ...process.env, - ...(config.awsCredentials ? asEnvironmentVariables(config.awsCredentials) : {}), ...(config.aws?.region ? { AWS_DEFAULT_REGION: config.aws.region } : {}), }) @@ -315,12 +313,13 @@ export async function runLambdaFunction( // SAM CLI and any API requests are executed in parallel // A failure from either is a failure for the whole invocation - const [processConst] = await Promise.all([invokeLambdaHandler(timer, envVars, config, settings), apiRequest]).catch( - (err) => { - timer.cancel() - throw err - } - ) + const [processInvoker] = await Promise.all([ + invokeLambdaHandler(timer, envVars, config, settings), + apiRequest, + ]).catch((err) => { + timer.cancel() + throw err + }) if (config.noDebug) { return config @@ -329,7 +328,7 @@ export async function runLambdaFunction( const terminationListener = vscode.debug.onDidTerminateDebugSession((session) => { const config = session.configuration as SamLaunchRequestArgs if (config.invokeTarget?.target === 'api') { - stopApi(processConst, config) + stopApi(processInvoker, config) } }) From fd9457f5d1d0d717b76fd6ff85a55a6c83a74335 Mon Sep 17 00:00:00 2001 From: Roger Zhang Date: Wed, 6 Nov 2024 10:25:07 -0800 Subject: [PATCH 05/12] add test --- .../src/test/shared/env/resolveEnv.test.ts | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 packages/core/src/test/shared/env/resolveEnv.test.ts diff --git a/packages/core/src/test/shared/env/resolveEnv.test.ts b/packages/core/src/test/shared/env/resolveEnv.test.ts new file mode 100644 index 00000000000..522c9ffe465 --- /dev/null +++ b/packages/core/src/test/shared/env/resolveEnv.test.ts @@ -0,0 +1,30 @@ +/*! + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +import assert from 'assert' +import { mergeResolvedShellPath } from '../../../shared/env/resolveEnv' +import sinon from 'sinon' +import { EnvironmentVariables } from '../../../shared/environmentVariables' + +describe('resolveEnv', async function () { + let sandbox: sinon.SinonSandbox + beforeEach(function () { + sandbox = sinon.createSandbox() + sandbox.stub(process, 'env').value({ + platform: 'win32', + } as EnvironmentVariables) + }) + + afterEach(function () { + sandbox.restore() + }) + + describe('resolveWindows', async function () { + it('mergeResolvedShellPath should not change path on windows', async function () { + const env = await mergeResolvedShellPath(process.env) + assert.strictEqual(env.path, process.env.path) + }) + }) +}) From 9b66d8c766f26d97a620d78d3b0930d6e33ab918 Mon Sep 17 00:00:00 2001 From: Roger Zhang Date: Wed, 6 Nov 2024 10:34:34 -0800 Subject: [PATCH 06/12] update test --- packages/core/src/test/shared/env/resolveEnv.test.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/core/src/test/shared/env/resolveEnv.test.ts b/packages/core/src/test/shared/env/resolveEnv.test.ts index 522c9ffe465..a4d2290a127 100644 --- a/packages/core/src/test/shared/env/resolveEnv.test.ts +++ b/packages/core/src/test/shared/env/resolveEnv.test.ts @@ -6,15 +6,12 @@ import assert from 'assert' import { mergeResolvedShellPath } from '../../../shared/env/resolveEnv' import sinon from 'sinon' -import { EnvironmentVariables } from '../../../shared/environmentVariables' describe('resolveEnv', async function () { let sandbox: sinon.SinonSandbox beforeEach(function () { sandbox = sinon.createSandbox() - sandbox.stub(process, 'env').value({ - platform: 'win32', - } as EnvironmentVariables) + sandbox.stub(process, 'platform').value('win32') }) afterEach(function () { @@ -24,7 +21,8 @@ describe('resolveEnv', async function () { describe('resolveWindows', async function () { it('mergeResolvedShellPath should not change path on windows', async function () { const env = await mergeResolvedShellPath(process.env) - assert.strictEqual(env.path, process.env.path) + assert(env.PATH) + assert.strictEqual(env, process.env) }) }) }) From 127003fbc378c572cbdd37d477991ca4a77b74d8 Mon Sep 17 00:00:00 2001 From: Roger Zhang Date: Wed, 6 Nov 2024 12:20:34 -0800 Subject: [PATCH 07/12] comment --- .../core/src/test/shared/env/resolveEnv.test.ts | 17 +++++++++++++++++ ...ix-268ceb41-7bec-4b57-b195-d72f19d00faa.json | 4 ++++ 2 files changed, 21 insertions(+) create mode 100644 packages/toolkit/.changes/next-release/Bug Fix-268ceb41-7bec-4b57-b195-d72f19d00faa.json diff --git a/packages/core/src/test/shared/env/resolveEnv.test.ts b/packages/core/src/test/shared/env/resolveEnv.test.ts index a4d2290a127..5c0f246b604 100644 --- a/packages/core/src/test/shared/env/resolveEnv.test.ts +++ b/packages/core/src/test/shared/env/resolveEnv.test.ts @@ -19,10 +19,27 @@ describe('resolveEnv', async function () { }) describe('resolveWindows', async function () { + beforeEach(function () { + sandbox.stub(process, 'platform').value('win32') + }) + it('mergeResolvedShellPath should not change path on windows', async function () { const env = await mergeResolvedShellPath(process.env) assert(env.PATH) assert.strictEqual(env, process.env) }) }) + + describe('resolveMac', async function () { + beforeEach(function () { + sandbox.stub(process, 'platform').value('darwin') + }) + + it('mergeResolvedShellPath should get path on mac', async function () { + sandbox.stub(process.env, 'PATH').value('') + const env = await mergeResolvedShellPath(process.env) + assert(env.PATH) + assert.notEqual(env, process.env) + }) + }) }) diff --git a/packages/toolkit/.changes/next-release/Bug Fix-268ceb41-7bec-4b57-b195-d72f19d00faa.json b/packages/toolkit/.changes/next-release/Bug Fix-268ceb41-7bec-4b57-b195-d72f19d00faa.json new file mode 100644 index 00000000000..856f2a21365 --- /dev/null +++ b/packages/toolkit/.changes/next-release/Bug Fix-268ceb41-7bec-4b57-b195-d72f19d00faa.json @@ -0,0 +1,4 @@ +{ + "type": "Bug Fix", + "description": "getSpawnEnv always return correct env on windows" +} From af5ae8f6a04ad977ae2891c21d1b6a1562d33230 Mon Sep 17 00:00:00 2001 From: Roger Zhang Date: Wed, 6 Nov 2024 13:06:19 -0800 Subject: [PATCH 08/12] stub getResolvedShellEnv for mac test --- .../src/test/shared/env/resolveEnv.test.ts | 32 +++++++++++++++++-- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/packages/core/src/test/shared/env/resolveEnv.test.ts b/packages/core/src/test/shared/env/resolveEnv.test.ts index 5c0f246b604..8a8ff79fb47 100644 --- a/packages/core/src/test/shared/env/resolveEnv.test.ts +++ b/packages/core/src/test/shared/env/resolveEnv.test.ts @@ -4,8 +4,9 @@ */ import assert from 'assert' -import { mergeResolvedShellPath } from '../../../shared/env/resolveEnv' +import * as resolveEnv from '../../../shared/env/resolveEnv' import sinon from 'sinon' +import path from 'path' describe('resolveEnv', async function () { let sandbox: sinon.SinonSandbox @@ -18,26 +19,51 @@ describe('resolveEnv', async function () { sandbox.restore() }) + // a copy of resolveEnv.mergeResolvedShellPath for stubbing + const testMergeResolveShellPath = async function mergeResolvedShellPath( + env: resolveEnv.IProcessEnvironment + ): Promise { + const shellEnv = await resolveEnv.getResolvedShellEnv(env) + // resolve failed or doesn't need to resolve + if (!shellEnv || Object.keys(shellEnv).length === 0) { + return env + } + try { + const envPaths: string[] = env.PATH ? env.PATH.split(path.delimiter) : [] + const resolvedPaths: string[] = shellEnv.PATH ? shellEnv.PATH.split(path.delimiter) : [] + const envReturn = { ...env } + // merge, dedup, join + envReturn.PATH = [...new Set(envPaths.concat(resolvedPaths))].join(path.delimiter) + + return envReturn + } catch (err) { + return env + } + } + describe('resolveWindows', async function () { beforeEach(function () { sandbox.stub(process, 'platform').value('win32') }) it('mergeResolvedShellPath should not change path on windows', async function () { - const env = await mergeResolvedShellPath(process.env) + const env = await resolveEnv.mergeResolvedShellPath(process.env) assert(env.PATH) assert.strictEqual(env, process.env) }) }) describe('resolveMac', async function () { + const originalEnv = { ...process.env } beforeEach(function () { sandbox.stub(process, 'platform').value('darwin') }) it('mergeResolvedShellPath should get path on mac', async function () { sandbox.stub(process.env, 'PATH').value('') - const env = await mergeResolvedShellPath(process.env) + // stub the resolve Env logic cause this is platform sensitive. + sandbox.stub(resolveEnv, 'getResolvedShellEnv').resolves(originalEnv) + const env = await testMergeResolveShellPath(process.env) assert(env.PATH) assert.notEqual(env, process.env) }) From 6f0d2b595f2e883e18ccc375431010247cb5af96 Mon Sep 17 00:00:00 2001 From: Roger Zhang Date: Wed, 6 Nov 2024 13:24:15 -0800 Subject: [PATCH 09/12] skip unix test on windows --- .../src/test/shared/env/resolveEnv.test.ts | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/packages/core/src/test/shared/env/resolveEnv.test.ts b/packages/core/src/test/shared/env/resolveEnv.test.ts index 8a8ff79fb47..70d2032c223 100644 --- a/packages/core/src/test/shared/env/resolveEnv.test.ts +++ b/packages/core/src/test/shared/env/resolveEnv.test.ts @@ -10,6 +10,7 @@ import path from 'path' describe('resolveEnv', async function () { let sandbox: sinon.SinonSandbox + let originalPlatform = process.platform beforeEach(function () { sandbox = sinon.createSandbox() sandbox.stub(process, 'platform').value('win32') @@ -53,19 +54,18 @@ describe('resolveEnv', async function () { }) }) - describe('resolveMac', async function () { + describe('resolveUnix', async function () { const originalEnv = { ...process.env } - beforeEach(function () { - sandbox.stub(process, 'platform').value('darwin') - }) - - it('mergeResolvedShellPath should get path on mac', async function () { - sandbox.stub(process.env, 'PATH').value('') - // stub the resolve Env logic cause this is platform sensitive. - sandbox.stub(resolveEnv, 'getResolvedShellEnv').resolves(originalEnv) - const env = await testMergeResolveShellPath(process.env) - assert(env.PATH) - assert.notEqual(env, process.env) - }) + // skip mac test on windows + if (originalPlatform !== 'win32') { + it('mergeResolvedShellPath should get path on mac/unix', async function () { + sandbox.stub(process.env, 'PATH').value('') + // stub the resolve Env logic cause this is platform sensitive. + sandbox.stub(resolveEnv, 'getResolvedShellEnv').resolves(originalEnv) + const env = await testMergeResolveShellPath(process.env) + assert(env.PATH) + assert.notEqual(env, process.env) + }) + } }) }) From add2e45d14d1b94c062114919400cdce192d5336 Mon Sep 17 00:00:00 2001 From: Roger Zhang Date: Wed, 6 Nov 2024 13:33:49 -0800 Subject: [PATCH 10/12] lint --- packages/core/src/test/shared/env/resolveEnv.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/core/src/test/shared/env/resolveEnv.test.ts b/packages/core/src/test/shared/env/resolveEnv.test.ts index 70d2032c223..02c4b00b3f1 100644 --- a/packages/core/src/test/shared/env/resolveEnv.test.ts +++ b/packages/core/src/test/shared/env/resolveEnv.test.ts @@ -10,10 +10,8 @@ import path from 'path' describe('resolveEnv', async function () { let sandbox: sinon.SinonSandbox - let originalPlatform = process.platform beforeEach(function () { sandbox = sinon.createSandbox() - sandbox.stub(process, 'platform').value('win32') }) afterEach(function () { @@ -57,7 +55,7 @@ describe('resolveEnv', async function () { describe('resolveUnix', async function () { const originalEnv = { ...process.env } // skip mac test on windows - if (originalPlatform !== 'win32') { + if (process.platform !== 'win32') { it('mergeResolvedShellPath should get path on mac/unix', async function () { sandbox.stub(process.env, 'PATH').value('') // stub the resolve Env logic cause this is platform sensitive. From 033c8e8c017bbf61ce35b0054b0c6382b40202e8 Mon Sep 17 00:00:00 2001 From: Roger Zhang Date: Wed, 6 Nov 2024 15:20:59 -0800 Subject: [PATCH 11/12] address comments --- packages/core/src/shared/env/resolveEnv.ts | 19 ++++++++++++------- .../src/test/shared/env/resolveEnv.test.ts | 12 +++++++----- ...-268ceb41-7bec-4b57-b195-d72f19d00faa.json | 2 +- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/packages/core/src/shared/env/resolveEnv.ts b/packages/core/src/shared/env/resolveEnv.ts index f8710233d05..75241c71a46 100644 --- a/packages/core/src/shared/env/resolveEnv.ts +++ b/packages/core/src/shared/env/resolveEnv.ts @@ -114,7 +114,7 @@ export async function getSpawnEnv( export async function mergeResolvedShellPath(env: IProcessEnvironment): Promise { const shellEnv = await getResolvedShellEnv(env) // resolve failed or doesn't need to resolve - if (!shellEnv || Object.keys(shellEnv).length === 0) { + if (!shellEnv) { return env } try { @@ -139,7 +139,7 @@ export async function mergeResolvedShellPath(env: IProcessEnvironment): Promise< * - we hit a timeout of `MAX_SHELL_RESOLVE_TIME` * - any other error from spawning a shell to figure out the environment */ -export async function getResolvedShellEnv(env?: IProcessEnvironment): Promise { +export async function getResolvedShellEnv(env?: IProcessEnvironment): Promise { if (!env) { env = process.env } @@ -147,21 +147,21 @@ export async function getResolvedShellEnv(env?: IProcessEnvironment): Promise 0) { + resolve(shellEnv) + } else { + return undefined + } } catch { // failed resolve should not affect other feature. - resolve({}) + return undefined } }) } diff --git a/packages/core/src/test/shared/env/resolveEnv.test.ts b/packages/core/src/test/shared/env/resolveEnv.test.ts index 02c4b00b3f1..d645df57f81 100644 --- a/packages/core/src/test/shared/env/resolveEnv.test.ts +++ b/packages/core/src/test/shared/env/resolveEnv.test.ts @@ -18,13 +18,15 @@ describe('resolveEnv', async function () { sandbox.restore() }) - // a copy of resolveEnv.mergeResolvedShellPath for stubbing + // a copy of resolveEnv.mergeResolvedShellPath for stubbing getResolvedShellEnv + // mergeResolvedShellPath is calling getResolvedShellEnv within the same file + // thus we need a copy to stub getResolvedShellEnv correctly const testMergeResolveShellPath = async function mergeResolvedShellPath( env: resolveEnv.IProcessEnvironment ): Promise { const shellEnv = await resolveEnv.getResolvedShellEnv(env) // resolve failed or doesn't need to resolve - if (!shellEnv || Object.keys(shellEnv).length === 0) { + if (!shellEnv) { return env } try { @@ -40,7 +42,7 @@ describe('resolveEnv', async function () { } } - describe('resolveWindows', async function () { + describe('windows', async function () { beforeEach(function () { sandbox.stub(process, 'platform').value('win32') }) @@ -52,11 +54,11 @@ describe('resolveEnv', async function () { }) }) - describe('resolveUnix', async function () { + describe('unix', async function () { const originalEnv = { ...process.env } // skip mac test on windows if (process.platform !== 'win32') { - it('mergeResolvedShellPath should get path on mac/unix', async function () { + it('mergeResolvedShellPath should get path on mac/linux', async function () { sandbox.stub(process.env, 'PATH').value('') // stub the resolve Env logic cause this is platform sensitive. sandbox.stub(resolveEnv, 'getResolvedShellEnv').resolves(originalEnv) diff --git a/packages/toolkit/.changes/next-release/Bug Fix-268ceb41-7bec-4b57-b195-d72f19d00faa.json b/packages/toolkit/.changes/next-release/Bug Fix-268ceb41-7bec-4b57-b195-d72f19d00faa.json index 856f2a21365..65c75e142a3 100644 --- a/packages/toolkit/.changes/next-release/Bug Fix-268ceb41-7bec-4b57-b195-d72f19d00faa.json +++ b/packages/toolkit/.changes/next-release/Bug Fix-268ceb41-7bec-4b57-b195-d72f19d00faa.json @@ -1,4 +1,4 @@ { "type": "Bug Fix", - "description": "getSpawnEnv always return correct env on windows" + "description": "System Path parsing should ignore Windows and only parse Mac/Linux system, Sam Local Invoke should get correct system Path on windows" } From 90e1996c1b666541c49ebf56bcdf90fe7c0cb424 Mon Sep 17 00:00:00 2001 From: Roger Zhang Date: Thu, 7 Nov 2024 12:37:00 -0800 Subject: [PATCH 12/12] node min version fix --- packages/core/src/shared/env/resolveEnv.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/core/src/shared/env/resolveEnv.ts b/packages/core/src/shared/env/resolveEnv.ts index 75241c71a46..10029403dab 100644 --- a/packages/core/src/shared/env/resolveEnv.ts +++ b/packages/core/src/shared/env/resolveEnv.ts @@ -20,7 +20,7 @@ import { asEnvironmentVariables } from '../../auth/credentials/utils' import { getIAMConnection } from '../../auth/utils' import { ChildProcess } from '../utilities/processUtils' -let unixShellEnvPromise: Promise | undefined = undefined +let unixShellEnvPromise: Promise | undefined = undefined let envCacheExpireTime: number export interface IProcessEnvironment { @@ -176,7 +176,7 @@ export async function getResolvedShellEnv(env?: IProcessEnvironment): Promise envCacheExpireTime) { // cache valid for 5 minutes envCacheExpireTime = Date.now() + 5 * 60 * 1000 - unixShellEnvPromise = new Promise(async (resolve, reject) => { + unixShellEnvPromise = new Promise(async (resolve, reject) => { const timeout = new Timeout(10000) // Resolve shell env and handle errors @@ -185,11 +185,11 @@ export async function getResolvedShellEnv(env?: IProcessEnvironment): Promise 0) { resolve(shellEnv) } else { - return undefined + resolve(undefined) } } catch { // failed resolve should not affect other feature. - return undefined + resolve(undefined) } }) }