Add Swiftly toolchain management #1717
Conversation
| } | ||
| } | ||
|
|
||
| export const swiftly = new Swiftly(); |
There was a problem hiding this comment.
Having global state like this will make it harder to test future integrations with Swiftly. This should probably be constructed in WorkspaceContext. Things like ToolchainSelection.ts can then either have it passed in as a dependency or pull it out of the workspace.
| // | ||
| // This source file is part of the VS Code Swift open source project | ||
| // | ||
| // Copyright (c) 2021 the VS Code Swift project authors |
There was a problem hiding this comment.
| // Copyright (c) 2021 the VS Code Swift project authors | |
| // Copyright (c) 2025 the VS Code Swift project authors |
| @@ -0,0 +1,157 @@ | |||
| import * as path from "node:path"; | |||
There was a problem hiding this comment.
Missing copyright header.
| } catch (err: unknown) { | ||
| const error = err as ExecFileError; | ||
| // Its possible the toolchain in .swift-version is misconfigured or doesn't exist. | ||
| void vscode.window.showErrorMessage(`${error.stderr}`); |
There was a problem hiding this comment.
This seems like something we should be logging and then show a better error message to the user. Something like "Failed to load toolchain from Swiftly: ${error.stderr}".
What are the possible error messages from Swiftly here? Asking because I'm not sure how well VS Code notifications deal with newlines and such.
| .filter((toolchain): toolchain is string => typeof toolchain === "string") | ||
| .map(toolchain => path.join(swiftlyHomeDir, "toolchains", toolchain)); | ||
| } catch (error) { | ||
| throw new Error("Failed to retrieve Swiftly installations from disk."); |
There was a problem hiding this comment.
Should probably be logging the original error here rather than swallowing it entirely. Either that or append the message here with ${error.message}.
| let swiftly: Swiftly; | ||
|
|
||
| setup(() => { | ||
| execFileStub = sinon.stub(utilities, "execFile"); |
There was a problem hiding this comment.
We actually have some utilities that will mock and do the teardown automatically:
import { mockGlobalObject } from "../MockUtils.ts";
import * as utilities from "../../../src/utilities/utilities";
suite("Swiftly Unit Tests", () => {
const mockExecFile = mockGlobalObject(utilities, "execFile");
let swiftly: Swiftly;
...
}This way you don't need the teardown() block at all.
| const result = await swiftly.getSwiftlyToolchainInstalls(); | ||
|
|
||
| expect(result).to.deep.equal([]); | ||
| expect(execFileStub.called).to.be.false; |
There was a problem hiding this comment.
| expect(execFileStub.called).to.be.false; | |
| expect(execFileStub).to.not.have.been.called; |
| "swift-DEVELOPMENT-SNAPSHOT-2023-10-15-a" | ||
| ]); | ||
|
|
||
| expect(execFileStub.calledWith("swiftly", ["--version"])).to.be.true; |
There was a problem hiding this comment.
| expect(execFileStub.calledWith("swiftly", ["--version"])).to.be.true; | |
| expect(execFileStub).to.have.been.calledWith("swiftly", ["--version"]); |
| ]); | ||
|
|
||
| expect(execFileStub.calledWith("swiftly", ["--version"])).to.be.true; | ||
| expect(execFileStub.calledWith("swiftly", ["list-available", "--format=json"])).to.be.true; |
There was a problem hiding this comment.
| expect(execFileStub.calledWith("swiftly", ["list-available", "--format=json"])).to.be.true; | |
| expect(execFileStub).to.have.been.calledWith("swiftly", ["list-available", "--format=json"]); |
Description
Issue: To be created
Tasks