From e0f2130e9af946c580795d986286a821ab8767b7 Mon Sep 17 00:00:00 2001 From: Andy Jordan <2226434+andyleejordan@users.noreply.github.com> Date: Thu, 21 Nov 2024 12:12:06 -0800 Subject: [PATCH] Fix additional PowerShell warning (take two) Since Show Session Menu always fully enumerates the iterator we needed to move the existence check into the generator. Fortunately the 'exists' method was already idempotent. I'd like this to be cleaner, but at least the tests now make sense (and required a stub fix). --- src/platform.ts | 34 ++++-- test/core/platform.test.ts | 210 ++++++++++--------------------------- 2 files changed, 85 insertions(+), 159 deletions(-) diff --git a/src/platform.ts b/src/platform.ts index 706c9f56e2..360fea1613 100644 --- a/src/platform.ts +++ b/src/platform.ts @@ -148,7 +148,7 @@ export class PowerShellExeFinder { // Also show any additionally configured PowerShells // These may be duplicates of the default installations, but given a different name. - for (const additionalPwsh of this.enumerateAdditionalPowerShellInstallations()) { + for await (const additionalPwsh of this.enumerateAdditionalPowerShellInstallations()) { if (await additionalPwsh.exists()) { yield additionalPwsh; } else if (!additionalPwsh.suppressWarning) { @@ -230,7 +230,7 @@ export class PowerShellExeFinder { * Iterates through the configured additional PowerShell executable locations, * without checking for their existence. */ - private *enumerateAdditionalPowerShellInstallations(): Iterable { + private async *enumerateAdditionalPowerShellInstallations(): AsyncIterable { for (const versionName in this.additionalPowerShellExes) { if (Object.prototype.hasOwnProperty.call(this.additionalPowerShellExes, versionName)) { let exePath: string | undefined = utils.stripQuotePair(this.additionalPowerShellExes[versionName]); @@ -245,7 +245,11 @@ export class PowerShellExeFinder { // Always search for what the user gave us first, but with the warning // suppressed so we can display it after all possibilities are exhausted - yield new PossiblePowerShellExe(exePath, ...args); + let pwsh = new PossiblePowerShellExe(exePath, ...args); + if (await pwsh.exists()) { + yield pwsh; + continue; + } // Also search for `pwsh[.exe]` and `powershell[.exe]` if missing if (this.platformDetails.operatingSystem === OperatingSystem.Windows) { @@ -253,16 +257,32 @@ export class PowerShellExeFinder { if (!exePath.endsWith("pwsh.exe") && !exePath.endsWith("powershell.exe")) { if (exePath.endsWith("pwsh") || exePath.endsWith("powershell")) { // Add extension if that was missing - yield new PossiblePowerShellExe(exePath + ".exe", ...args); + pwsh = new PossiblePowerShellExe(exePath + ".exe", ...args); + if (await pwsh.exists()) { + yield pwsh; + continue; + } } // Also add full exe names (this isn't an else just in case // the folder was named "pwsh" or "powershell") - yield new PossiblePowerShellExe(path.join(exePath, "pwsh.exe"), ...args); - yield new PossiblePowerShellExe(path.join(exePath, "powershell.exe"), ...args); + pwsh = new PossiblePowerShellExe(path.join(exePath, "pwsh.exe"), ...args); + if (await pwsh.exists()) { + yield pwsh; + continue; + } + pwsh = new PossiblePowerShellExe(path.join(exePath, "powershell.exe"), ...args); + if (await pwsh.exists()) { + yield pwsh; + continue; + } } } else if (!exePath.endsWith("pwsh")) { // Always just 'pwsh' on non-Windows - yield new PossiblePowerShellExe(path.join(exePath, "pwsh"), ...args); + pwsh = new PossiblePowerShellExe(path.join(exePath, "pwsh"), ...args); + if (await pwsh.exists()) { + yield pwsh; + continue; + } } // If we're still being iterated over, no permutation of the given path existed so yield an object with the warning unsuppressed diff --git a/test/core/platform.test.ts b/test/core/platform.test.ts index 299e125626..0953379639 100644 --- a/test/core/platform.test.ts +++ b/test/core/platform.test.ts @@ -18,10 +18,20 @@ import { stripQuotePair } from "../../src/utils"; const platformMock = rewire("../../src/platform"); // eslint-disable-next-line @typescript-eslint/require-await -async function fakeCheckIfFileOrDirectoryExists(targetPath: string | vscode.Uri): Promise { +async function fakeCheckIfFileExists(targetPath: string | vscode.Uri): Promise { try { - fs.lstatSync(targetPath instanceof vscode.Uri ? targetPath.fsPath : targetPath); - return true; + const stat = fs.lstatSync(targetPath instanceof vscode.Uri ? targetPath.fsPath : targetPath); + return stat.isFile(); + } catch { + return false; + } +} + +// eslint-disable-next-line @typescript-eslint/require-await +async function fakeCheckIfDirectoryExists(targetPath: string | vscode.Uri): Promise { + try { + const stat = fs.lstatSync(targetPath instanceof vscode.Uri ? targetPath.fsPath : targetPath); + return stat.isDirectory(); } catch { return false; } @@ -33,8 +43,8 @@ async function fakeReadDirectory(targetPath: string | vscode.Uri): Promise