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

marcosnav · 2025-01-28T16:44:56Z

Intent

PR with the goal to provide immediate help to users when renv errors show up while trying to deploy.

Common scenarios that it handles, providing the option within the error message to set it up easily:

No renv installed at all. Installs renv and runs init.
renv is installed but project requires init, we do the init for the user.
renv environment exists, but lockfile does not. We help with snapshot.

For any other case outside the common and known issues, we provide the user with guidance and set up the renv::status command to get more information.

Jan-28-2025.10-29-46.mp4

Resolves #2543

Type of Change

- Bug Fix
- New Feature
- Breaking Change
- Documentation
- Refactor
- Tooling

Approach

Rely more on the information renv::status provides to help users with common scenarios.

User Impact

Users with project using R and that the deployment requires a lockfile. When the lockfile or an renv environment do not exist, it'll be a matter of a click to get up to speed and continue the deployment.

Automated Tests

Created and updated unit tests for these changes.

Directions for Reviewers

The following checklist assumes you have a project that requires R:

With no renv installed at all and no lockfile on the project. (It can be uninstalled with remove.packages("renv"))
- Attempt to deploy the project. An error message should show up with an option to set things up.
- Hitting the "Setup renv" button installs and initializes the project.
- Trying to deploy one more time succeeds.
renv is installed but project does not have lockfile nor renv/.
- Attempt to deploy the project. An error message should show up with an option to set things up.
- Hitting the "Setup renv" button initializes the project.
- Trying to deploy one more time succeeds.
renv environment exists, but lockfile is not present.
- Attempt to deploy the project. An error message should show up with an option to set things up.
- Hitting the "Setup renv" button initializes the project.
- Trying to deploy one more time succeeds.

Checklist

I have updated CHANGELOG.md to cover notable changes.

…ent is not met when deploying

dotNomad

This is looking great. I tried out each of the states you described in your reviewer directions and each worked great.

I did hit an odd case with renv attempting to activate via an R profile that had an unhelpful error, but I don't think that should hold up this PR given how much this improves setting up renv

I had a few comments that are minor suggestions you can take or leave prior to merging.

dotNomad · 2025-01-28T18:21:54Z

extensions/vscode/src/views/logs.ts

+      const deploymentFailureRenvHandler = new DeploymentFailureRenvHandler();
+      if (deploymentFailureRenvHandler.shouldHandleEventMsg(msg)) {
+        return deploymentFailureRenvHandler.handle(msg).then(this.refresh);
+      }


This pattern is really clean between this and the deployHandlers file. 💯

dotNomad · 2025-01-28T18:53:27Z

internal/inspect/dependencies/renv/manifest_packages.go

@@ -246,3 +258,18 @@ func (m *defaultPackageMapper) GetManifestPackages(
 	}
 	return manifestPackages, nil
 }
+
+func (m *defaultPackageMapper) renvEnvironmentCheck(
+	base util.AbsolutePath,


It looks like base is unused here, can we remove it?

dotNomad · 2025-01-28T18:55:28Z

internal/interpreters/r.go

+}
+
+func (i *defaultRInterpreter) isRenvInstalled(rexecPath string) *types.AgentError {
+	args := []string{"-s", "-e", "cat(system.file(package = \"renv\"))"}


When I had an R profile lingering after removing renv I got a nice exit 1 error when running this. I think that is an edge case, and more about my poor job cleaning up renv, but worth noting here as a potential improvement.

dotNomad · 2025-01-28T18:56:59Z

internal/interpreters/r.go

+	// The default command suggested is renv::status()
+	renvDescError := errors.New("the renv environment for this project is not in a healthy state. Run renv::status() for more details")


This is a fantastic fallback here 👍

dotNomad · 2025-01-28T19:05:30Z

extensions/vscode/src/views/deployHandlers.test.ts

Fantastic tests here. The mocking was a lot of work, but it is very much appreciated.

dotNomad · 2025-01-28T19:06:15Z

extensions/vscode/src/utils/window.test.ts

+  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.",
+    );


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

dotNomad · 2025-01-28T19:08:02Z

extensions/vscode/src/utils/window.ts

+    cancellable: boolean = false,
+    onCancel?: () => void,


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.

dotNomad

Thanks for those changes. Everything is a toucher cleaner now 🧹

marcosnav added 2 commits January 27, 2025 13:46

Add additional checks and rely on renv package when lockfile requirem…

0e5d582

…ent is not met when deploying

Handle renv issues and suggested commands on the UI

eb00b69

marcosnav requested review from dotNomad and sagerb as code owners January 28, 2025 16:44

marcosnav changed the title ~~renv setup - Provide users with immediate solutions for renv errors~~ renv setup - Provide users with an easy solution for renv errors Jan 28, 2025

Update a few tests to work with windows paths system

44918ec

marcosnav force-pushed the mnv-renv branch from f3788ae to 44918ec Compare January 28, 2025 17:33

dotNomad approved these changes Jan 28, 2025

View reviewed changes

PR feedback

a870cb0

dotNomad approved these changes Jan 28, 2025

View reviewed changes

marcosnav merged commit 00278b7 into main Jan 28, 2025
13 checks passed

marcosnav deleted the mnv-renv branch January 28, 2025 20:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

marcosnav commented Jan 28, 2025

dotNomad left a comment •

edited

Loading

dotNomad Jan 28, 2025

dotNomad Jan 28, 2025

dotNomad Jan 28, 2025

dotNomad Jan 28, 2025

dotNomad Jan 28, 2025

dotNomad Jan 28, 2025

dotNomad Jan 28, 2025

dotNomad left a comment

		// The default command suggested is renv::status()
		renvDescError := errors.New("the renv environment for this project is not in a healthy state. Run renv::status() for more details")

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

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

Conversation

marcosnav commented Jan 28, 2025

Intent

Type of Change

Approach

User Impact

Automated Tests

Directions for Reviewers

Checklist

dotNomad left a comment • edited Loading

Choose a reason for hiding this comment

dotNomad Jan 28, 2025

Choose a reason for hiding this comment

dotNomad Jan 28, 2025

Choose a reason for hiding this comment

dotNomad Jan 28, 2025

Choose a reason for hiding this comment

dotNomad Jan 28, 2025

Choose a reason for hiding this comment

dotNomad Jan 28, 2025

Choose a reason for hiding this comment

dotNomad Jan 28, 2025

Choose a reason for hiding this comment

dotNomad Jan 28, 2025

Choose a reason for hiding this comment

dotNomad left a comment

Choose a reason for hiding this comment

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

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

dotNomad left a comment •

edited

Loading