Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

renv setup - Provide users with an easy solution for renv errors #2560

Merged
merged 4 commits into from
Jan 28, 2025
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions extensions/vscode/src/eventErrors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,18 @@ type renvPackageEvtErr = baseEvtErr & {
libraryVersion: string;
};

export type RenvAction =
| "renvsetup"
| "renvinit"
| "renvsnapshot"
| "renvstatus";

export type renvSetupEvtErr = baseEvtErr & {
command: string;
action: RenvAction;
actionLabel: string;
};

export const isEvtErrDeploymentFailed = (
emsg: EventStreamMessageErrorCoded,
): emsg is EventStreamMessageErrorCoded<baseEvtErr> => {
Expand All @@ -60,6 +72,15 @@ export const isEvtErrRenvPackageSourceMissing = (
return emsg.errCode === "renvPackageSourceMissing";
};

export const isEvtErrRenvEnvironmentSetup = (
emsg: EventStreamMessageErrorCoded,
): emsg is EventStreamMessageErrorCoded<renvSetupEvtErr> => {
return (
emsg.errCode === "renvPackageNotInstalledError" ||
emsg.errCode === "renvActionRequiredError"
);
};

export const isEvtErrRequirementsReadingFailed = (
emsg: EventStreamMessageErrorCoded,
): emsg is EventStreamMessageErrorCoded<requirementsReadingEvtErr> => {
Expand Down
2 changes: 2 additions & 0 deletions extensions/vscode/src/utils/errorTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ export type ErrorCode =
| "renvPackageVersionMismatch"
| "renvPackageSourceMissing"
| "renvlockPackagesReadingError"
| "renvPackageNotInstalledError"
| "renvActionRequiredError"
| "requirementsFileReadingError"
| "deployedContentNotRunning"
| "tomlValidationError"
Expand Down
141 changes: 141 additions & 0 deletions extensions/vscode/src/utils/window.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
// Copyright (C) 2025 by Posit Software, PBC.

import { describe, expect, beforeEach, test, vi } from "vitest";
import { window } from "vscode";
import {
showErrorMessageWithTroubleshoot,
showInformationMsg,
taskWithProgressMsg,
runTerminalCommand,
} from "./window";

const terminalMock = {
sendText: vi.fn(),
show: vi.fn(),
exitStatus: {
code: 0,
},
};

vi.mock("vscode", () => {
// mock Disposable
const disposableMock = vi.fn();
disposableMock.prototype.dispose = vi.fn();

// mock window
const windowMock = {
showErrorMessage: vi.fn(),
showWarningMessage: vi.fn(),
showInformationMessage: vi.fn(),
withProgress: vi.fn().mockImplementation((_, progressCallback) => {
progressCallback();
}),
createTerminal: vi.fn().mockImplementation(() => {
terminalMock.sendText = vi.fn();
terminalMock.show = vi.fn();
return terminalMock;
}),
onDidCloseTerminal: vi.fn().mockImplementation((fn) => {
setTimeout(() => fn(terminalMock), 100);
return new disposableMock();
}),
};

return {
Disposable: disposableMock,
window: windowMock,
ProgressLocation: {
SourceControl: 1,
Window: 10,
Notification: 15,
},
};
});

describe("Consumers of vscode window", () => {
beforeEach(() => {
terminalMock.exitStatus.code = 0;
});

test("showErrorMessageWithTroubleshoot", () => {
showErrorMessageWithTroubleshoot("Bad things happened");
expect(window.showErrorMessage).toHaveBeenCalledWith(
"Bad things happened. See [Troubleshooting docs](https://github.com/posit-dev/publisher/blob/main/docs/troubleshooting.md) for help.",
);
Comment on lines +64 to +68
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding a test here. I should have done it with the introduction of the function 😅


showErrorMessageWithTroubleshoot(
"Bad things happened.",
"one",
"two",
"three",
);
expect(window.showErrorMessage).toHaveBeenCalledWith(
"Bad things happened. See [Troubleshooting docs](https://github.com/posit-dev/publisher/blob/main/docs/troubleshooting.md) for help.",
"one",
"two",
"three",
);
});

test("showInformationMsg", () => {
showInformationMsg("Good thing happened");
expect(window.showInformationMessage).toHaveBeenCalledWith(
"Good thing happened",
);

showInformationMsg("Good thing happened", "one", "two", "three");
expect(window.showInformationMessage).toHaveBeenCalledWith(
"Good thing happened",
"one",
"two",
"three",
);
});

test("taskWithProgressMsg", () => {
const taskMock = vi.fn();
taskWithProgressMsg(
"Running a task with a progress notification",
taskMock,
);
expect(window.withProgress).toHaveBeenCalledWith(
{
location: 15,
title: "Running a task with a progress notification",
cancellable: false,
},
expect.any(Function),
);
expect(taskMock).toHaveBeenCalled();
});

describe("runTerminalCommand", () => {
test("showing the terminal", async () => {
await runTerminalCommand("stat somefile.txt", true);
expect(terminalMock.sendText).toHaveBeenCalledWith("stat somefile.txt");
expect(terminalMock.show).toHaveBeenCalled();
// For terminals that we open, we don't track close events
expect(window.onDidCloseTerminal).not.toHaveBeenCalled();
});

test("NOT showing the terminal", async () => {
await runTerminalCommand("stat somefile.txt");
expect(terminalMock.sendText).toHaveBeenCalledWith("stat somefile.txt");
// For terminals that we DO NOT open, we DO track close events
expect(terminalMock.show).not.toHaveBeenCalled();
expect(window.onDidCloseTerminal).toHaveBeenCalled();
});

test("catch non zero exit status", async () => {
terminalMock.exitStatus.code = 1;
try {
await runTerminalCommand("stat somefile.txt");
} catch (_) {
expect(terminalMock.sendText).toHaveBeenCalledWith("stat somefile.txt");
// For terminals that we DO NOT open, we DO track close events
expect(terminalMock.show).not.toHaveBeenCalled();
expect(window.onDidCloseTerminal).toHaveBeenCalled();
}
});
});
});
66 changes: 65 additions & 1 deletion extensions/vscode/src/utils/window.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Copyright (C) 2025 by Posit Software, PBC.

import { window } from "vscode";
import { window, ProgressLocation, Progress, CancellationToken } from "vscode";

export function showErrorMessageWithTroubleshoot(
message: string,
Expand All @@ -14,3 +14,67 @@ export function showErrorMessageWithTroubleshoot(
" See [Troubleshooting docs](https://github.com/posit-dev/publisher/blob/main/docs/troubleshooting.md) for help.";
return window.showErrorMessage(msg, ...items);
}

export function showInformationMsg(msg: string, ...items: string[]) {
return window.showInformationMessage(msg, ...items);
}

type taskFunc = <T>(p: Progress<T>, t: CancellationToken) => Promise<void>;
const progressCallbackHandlerFactory =
(
task: taskFunc,
cancellable: boolean = false,
onCancel?: () => void,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor suggestion: could we combine the cancellable and onCancel and rely on the onCancel to determine if the handler is cancellable? If onCancel was passed it is cancellable, if not it isn't.

): taskFunc =>
(progress, token) => {
if (cancellable && onCancel) {
token.onCancellationRequested(() => {
onCancel();
});
}
return task(progress, token);
};

export function taskWithProgressMsg(
msg: string,
task: taskFunc,
cancellable: boolean = false,
onCancel?: () => void,
) {
return window.withProgress(
{
location: ProgressLocation.Notification,
title: msg,
cancellable,
},
progressCallbackHandlerFactory(task, cancellable, onCancel),
);
}

export function runTerminalCommand(
cmd: string,
show: boolean = false,
): Promise<void> {
const term = window.createTerminal();
term.sendText(cmd);

// If terminal is shown, there is no need to track exit status for it
// everything will be visible on it.
if (show) {
term.show();
return Promise.resolve();
}

return new Promise((resolve, reject) => {
const disposeToken = window.onDidCloseTerminal((closedTerminal) => {
if (closedTerminal === term) {
disposeToken.dispose();
if (term.exitStatus && term.exitStatus.code === 0) {
resolve();
} else {
reject();
}
}
});
});
}
Loading