From d683678d4ea4c1660b382a11d9ad5686ae4566eb Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Tue, 17 Dec 2024 12:38:18 -0800 Subject: [PATCH 01/28] refactor R executable into separate package for re-use, while implementing new usage priority of active R environment --- internal/initialize/initialize.go | 30 +- .../dependencies/renv/available_packages.go | 46 +- .../renv/available_packages_test.go | 54 ++- .../dependencies/renv/manifest_packages.go | 9 +- .../renv/manifest_packages_test.go | 22 +- internal/inspect/r.go | 274 ++--------- internal/inspect/r_mock.go | 19 +- internal/inspect/r_test.go | 314 +----------- internal/interpreters/r.go | 354 ++++++++++++++ internal/interpreters/r_mock.go | 82 ++++ internal/interpreters/r_test.go | 453 ++++++++++++++++++ internal/publish/bundle.go | 3 +- internal/publish/publish.go | 7 +- internal/publish/publish_test.go | 1 + internal/publish/r_package_descriptions.go | 4 +- .../services/api/get_config_r_packages.go | 4 +- internal/services/api/post_packages_r_scan.go | 16 +- .../services/api/post_packages_r_scan_test.go | 62 +-- 18 files changed, 1125 insertions(+), 629 deletions(-) create mode 100644 internal/interpreters/r.go create mode 100644 internal/interpreters/r_mock.go create mode 100644 internal/interpreters/r_test.go diff --git a/internal/initialize/initialize.go b/internal/initialize/initialize.go index 636010d68..08aee8342 100644 --- a/internal/initialize/initialize.go +++ b/internal/initialize/initialize.go @@ -10,6 +10,7 @@ import ( "github.com/posit-dev/publisher/internal/config" "github.com/posit-dev/publisher/internal/inspect" "github.com/posit-dev/publisher/internal/inspect/detectors" + "github.com/posit-dev/publisher/internal/interpreters" "github.com/posit-dev/publisher/internal/logging" "github.com/posit-dev/publisher/internal/util" ) @@ -59,17 +60,25 @@ func inspectProject(base util.AbsolutePath, python util.Path, rExecutable util.P } cfg.Python = pyConfig } - needR, err := requiresR(cfg, base) + + rInspector, err := RInspectorFactory(base, rExecutable, log) + if err != nil { + return nil, err + } + + needR, err := rInspector.RequiresR(cfg) if err != nil { + log.Debug("Error while determining R as a requirement", "error", err.Error()) return nil, err } if needR { - inspector := RInspectorFactory(base, rExecutable, log) - rConfig, err := inspector.InspectR() + rConfig, err := rInspector.InspectR() if err != nil { + log.Debug("Error while inspecting to generate an R based configuration", "error", err.Error()) return nil, err } cfg.R = rConfig + cfg.Files = append(cfg.Files, fmt.Sprint("/", cfg.R.PackageFile)) } cfg.Comments = strings.Split(initialComment, "\n") @@ -92,7 +101,7 @@ func requiresPython(cfg *config.Config, base util.AbsolutePath) (bool, error) { return exists, nil } -func requiresR(cfg *config.Config, base util.AbsolutePath) (bool, error) { +func requiresR(cfg *config.Config, rInterpreter interpreters.RInterpreter) (bool, error) { if cfg.R != nil { // InferType returned an R configuration for us to fill in. return true, nil @@ -102,8 +111,7 @@ func requiresR(cfg *config.Config, base util.AbsolutePath) (bool, error) { // unless we're deploying pre-rendered Rmd or Quarto // (where there will usually be a source file and // associated lockfile in the directory) - lockfilePath := base.Join(inspect.DefaultRenvLockfile) - exists, err := lockfilePath.Exists() + _, exists, err := rInterpreter.GetLockFilePath() if err != nil { return false, err } @@ -161,14 +169,18 @@ func normalizeConfig( cfg.Python = pyConfig cfg.Files = append(cfg.Files, fmt.Sprint("/", cfg.Python.PackageFile)) } - needR, err := requiresR(cfg, base) + + rInspector, err := RInspectorFactory(base, rExecutable, log) + if err != nil { + return err + } + needR, err := rInspector.RequiresR(cfg) if err != nil { log.Debug("Error while determining R as a requirement", "error", err.Error()) return err } if needR { - inspector := RInspectorFactory(base, rExecutable, log) - rConfig, err := inspector.InspectR() + rConfig, err := rInspector.InspectR() if err != nil { log.Debug("Error while inspecting to generate an R based configuration", "error", err.Error()) return err diff --git a/internal/inspect/dependencies/renv/available_packages.go b/internal/inspect/dependencies/renv/available_packages.go index fe051fd13..3b1b279a6 100644 --- a/internal/inspect/dependencies/renv/available_packages.go +++ b/internal/inspect/dependencies/renv/available_packages.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/posit-dev/publisher/internal/executor" + "github.com/posit-dev/publisher/internal/interpreters" "github.com/posit-dev/publisher/internal/logging" "github.com/posit-dev/publisher/internal/util" ) @@ -24,20 +25,21 @@ type AvailablePackagesLister interface { } type defaultAvailablePackagesLister struct { - base util.AbsolutePath - rExecutable util.Path - rExecutor executor.Executor + base util.AbsolutePath + rInterpreter interpreters.RInterpreter + rExecutor executor.Executor } -func NewAvailablePackageLister(base util.AbsolutePath, rExecutable util.Path) *defaultAvailablePackagesLister { - if rExecutable.String() == "" { - rExecutable = util.NewPath("R", nil) - } +func NewAvailablePackageLister(base util.AbsolutePath, rExecutable util.Path, log logging.Logger) (*defaultAvailablePackagesLister, error) { + + rInterpreter := interpreters.NewRInterpreter(base, rExecutable, log) + err := rInterpreter.Init() + return &defaultAvailablePackagesLister{ - base: base, - rExecutable: rExecutable, - rExecutor: executor.NewExecutor(), - } + base: base, + rInterpreter: rInterpreter, + rExecutor: executor.NewExecutor(), + }, err } func repoUrlsAsStrings(repos []Repository) string { @@ -70,8 +72,13 @@ func (l *defaultAvailablePackagesLister) ListAvailablePackages(repos []Repositor repoNames := repoNamesAsStrings(repos) packageListCode := fmt.Sprintf(packageListCodeTemplate, repoUrls, repoNames) + rExecutable, err := l.rInterpreter.GetRExecutable() + if err != nil { + return nil, err + } + out, _, err := l.rExecutor.RunCommand( - l.rExecutable.String(), + rExecutable.String(), []string{ "-s", "-e", @@ -109,8 +116,13 @@ func (l *defaultAvailablePackagesLister) GetBioconductorRepos(base util.Absolute escapedBase := strings.ReplaceAll(l.base.String(), `\`, `\\`) biocRepoListCode := fmt.Sprintf(bioconductorReposCodeTemplate, escapedBase) + rExecutable, err := l.rInterpreter.GetRExecutable() + if err != nil { + return nil, err + } + out, _, err := l.rExecutor.RunCommand( - l.rExecutable.String(), + rExecutable.String(), []string{ "-s", "-e", @@ -144,9 +156,15 @@ func (l *defaultAvailablePackagesLister) GetBioconductorRepos(base util.Absolute } func (l *defaultAvailablePackagesLister) GetLibPaths(log logging.Logger) ([]util.AbsolutePath, error) { + + rExecutable, err := l.rInterpreter.GetRExecutable() + if err != nil { + return nil, err + } + const getLibPathsCode = `cat(.libPaths(), sep="\n")` out, _, err := l.rExecutor.RunCommand( - l.rExecutable.String(), + rExecutable.String(), []string{ "-s", "-e", diff --git a/internal/inspect/dependencies/renv/available_packages_test.go b/internal/inspect/dependencies/renv/available_packages_test.go index 1b576b7c9..238a83255 100644 --- a/internal/inspect/dependencies/renv/available_packages_test.go +++ b/internal/inspect/dependencies/renv/available_packages_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/posit-dev/publisher/internal/executor/executortest" + "github.com/posit-dev/publisher/internal/interpreters" "github.com/posit-dev/publisher/internal/logging" "github.com/posit-dev/publisher/internal/util" "github.com/posit-dev/publisher/internal/util/utiltest" @@ -18,6 +19,7 @@ import ( type AvailablePackagesSuite struct { utiltest.Suite base util.AbsolutePath + log logging.Logger } func TestAvailablePackagesSuite(t *testing.T) { @@ -30,12 +32,24 @@ func (s *AvailablePackagesSuite) SetupTest() { err = cwd.MkdirAll(0777) s.NoError(err) s.base = cwd + s.log = logging.New() } func (s *AvailablePackagesSuite) TestListAvailablePackages() { - lister := NewAvailablePackageLister(s.base, util.Path{}) + interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { + i := interpreters.NewMockRInterpreter() + i.On("Init").Return(nil) + i.On("GetRExecutable").Return("R", nil) + return i + } + + lister, err := NewAvailablePackageLister(s.base, util.Path{}, s.log) + s.NoError(err) + + rExecutablePath := util.NewAbsolutePath("R", nil) + executor := executortest.NewMockExecutor() - executor.On("RunCommand", "R", mock.Anything, s.base, mock.Anything).Return([]byte( + executor.On("RunCommand", rExecutablePath.String(), mock.Anything, s.base, mock.Anything).Return([]byte( "pkg1 1.0 https://cran.rstudio.com/src/contrib \npkg2 2.0 https://cran.rstudio.com/src/contrib \n"), []byte{}, nil) lister.rExecutor = executor @@ -57,9 +71,18 @@ BioCbooks https://bioconductor.org/packages/3.18/books ` func (s *AvailablePackagesSuite) TestGetBioconductorRepos() { - lister := NewAvailablePackageLister(s.base, util.Path{}) + interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { + i := interpreters.NewMockRInterpreter() + i.On("Init").Return(nil) + i.On("GetRExecutable").Return("R", nil) + return i + } + + lister, _ := NewAvailablePackageLister(s.base, util.Path{}, s.log) + rExecutablePath := util.NewAbsolutePath("R", nil) + executor := executortest.NewMockExecutor() - executor.On("RunCommand", "R", mock.Anything, s.base, mock.Anything).Return([]byte(biocReposOutput), []byte{}, nil) + executor.On("RunCommand", rExecutablePath.String(), mock.Anything, s.base, mock.Anything).Return([]byte(biocReposOutput), []byte{}, nil) lister.rExecutor = executor repos, err := lister.GetBioconductorRepos(s.base, logging.New()) @@ -81,9 +104,20 @@ func (s *AvailablePackagesSuite) TestGetLibPaths() { if runtime.GOOS == "windows" { s.T().Skip() } - lister := NewAvailablePackageLister(s.base, util.Path{}) + + interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { + i := interpreters.NewMockRInterpreter() + i.On("Init").Return(nil) + i.On("GetRExecutable").Return("R", nil) + return i + } + rExecutablePath := util.NewAbsolutePath("R", nil) + + lister, err := NewAvailablePackageLister(s.base, util.NewPath("R", nil), s.log) + s.NoError(err) + executor := executortest.NewMockExecutor() - executor.On("RunCommand", "R", mock.Anything, s.base, mock.Anything).Return([]byte(libPathsOutput), []byte{}, nil) + executor.On("RunCommand", rExecutablePath.String(), mock.Anything, s.base, mock.Anything).Return([]byte(libPathsOutput), []byte{}, nil) lister.rExecutor = executor repos, err := lister.GetLibPaths(logging.New()) @@ -101,9 +135,13 @@ func (s *AvailablePackagesSuite) TestGetLibPathsWindows() { if runtime.GOOS != "windows" { s.T().Skip() } - lister := NewAvailablePackageLister(s.base, util.Path{}) + lister, err := NewAvailablePackageLister(s.base, util.Path{}, s.log) + s.NoError(err) + + rExecutablePath := util.NewAbsolutePath("R", nil) + executor := executortest.NewMockExecutor() - executor.On("RunCommand", "R", mock.Anything, s.base, mock.Anything).Return([]byte(windowsLibPathsOutput), []byte{}, nil) + executor.On("RunCommand", rExecutablePath.String(), mock.Anything, s.base, mock.Anything).Return([]byte(windowsLibPathsOutput), []byte{}, nil) lister.rExecutor = executor repos, err := lister.GetLibPaths(logging.New()) diff --git a/internal/inspect/dependencies/renv/manifest_packages.go b/internal/inspect/dependencies/renv/manifest_packages.go index 11c0c03df..6d9d87ce0 100644 --- a/internal/inspect/dependencies/renv/manifest_packages.go +++ b/internal/inspect/dependencies/renv/manifest_packages.go @@ -25,10 +25,13 @@ type defaultPackageMapper struct { lister AvailablePackagesLister } -func NewPackageMapper(base util.AbsolutePath, rExecutable util.Path) *defaultPackageMapper { +func NewPackageMapper(base util.AbsolutePath, rExecutable util.Path, log logging.Logger) (*defaultPackageMapper, error) { + + lister, err := NewAvailablePackageLister(base, rExecutable, log) + return &defaultPackageMapper{ - lister: NewAvailablePackageLister(base, rExecutable), - } + lister: lister, + }, err } func findAvailableVersion(pkgName PackageName, availablePackages []AvailablePackage) string { diff --git a/internal/inspect/dependencies/renv/manifest_packages_test.go b/internal/inspect/dependencies/renv/manifest_packages_test.go index c75586fd3..f39ea135a 100644 --- a/internal/inspect/dependencies/renv/manifest_packages_test.go +++ b/internal/inspect/dependencies/renv/manifest_packages_test.go @@ -19,6 +19,7 @@ import ( type ManifestPackagesSuite struct { utiltest.Suite testdata util.AbsolutePath + log logging.Logger } func TestManifestPackagesSuite(t *testing.T) { @@ -29,6 +30,7 @@ func (s *ManifestPackagesSuite) SetupTest() { cwd, err := util.Getwd(nil) s.NoError(err) s.testdata = cwd.Join("testdata") + s.log = logging.New() } type mockPackageLister struct { @@ -71,7 +73,9 @@ func (s *ManifestPackagesSuite) TestCRAN() { libPath := base.Join("renv_library") otherlibPath := util.NewAbsolutePath("/nonexistent", afero.NewMemMapFs()) - mapper := NewPackageMapper(base, util.Path{}) + mapper, err := NewPackageMapper(base, util.Path{}, s.log) + s.NoError(err) + lister := &mockPackageLister{} lister.On("GetLibPaths", mock.Anything).Return([]util.AbsolutePath{otherlibPath, libPath}, nil) lister.On("GetBioconductorRepos", mock.Anything, mock.Anything).Return(nil, nil) @@ -108,7 +112,9 @@ func (s *ManifestPackagesSuite) TestBioconductor() { libPath := base.Join("renv_library") otherlibPath := util.NewAbsolutePath("/nonexistent", afero.NewMemMapFs()) - mapper := NewPackageMapper(base, util.Path{}) + mapper, err := NewPackageMapper(base, util.Path{}, s.log) + s.NoError(err) + lister := &mockPackageLister{} lockfileRepos := []Repository{ {Name: "CRAN", URL: "https://cran.rstudio.com"}, @@ -171,7 +177,9 @@ func (s *ManifestPackagesSuite) TestVersionMismatch() { lockfilePath := base.Join("renv.lock") libPath := base.Join("renv_library") - mapper := NewPackageMapper(base, util.Path{}) + mapper, err := NewPackageMapper(base, util.Path{}, s.log) + s.NoError(err) + lister := &mockPackageLister{} lister.On("GetLibPaths", mock.Anything).Return([]util.AbsolutePath{libPath}, nil) lister.On("GetBioconductorRepos", mock.Anything, mock.Anything).Return(nil, nil) @@ -199,7 +207,9 @@ func (s *ManifestPackagesSuite) TestDevVersion() { lockfilePath := base.Join("renv.lock") libPath := base.Join("renv_library") - mapper := NewPackageMapper(base, util.Path{}) + mapper, err := NewPackageMapper(base, util.Path{}, s.log) + s.NoError(err) + lister := &mockPackageLister{} lister.On("GetLibPaths", mock.Anything).Return([]util.AbsolutePath{libPath}, nil) lister.On("GetBioconductorRepos", mock.Anything, mock.Anything).Return(nil, nil) @@ -226,7 +236,9 @@ func (s *ManifestPackagesSuite) TestMissingDescriptionFile() { base := s.testdata.Join("cran_project") lockfilePath := base.Join("renv.lock") - mapper := NewPackageMapper(base, util.Path{}) + mapper, err := NewPackageMapper(base, util.Path{}, s.log) + s.NoError(err) + lister := &mockPackageLister{} lister.On("GetLibPaths", mock.Anything).Return([]util.AbsolutePath{}, nil) lister.On("GetBioconductorRepos", mock.Anything, mock.Anything).Return(nil, nil) diff --git a/internal/inspect/r.go b/internal/inspect/r.go index 9b11e8eb7..fba9504b2 100644 --- a/internal/inspect/r.go +++ b/internal/inspect/r.go @@ -3,274 +3,80 @@ package inspect // Copyright (C) 2023 by Posit Software, PBC. import ( - "bytes" - "encoding/json" - "errors" - "fmt" - "io/fs" - "os" - "os/exec" - "regexp" - "strings" - "github.com/posit-dev/publisher/internal/config" "github.com/posit-dev/publisher/internal/executor" + "github.com/posit-dev/publisher/internal/interpreters" "github.com/posit-dev/publisher/internal/logging" - "github.com/posit-dev/publisher/internal/types" "github.com/posit-dev/publisher/internal/util" ) type RInspector interface { InspectR() (*config.R, error) - CreateLockfile(lockfilePath util.AbsolutePath) error + RequiresR(*config.Config) (bool, error) } type defaultRInspector struct { - base util.AbsolutePath - executor executor.Executor - pathLooker util.PathLooker - rExecutable util.Path - log logging.Logger + base util.AbsolutePath + executor executor.Executor + pathLooker util.PathLooker + rInterpreter interpreters.RInterpreter + log logging.Logger } var _ RInspector = &defaultRInspector{} -const DefaultRenvLockfile = "renv.lock" +func NewRInspector(base util.AbsolutePath, rExecutable util.Path, log logging.Logger) (RInspector, error) { -var rVersionCache = make(map[string]string) + rInterpreter := interpreters.NewRInterpreter(base, rExecutable, log) + err := rInterpreter.Init() -func NewRInspector(base util.AbsolutePath, rExecutable util.Path, log logging.Logger) RInspector { return &defaultRInspector{ - base: base, - executor: executor.NewExecutor(), - pathLooker: util.NewPathLooker(), - rExecutable: rExecutable, - log: log, - } + base: base, + executor: executor.NewExecutor(), + pathLooker: util.NewPathLooker(), + rInterpreter: rInterpreter, + log: log, + }, err } // InspectR inspects the specified project directory, // returning an R configuration. -// If R is available, use it to determine the renv lockfile path -// (to support renv profiles). Otherwise, look for renv.lock. -// If there's a lockfile, we get the R version from there. -// Otherwise, we run R to get the version (and if it's not -// available, that's an error). func (i *defaultRInspector) InspectR() (*config.R, error) { - lockfilePath := i.base.Join(DefaultRenvLockfile) - exists, err := lockfilePath.Exists() + _, err := i.rInterpreter.GetRExecutable() if err != nil { - i.log.Debug("Error while looking up renv lock file", "renv_lock", lockfilePath) - return nil, err - } - - var rExecutable string - var getRExecutableErr error - - if !exists { - // Maybe R can give us the lockfile path (e.g. from an renv profile) - i.log.Debug("Attempting to get renv lock file via R executable") - rExecutable, getRExecutableErr = i.getRExecutable() - if getRExecutableErr == nil { - lockfilePath, err = i.getRenvLockfile(rExecutable) - if err != nil { - i.log.Debug("Error attempting to get renv lockfile path via R executable", "r_executable", rExecutable, "error", err.Error()) - return nil, err - } - exists, err = lockfilePath.Exists() - if err != nil { - i.log.Debug("Error asserting renv lockfile exists", "renv_lock", lockfilePath, "error", err.Error()) - return nil, err - } - if exists { - i.log.Debug("renv lockfile found via R executable", "r_executable", rExecutable, "renv_lock", lockfilePath) - } - } // else stay with the default lockfile path - } - - var rVersion string - - if exists { - // Get R version from the lockfile - rVersion, err = i.getRVersionFromLockfile(lockfilePath) - if err != nil { - i.log.Debug("Error getting R version from existing renv lockfile. Attempting to get R version from executable", "renv_lock", lockfilePath, "error", err.Error()) - rExecutable, getRExecutableErr = i.getRExecutable() - } + i.log.Debug("Error retrieving R Executable", "error", err) } - - // renv.lock exists but could not be read - // thus, R version is still empty - renvFallback := exists && rVersion == "" - if !exists || renvFallback { - // Now R is required, err if it couldn't be found. - if getRExecutableErr != nil { - i.log.Debug("R is required and could not be found", "error", getRExecutableErr) - return nil, getRExecutableErr - } - rVersion, err = i.getRVersion(rExecutable) - if err != nil { - i.log.Debug("Error while looking up for R version from executable", "r_executable", rExecutable, "error", err.Error()) - return nil, err - } + version, err := i.rInterpreter.GetRVersion() + if err != nil { + i.log.Debug("Error retrieving R Version", "error", err) } - lockfileRelPath, err := lockfilePath.Rel(i.base) + packageFile, _, err := i.rInterpreter.GetLockFilePath() if err != nil { - i.log.Debug("Error getting relative path for renv lockfile", "basepath", i.base.String(), "error", err.Error()) - return nil, err + i.log.Debug("Error retrieving R package lock file", "error", err) } + return &config.R{ - Version: rVersion, - PackageFile: lockfileRelPath.String(), + Version: version, + PackageFile: packageFile.String(), PackageManager: "renv", }, nil } -// CreateLockfile creates a lockfile at the specified path -// by invoking R to run `renv::snapshot()`. -func (i *defaultRInspector) CreateLockfile(lockfilePath util.AbsolutePath) error { - rExecutable, err := i.getRExecutable() - if err != nil { - return err - } - i.log.Info("Creating renv lockfile", "path", lockfilePath.String(), "r", rExecutable) - - err = lockfilePath.Dir().MkdirAll(0777) - if err != nil { - return err - } - - escaped := strings.ReplaceAll(lockfilePath.String(), `\`, `\\`) - code := fmt.Sprintf(`renv::snapshot(lockfile="%s")`, escaped) - args := []string{"-s", "-e", code} - stdout, stderr, err := i.executor.RunCommand(rExecutable, args, i.base, i.log) - i.log.Debug("renv::snapshot()", "out", string(stdout), "err", string(stderr)) - return err -} - -func (i *defaultRInspector) validateRExecutable(rExecutable string) error { - _, err := i.getRVersion(rExecutable) - if err != nil { - return fmt.Errorf("could not run R executable '%s': %w", rExecutable, err) - } - return nil -} - -func (i *defaultRInspector) getRExecutable() (string, error) { - if i.rExecutable.String() != "" { - // User-provided R executable - exists, err := i.rExecutable.Exists() +func (i *defaultRInspector) RequiresR(cfg *config.Config) (bool, error) { + if cfg.R != nil { + // InferType returned an R configuration for us to fill in. + return true, nil + } + if cfg.Type != config.ContentTypeHTML && !cfg.Type.IsPythonContent() { + // Presence of renv.lock implies R is needed, + // unless we're deploying pre-rendered Rmd or Quarto + // (where there will usually be a source file and + // associated lockfile in the directory) + _, exists, err := i.rInterpreter.GetLockFilePath() if err != nil { - return "", err - } - if exists { - return i.rExecutable.String(), nil - } - noExecErr := fmt.Errorf( - "cannot find the specified R executable %s: %w", - i.rExecutable, fs.ErrNotExist) - return "", types.NewAgentError(types.ErrorRExecNotFound, noExecErr, nil) - } - // Find the executable on PATH - var path string - var err error - - i.log.Info("Looking for R on PATH", "PATH", os.Getenv("PATH")) - path, err = i.pathLooker.LookPath("R") - if err == nil { - // Ensure the R is actually runnable. - err = i.validateRExecutable(path) - if err == nil { - return path, nil + return false, err } + return exists, nil } - - if errors.Is(err, exec.ErrNotFound) { - return "", types.NewAgentError(types.ErrorRExecNotFound, err, nil) - } - - return "", err -} - -var rVersionRE = regexp.MustCompile(`^R version (\d+\.\d+\.\d+)`) - -func (i *defaultRInspector) getRVersion(rExecutable string) (string, error) { - if version, ok := rVersionCache[rExecutable]; ok { - return version, nil - } - i.log.Info("Getting R version", "r", rExecutable) - args := []string{"--version"} - output, stderr, err := i.executor.RunCommand(rExecutable, args, util.AbsolutePath{}, i.log) - if err != nil { - return "", err - } - lines := strings.SplitN(string(append(output, stderr...)), "\n", -1) - for _, l := range lines { - i.log.Info("Parsing line for R version", "l", l) - m := rVersionRE.FindStringSubmatch(l) - if len(m) < 2 { - continue - } - version := m[1] - i.log.Info("Detected R version", "version", version) - rVersionCache[rExecutable] = version - return version, nil - } - return "", fmt.Errorf("couldn't parse R version from command output (%s --version)", rExecutable) -} - -var renvLockRE = regexp.MustCompile(`^\[1\] "(.*)"`) - -func (i *defaultRInspector) getRenvLockfile(rExecutable string) (util.AbsolutePath, error) { - defaultLockfilePath := i.base.Join(DefaultRenvLockfile) - exists, err := defaultLockfilePath.Exists() - if err != nil { - return util.AbsolutePath{}, err - } - if exists { - i.log.Info("Found default renv lockfile", "path", defaultLockfilePath.String()) - return defaultLockfilePath, nil - } - i.log.Info("Getting renv lockfile path", "r", rExecutable) - args := []string{"-s", "-e", "renv::paths$lockfile()"} - output, _, err := i.executor.RunCommand(rExecutable, args, i.base, i.log) - if err != nil { - if _, ok := err.(*exec.ExitError); ok { - i.log.Warn("Couldn't detect lockfile path; is renv installed?") - return i.base.Join(DefaultRenvLockfile), nil - } else { - return util.AbsolutePath{}, err - } - } - for _, line := range strings.Split(string(output), "\n") { - m := renvLockRE.FindStringSubmatch(line) - if len(m) < 2 { - continue - } - // paths$lockfile returns an absolute path - path := m[1] - i.log.Info("renv::paths$lockfile returned lockfile path", "path", path) - return util.NewAbsolutePath(path, nil), nil - } - return util.AbsolutePath{}, fmt.Errorf("couldn't parse renv lockfile path from output: %s", output) -} - -type renvLockfile struct { - // Only the fields we use are here - R struct { - Version string - } -} - -func (i *defaultRInspector) getRVersionFromLockfile(lockfilePath util.AbsolutePath) (string, error) { - content, err := lockfilePath.ReadFile() - if err != nil { - return "", err - } - var lockfileContent renvLockfile - err = json.NewDecoder(bytes.NewReader(content)).Decode(&lockfileContent) - if err != nil { - return "", err - } - return lockfileContent.R.Version, nil + return false, nil } diff --git a/internal/inspect/r_mock.go b/internal/inspect/r_mock.go index b0fb66dcc..08598965d 100644 --- a/internal/inspect/r_mock.go +++ b/internal/inspect/r_mock.go @@ -3,8 +3,9 @@ package inspect // Copyright (C) 2024 by Posit Software, PBC. import ( + "errors" + "github.com/posit-dev/publisher/internal/config" - "github.com/posit-dev/publisher/internal/util" "github.com/stretchr/testify/mock" ) @@ -26,7 +27,17 @@ func (m *MockRInspector) InspectR() (*config.R, error) { } } -func (m *MockRInspector) CreateLockfile(lockfilePath util.AbsolutePath) error { - args := m.Called(lockfilePath) - return args.Error(0) +func (m *MockRInspector) RequiresR(*config.Config) (bool, error) { + args := m.Called() + arg0 := args.Get(0) + if arg0 == nil { + return false, args.Error(1) + } else { + var i interface{} = arg0 + if b, ok := i.(bool); ok { + return b, args.Error(1) + } else { + return false, errors.New("invalid boolean argument") + } + } } diff --git a/internal/inspect/r_test.go b/internal/inspect/r_test.go index 0032b8c97..56b1efe26 100644 --- a/internal/inspect/r_test.go +++ b/internal/inspect/r_test.go @@ -3,18 +3,12 @@ package inspect // Copyright (C) 2023 by Posit Software, PBC. import ( - "errors" - "os/exec" - "runtime" "testing" - "github.com/posit-dev/publisher/internal/executor/executortest" "github.com/posit-dev/publisher/internal/logging" - "github.com/posit-dev/publisher/internal/types" "github.com/posit-dev/publisher/internal/util" "github.com/posit-dev/publisher/internal/util/utiltest" "github.com/spf13/afero" - "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" ) @@ -33,318 +27,16 @@ func (s *RSuite) SetupTest() { s.cwd = cwd err = cwd.MkdirAll(0700) s.NoError(err) - - rVersionCache = make(map[string]string) } func (s *RSuite) TestNewRInspector() { log := logging.New() rPath := util.NewPath("/usr/bin/R", nil) - i := NewRInspector(s.cwd, rPath, log) - inspector := i.(*defaultRInspector) - s.Equal(rPath, inspector.rExecutable) - s.Equal(log, inspector.log) -} - -type OutputTestData struct { - output, expectedVersion string -} - -func getOutputTestData() []OutputTestData { - data := []OutputTestData{ - // typical output from command `r --version` - {`R version 4.3.0 (2023-04-21) -- "Already Tomorrow" -Copyright (C) 2023 The R Foundation for Statistical Computing -Platform: x86_64-apple-darwin20 (64-bit) - -R is free software and comes with ABSOLUTELY NO WARRANTY. -You are welcome to redistribute it under the terms of the -GNU General Public License versions 2 or 3. -For more information about these matters see -https://www.gnu.org/licenses/. -`, "4.3.0"}, - // output when there is a warning - {`WARNING: ignoring environment value of R_HOME -R version 4.3.3 (2024-02-29) -- "Angel Food Cake" -Copyright (C) 2024 The R Foundation for Statistical Computing -Platform: x86_64-apple-darwin20 (64-bit) - -R is free software and comes with ABSOLUTELY NO WARRANTY. -You are welcome to redistribute it under the terms of the -GNU General Public License versions 2 or 3. -For more information about these matters see -https://www.gnu.org/licenses/.`, "4.3.3"}, - - // output when there are multiple warnings - // as well as closely matching version strings - {`WARNING: ignoring environment value of R_HOME -WARNING: your mom is calling -WARNING: time to stand -Somewhere below is the correct R version 4.3.* that we're looking for -R version 4.3.3 (2024-02-29) -- "Angel Food Cake" -Copyright (C) 2024 The R Foundation for Statistical Computing -Platform: x86_64-apple-darwin20 (64-bit) - -R is free software and comes with ABSOLUTELY NO WARRANTY. -You are welcome to redistribute it under the terms of the -GNU General Public License versions 2 or 3. -For more information about these matters see -https://www.gnu.org/licenses/.`, "4.3.3"}, - - // test output where version exists in multiple locations - // we want to get it from the first location - {` -R version 4.3.3 (2024-02-29) -- "Angel Food Cake" -Copyright (C) 2024 The R Foundation for Statistical Computing -Platform: x86_64-apple-darwin20 (64-bit) -R version 4.1.1 (2023-12-29) -- "Fantasy Island" - -R is free software and comes with ABSOLUTELY NO WARRANTY. -You are welcome to redistribute it under the terms of the -GNU General Public License versions 2 or 3. -For more information about these matters see -https://www.gnu.org/licenses/.`, "4.3.3"}, - } - return data -} - -func (s *RSuite) TestGetRVersionFromExecutable() { - for _, tc := range getOutputTestData() { - s.SetupTest() - log := logging.New() - rPath := s.cwd.Join("bin", "R") - rPath.Dir().MkdirAll(0777) - rPath.WriteFile(nil, 0777) - i := NewRInspector(s.cwd, rPath.Path, log) - inspector := i.(*defaultRInspector) - - executor := executortest.NewMockExecutor() - executor.On("RunCommand", rPath.String(), []string{"--version"}, mock.Anything, mock.Anything).Return([]byte(tc.output), nil, nil) - inspector.executor = executor - version, err := inspector.getRVersion(rPath.String()) - s.NoError(err) - s.Equal(tc.expectedVersion, version) - } -} - -func (s *RSuite) TestGetRVersionFromExecutableWindows() { - for _, tc := range getOutputTestData() { - s.SetupTest() - // R on Windows emits version information on stderr - log := logging.New() - rPath := s.cwd.Join("bin", "R") - rPath.Dir().MkdirAll(0777) - rPath.WriteFile(nil, 0777) - i := NewRInspector(s.cwd, rPath.Path, log) - inspector := i.(*defaultRInspector) - - executor := executortest.NewMockExecutor() - executor.On("RunCommand", rPath.String(), []string{"--version"}, mock.Anything, mock.Anything).Return(nil, []byte(tc.output), nil) - inspector.executor = executor - version, err := inspector.getRVersion(rPath.String()) - s.NoError(err) - s.Equal(tc.expectedVersion, version) - } -} - -func (s *RSuite) TestGetRVersionFromRealDefaultR() { - // This test can only run if R or R is on the PATH. - rPath, err := exec.LookPath("R") - if err != nil { - s.T().Skip("This test requires R to be available on PATH") - } - log := logging.New() - i := NewRInspector(s.cwd, util.Path{}, log) - inspector := i.(*defaultRInspector) - _, err = inspector.getRVersion(rPath) - s.NoError(err) -} - -func (s *RSuite) TestGetRenvLockfileFromDir() { - log := logging.New() - i := NewRInspector(s.cwd, util.Path{}, log) - inspector := i.(*defaultRInspector) - - expectedPath := s.cwd.Join(DefaultRenvLockfile) - err := expectedPath.WriteFile(nil, 0666) - s.NoError(err) - - // Executor should not be called - executor := executortest.NewMockExecutor() - inspector.executor = executor - - lockfilePath, err := inspector.getRenvLockfile("") - s.NoError(err) - s.Equal(expectedPath, lockfilePath) -} - -func (s *RSuite) TestGetRenvLockfileFromR() { - log := logging.New() - rPath := s.cwd.Join("bin", "R") - rPath.Dir().MkdirAll(0777) - rPath.WriteFile(nil, 0777) - i := NewRInspector(s.cwd, rPath.Path, log) - inspector := i.(*defaultRInspector) - - const getRenvLockOutput = "[1] \"/project/renv.lock\"\n" - executor := executortest.NewMockExecutor() - executor.On("RunCommand", rPath.String(), mock.Anything, mock.Anything, mock.Anything).Return([]byte(getRenvLockOutput), nil, nil) - inspector.executor = executor - lockfilePath, err := inspector.getRenvLockfile(rPath.String()) - s.NoError(err) - expected := util.NewAbsolutePath("/project/renv.lock", nil).String() - s.Equal(expected, lockfilePath.String()) -} - -const getRenvLockMismatchedOutput = `ℹ Using R 4.3.0 (lockfile was generated with R 3.6.3) -[1] "/project/renv.lock" -` - -func (s *RSuite) TestGetRenvLockfileMismatchedVersion() { - log := logging.New() - rPath := s.cwd.Join("bin", "R") - rPath.Dir().MkdirAll(0777) - rPath.WriteFile(nil, 0777) - i := NewRInspector(s.cwd, rPath.Path, log) - inspector := i.(*defaultRInspector) - - executor := executortest.NewMockExecutor() - executor.On("RunCommand", rPath.String(), mock.Anything, mock.Anything, mock.Anything).Return([]byte(getRenvLockMismatchedOutput), nil, nil) - inspector.executor = executor - lockfilePath, err := inspector.getRenvLockfile(rPath.String()) - s.NoError(err) - expected := util.NewAbsolutePath("/project/renv.lock", nil).String() - s.Equal(expected, lockfilePath.String()) -} - -func (s *RSuite) TestGetRenvLockfileRExitCode() { - if runtime.GOOS == "windows" { - s.T().Skip("This test does not run on Windows") - } - log := logging.New() - rPath := util.NewPath("/usr/bin/false", nil) - i := NewRInspector(s.cwd, rPath, log) - inspector := i.(*defaultRInspector) - - // if the R call fails, we get back a default lockfile path - lockfilePath, err := inspector.getRenvLockfile(rPath.String()) - s.NoError(err) - s.Equal(s.cwd.Join("renv.lock").String(), lockfilePath.String()) -} - -func (s *RSuite) TestGetRenvLockfileRError() { - log := logging.New() - rPath := s.cwd.Join("bin", "R") - rPath.Dir().MkdirAll(0777) - rPath.WriteFile(nil, 0777) - i := NewRInspector(s.cwd, rPath.Path, log) - inspector := i.(*defaultRInspector) - - testError := errors.New("test error from RunCommand") - executor := executortest.NewMockExecutor() - executor.On("RunCommand", rPath.String(), mock.Anything, mock.Anything, mock.Anything).Return(nil, nil, testError) - inspector.executor = executor - lockfilePath, err := inspector.getRenvLockfile(rPath.String()) - s.ErrorIs(err, testError) - s.Equal("", lockfilePath.String()) -} - -const lockFileContent = `{ - "R": { - "Version": "4.3.1", - "Repositories": [ - { - "Name": "CRAN", - "URL": "https://cloud.r-project.org" - } - ] - }, - "Packages": {} -}` - -func (s *RSuite) TestGetRVersionFromLockFile() { - log := logging.New() - i := NewRInspector(s.cwd, util.Path{}, log) - inspector := i.(*defaultRInspector) - - lockfilePath := s.cwd.Join("renv.lock") - err := lockfilePath.WriteFile([]byte(lockFileContent), 0666) - s.NoError(err) - version, err := inspector.getRVersionFromLockfile(lockfilePath) + i, err := NewRInspector(s.cwd, rPath, log) s.NoError(err) - s.Equal("4.3.1", version) -} - -func (s *RSuite) TestGetRExecutable() { - for _, tc := range getOutputTestData() { - log := logging.New() - executor := executortest.NewMockExecutor() - executor.On("RunCommand", "/some/R", []string{"--version"}, mock.Anything, mock.Anything).Return([]byte(tc.output), nil, nil) - i := &defaultRInspector{ - executor: executor, - log: log, - } - - pathLooker := util.NewMockPathLooker() - pathLooker.On("LookPath", "R").Return("/some/R", nil) - i.pathLooker = pathLooker - executable, err := i.getRExecutable() - s.NoError(err) - s.Equal("/some/R", executable) - } -} - -func (s *RSuite) TestGetRExecutableSpecifiedR() { - log := logging.New() - rPath := s.cwd.Join("bin", "R") - rPath.Dir().MkdirAll(0777) - rPath.WriteFile(nil, 0777) - - executor := executortest.NewMockExecutor() - executor.On("RunCommand", "/some/R", []string{"--version"}, mock.Anything, mock.Anything).Return(nil, nil, nil) - i := &defaultRInspector{ - rExecutable: rPath.Path, - executor: executor, - log: log, - } - - executable, err := i.getRExecutable() - s.NoError(err) - s.Equal(rPath.String(), executable) -} -func (s *RSuite) TestGetRExecutableSpecifiedRNotFound() { - log := logging.New() - - i := NewRInspector(s.cwd, util.NewPath("/some/R", nil), log) inspector := i.(*defaultRInspector) - executable, err := inspector.getRExecutable() - - aerr, yes := types.IsAgentError(err) - s.Equal(yes, true) - s.Equal(aerr.Code, types.ErrorRExecNotFound) - s.Contains(aerr.Message, "Cannot find the specified R executable /some/R: file does not exist.") - s.Equal("", executable) -} - -func (s *RSuite) TestGetRExecutableNotRunnable() { - log := logging.New() - - testError := errors.New("test error from RunCommand") - executor := executortest.NewMockExecutor() - executor.On("RunCommand", "/some/R", []string{"--version"}, mock.Anything, mock.Anything).Return(nil, nil, testError) - i := &defaultRInspector{ - executor: executor, - log: log, - } - - pathLooker := util.NewMockPathLooker() - pathLooker.On("LookPath", "R").Return("/some/R", nil) - i.pathLooker = pathLooker - - executable, err := i.getRExecutable() - s.NotNil(err) - s.ErrorIs(err, testError) - s.Equal("", executable) + s.Equal(s.cwd, inspector.base) + s.Equal(log, inspector.log) } diff --git a/internal/interpreters/r.go b/internal/interpreters/r.go new file mode 100644 index 000000000..0e4666e3d --- /dev/null +++ b/internal/interpreters/r.go @@ -0,0 +1,354 @@ +package interpreters + +import ( + "errors" + "fmt" + "os" + "os/exec" + "regexp" + "strings" + + "github.com/spf13/afero" + + "github.com/posit-dev/publisher/internal/executor" + "github.com/posit-dev/publisher/internal/logging" + "github.com/posit-dev/publisher/internal/types" + "github.com/posit-dev/publisher/internal/util" +) + +// Copyright (C) 2023 by Posit Software, PBC. + +const DefaultRenvLockfile = "renv.lock" + +var NotYetInitialized = types.NewAgentError(types.ErrorRExecNotFound, errors.New("Not yet initialized"), nil) +var MissingRError = types.NewAgentError(types.ErrorRExecNotFound, errors.New("Unable to detect any R interpreters"), nil) +var InvalidR = types.NewAgentError(types.ErrorRExecNotFound, errors.New("R executable is invalid"), nil) + +type RInterpreter interface { + Init() error + + GetRExecutable() (util.AbsolutePath, error) + GetRVersion() (string, error) + GetLockFilePath() (util.RelativePath, bool, error) + CreateLockfile(util.AbsolutePath) error +} + +type defaultRInterpreter struct { + executor executor.Executor + pathLooker util.PathLooker + + base util.AbsolutePath + preferredPath util.Path + rExecutable util.AbsolutePath + version string + lockfileRelPath util.RelativePath + lockfileExists bool + log logging.Logger + initialized bool + fs afero.Fs +} + +var _ RInterpreter = &defaultRInterpreter{} + +var NewRInterpreter = RInterpreterFactory + +func RInterpreterFactory(base util.AbsolutePath, + rExecutableParam util.Path, log logging.Logger) RInterpreter { + + return &defaultRInterpreter{ + executor: executor.NewExecutor(), + pathLooker: util.NewPathLooker(), + + base: base, + preferredPath: rExecutableParam, + rExecutable: util.AbsolutePath{}, + version: "", + lockfileRelPath: util.RelativePath{}, + lockfileExists: false, + log: log, + initialized: false, + fs: nil, + } +} + +// Initializes the attributes within the defaultRInterpreter structure +// 1. Resolves the path to the rExecutable to be used, with a preference +// towards the perferredPath, but otherwise, first one on path. If the +// executable is not a valid R interpreter, then will not be set. +// 2. Seeds the version of the rExecutable being used, if set. +// 3. Seeds the renv lock file for the rExecutable being used or if not found +// then the path to the default lock file +// 4. Seeds the existance of the lock file at the lockfileRelPath +// +// Errors are taken care of internally and determine the setting or non-setting +// of the attributes. +func (i *defaultRInterpreter) Init() error { + // entire flow doesn't need to occur to be initialized. Just want to be sure + // some of it was attempted to run + i.initialized = true + + // This will set the rExecutable and version for us + err := i.resolveRExecutable() + if err != nil { + return err + } + + // This will set lockfileRelPath and lockfileExists for us + err = i.resolveRenvLockFile(i.rExecutable.String()) + if err != nil { + return err + } + + return nil +} + +func (i *defaultRInterpreter) GetRExecutable() (util.AbsolutePath, error) { + if !i.initialized { + return util.AbsolutePath{}, NotYetInitialized + } + if i.IsRExecutableValid() { + return i.rExecutable, nil + } + return util.AbsolutePath{}, MissingRError +} + +func (i *defaultRInterpreter) GetRVersion() (string, error) { + if !i.initialized { + return "", NotYetInitialized + } + if i.IsRExecutableValid() { + return i.version, nil + } + return "", MissingRError +} + +func (i *defaultRInterpreter) GetLockFilePath() (relativePath util.RelativePath, exists bool, err error) { + if !i.initialized { + return util.RelativePath{}, false, NotYetInitialized + } + if i.initialized { + return i.lockfileRelPath, i.lockfileExists, nil + } + return util.RelativePath{}, false, NotYetInitialized +} + +func (i *defaultRInterpreter) IsRExecutableValid() bool { + return i.initialized && i.rExecutable.Path.String() != "" && i.version != "" +} + +// Determine which R Executable to use by: +// 1. If provided by user, (This is used to pass in selected versions from Positron and Extensions, +// as well as from CLI). +// 2. If not provided, then identify first R interpreter on PATH. +// +// Will fail if R executable does not physically exist or is not executable +// If successful, this will update to the rExecutable and its associated version within +// the defaultRInterpreter struct +func (i *defaultRInterpreter) resolveRExecutable() error { + var rExecutable = util.AbsolutePath{} + + // Passed in path to executable + if i.preferredPath.String() != "" { + // User-provided R executable + exists, err := i.preferredPath.Exists() + if err != nil { + i.log.Warn("Unable to check existence of preferred R interpreter. Proceeding with discovery.", "preferredPath", i.preferredPath, "error", err) + } else { + if exists { + rExecutable = util.NewAbsolutePath(i.preferredPath.String(), i.fs) + } else { + i.log.Warn("Preferred R interpreter does not exist. Proceeding with discovery.", "preferredPath", i.preferredPath) + } + } + } + + // If we don't have one yet... + if rExecutable.Path.String() == "" { + // Find the executable on PATH + var path string + var err error + + i.log.Debug("Looking for R on PATH", "PATH", os.Getenv("PATH")) + path, err = i.pathLooker.LookPath("R") + if err == nil { + i.log.Debug("Found R executable from PATH", "path", path) + tempRPath := util.NewPath(path, i.fs) + // make sure it exists + exists, err := tempRPath.Exists() + if err != nil { + i.log.Warn("Unable to check existence of R interpreter on PATH. Proceeding with discovery.", "path", path, "error", err) + } else { + if exists { + rExecutable = util.NewAbsolutePath(tempRPath.String(), i.fs) + } else { + i.log.Warn("R interpreter on PATH does not exist. Proceeding with discovery.", "path", path) + } + } + } else { + if errors.Is(err, exec.ErrNotFound) { + i.log.Debug("R executable not found on PATH. Proceeding with discovery.") + } else { + i.log.Debug("Unable to search path for R executable", "error", err) + } + } + } + + // If we still don't have one, then we ca + if rExecutable.Path.String() == "" { + return MissingRError + } + + // Need to validate the executable, so let's ask it for the version + i.log.Debug("Validating path to R executable found", "path", rExecutable) + // Ensure the R is actually runnable. + version, err := i.ValidateRExecutable(rExecutable.String()) + if err == nil { + i.log.Debug("Successful validation for R executable", "rExecutable", rExecutable) + } else { + i.log.Debug("R executable from PATH is invalid.", "rExecutable", rExecutable, "error", err) + return err + } + + // all is good! + i.rExecutable = util.NewAbsolutePath(rExecutable.String(), i.fs) + i.version = version + return nil +} + +// We assume if we can get a version from the rExecutable passed in, that it +// is really an R Executable. +func (i *defaultRInterpreter) ValidateRExecutable(rExecutable string) (string, error) { + if !i.initialized { + return "", NotYetInitialized + } + version, err := i.getRVersionFromRExecutable(rExecutable) + if err != nil { + i.log.Debug("could not run R executable", "rExecutable", rExecutable, "error", err) + return "", err + } + return version, nil +} + +// Retrieve the version of R from the rExecutable passed in +// This function searches the output for a very specific text pattern (see the regex `rVersionRE`) +// and only returns the version if found in that way. This allows us to confirm it is an R interpreter +// versus any other executable which might return a version. +func (i *defaultRInterpreter) getRVersionFromRExecutable(rExecutable string) (string, error) { + var rVersionRE = regexp.MustCompile(`^R version (\d+\.\d+\.\d+)`) + + i.log.Info("Getting R version", "r", rExecutable) + args := []string{"--version"} + output, stderr, err := i.executor.RunCommand(rExecutable, args, util.AbsolutePath{}, i.log) + if err != nil { + return "", err + } + lines := strings.SplitN(string(append(output, stderr...)), "\n", -1) + for _, l := range lines { + i.log.Info("Parsing line for R version", "l", l) + m := rVersionRE.FindStringSubmatch(l) + if len(m) < 2 { + continue + } + version := m[1] + i.log.Info("Detected R version", "version", version) + return version, nil + } + return "", fmt.Errorf("couldn't parse R version from command output (%s --version)", rExecutable) +} + +// Determine if the configured (or not configured) renv lock file exists +// by falling through a specific search criteria +// 1. Does the R executable indicate a renv lock file path? +// 2. Use the default lock file path +// 3. Does the lock file exist +// +// Return path, existence and any error if encountered. +func (i *defaultRInterpreter) resolveRenvLockFile(rExecutable string) error { + lockfilePath, err := i.getRenvLockfilePathFromRExecutable(rExecutable) + if err == nil { + i.log.Debug("renv lockfile found via R executable", "renv_lock", lockfilePath) + } else { + // we'll handle the error by looking elsewhere + + i.log.Debug("Unable to get renv lockfile path via R executable", "error", err.Error()) + // we'll default if we can't get it from R executable + lockfilePath = i.base.Join(DefaultRenvLockfile) + i.log.Debug("using default renv lockfile", "lockfilePath", lockfilePath) + } + + lockfileRelPath, err := lockfilePath.Rel(i.base) + if err != nil { + i.log.Debug("Error getting relative path for renv lockfile", "basepath", i.base.String(), "error", err.Error()) + return err + } + + lockfileExists, err := lockfilePath.Exists() + if err != nil { + i.log.Debug("Error while confirming existence of renv lock file", "renv_lock", lockfilePath, "error", err.Error()) + return err + } + + i.lockfileRelPath = lockfileRelPath + i.lockfileExists = lockfileExists + return nil +} + +// Determine the renv lock file as configured. (renv::init() allows for different file names for lock file) +// We can get the renv lockfile path from the active R executable +// If we can't then we return an error. Using default is responsibility of caller +// NOTE: Do not need to be initialized for this functionality, since it operates on external rExecutable +func (i *defaultRInterpreter) getRenvLockfilePathFromRExecutable(rExecutable string) (util.AbsolutePath, error) { + var renvLockRE = regexp.MustCompile(`^\[1\] "(.*)"`) + + i.log.Info("Getting renv lockfile path from R executable", "r", rExecutable) + args := []string{"-s", "-e", "renv::paths$lockfile()"} + output, stderr, err := i.executor.RunCommand(rExecutable, args, i.base, i.log) + if err != nil { + if _, ok := err.(*exec.ExitError); ok { + i.log.Warn("Couldn't detect lockfile path from R executable (renv::paths$lockfile()); is renv installed?") + } else { + i.log.Warn("Error running R executable", "args", args) + } + return util.AbsolutePath{}, err + } + lines := strings.SplitN(string(append(output, stderr...)), "\n", -1) + for _, l := range lines { + i.log.Info("Parsing line for renv::path output", "l", l) + m := renvLockRE.FindStringSubmatch(l) + if len(m) < 2 { + continue + } + // paths$lockfile returns an absolute path + path := m[1] + i.log.Info("renv::paths$lockfile returned lockfile path", "path", path) + return util.NewAbsolutePath(path, i.fs), nil + } + i.log.Warn("couldn't parse renv lockfile path from renv::paths$lockfile", "output", output) + return util.AbsolutePath{}, errors.New("couldn't parse renv lockfile path from renv::paths$lockfile") +} + +// CreateLockfile creates a lockfile at the specified path +// by invoking R to run `renv::snapshot()`. +func (i *defaultRInterpreter) CreateLockfile(lockfilePath util.AbsolutePath) error { + if !i.initialized { + return NotYetInitialized + } + + rExecutable, err := i.GetRExecutable() + if err != nil { + return err + } + i.log.Info("Creating renv lockfile", "path", lockfilePath.String(), "r", rExecutable) + + err = lockfilePath.Dir().MkdirAll(0777) + if err != nil { + return err + } + + escaped := strings.ReplaceAll(lockfilePath.String(), `\`, `\\`) + code := fmt.Sprintf(`renv::snapshot(lockfile="%s")`, escaped) + args := []string{"-s", "-e", code} + stdout, stderr, err := i.executor.RunCommand(rExecutable.String(), args, i.base, i.log) + i.log.Debug("renv::snapshot()", "out", string(stdout), "err", string(stderr)) + return err +} diff --git a/internal/interpreters/r_mock.go b/internal/interpreters/r_mock.go new file mode 100644 index 000000000..c6f835451 --- /dev/null +++ b/internal/interpreters/r_mock.go @@ -0,0 +1,82 @@ +package interpreters + +// Copyright (C) 2024 by Posit Software, PBC. + +import ( + "errors" + + "github.com/posit-dev/publisher/internal/util" + "github.com/stretchr/testify/mock" +) + +type MockRInterpreter struct { + mock.Mock +} + +func NewMockRInterpreter() *MockRInterpreter { + return &MockRInterpreter{} +} + +// We need just the simplest of a mock to start, as most of the +// functionality can be accomplished via setting items within +// the working structure. + +func (m *MockRInterpreter) Init() error { + return nil +} + +func (m *MockRInterpreter) GetRExecutable() (util.AbsolutePath, error) { + args := m.Called() + arg0 := args.Get(0) + if arg0 == nil { + return util.AbsolutePath{}, args.Error(1) + } else { + var i interface{} = arg0 + if path, ok := i.(string); ok { + return util.NewAbsolutePath(path, nil), args.Error(1) + } else { + return util.AbsolutePath{}, errors.New("invalid string argument") + } + } +} + +func (m *MockRInterpreter) GetRVersion() (string, error) { + args := m.Called() + arg0 := args.Get(0) + if arg0 == nil { + return "", args.Error(1) + } else { + var i interface{} = arg0 + if version, ok := i.(string); ok { + return version, args.Error(1) + } else { + return "", errors.New("invalid string argument") + } + } +} + +func (m *MockRInterpreter) GetLockFilePath() (util.RelativePath, bool, error) { + args := m.Called() + arg0 := args.Get(0) + arg1 := args.Get(1) + if arg0 == nil { + return util.RelativePath{}, false, args.Error(1) + } else { + var iPath interface{} = arg0 + path, ok := iPath.(string) + if !ok { + path = "" + } + var iExists interface{} = arg1 + exists, ok := iExists.(bool) + if !ok { + exists = false + } + return util.NewRelativePath(path, nil), exists, args.Error(1) + } +} + +func (m *MockRInterpreter) CreateLockfile(lockfilePath util.AbsolutePath) error { + args := m.Called(lockfilePath) + return args.Error(0) +} diff --git a/internal/interpreters/r_test.go b/internal/interpreters/r_test.go new file mode 100644 index 000000000..e523f6bd1 --- /dev/null +++ b/internal/interpreters/r_test.go @@ -0,0 +1,453 @@ +package interpreters + +// Copyright (C) 2023 by Posit Software, PBC. + +import ( + "fmt" + "runtime" + "strings" + "testing" + + "github.com/posit-dev/publisher/internal/executor/executortest" + "github.com/posit-dev/publisher/internal/logging" + "github.com/posit-dev/publisher/internal/util" + "github.com/posit-dev/publisher/internal/util/utiltest" + "github.com/spf13/afero" + + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/suite" +) + +type RSuite struct { + utiltest.Suite + cwd util.AbsolutePath +} + +func TestRSuite(t *testing.T) { + suite.Run(t, new(RSuite)) +} + +func (s *RSuite) SetupTest() { + cwd, err := util.Getwd(afero.NewMemMapFs()) + s.NoError(err) + s.cwd = cwd + err = cwd.MkdirAll(0700) + s.NoError(err) +} + +func (s *RSuite) TestNewRInterpreter() { + log := logging.New() + rPath := util.NewPath("/usr/bin/R", nil) + i := NewRInterpreter(s.cwd, rPath, log) + interpreter := i.(*defaultRInterpreter) + s.Equal(rPath, interpreter.preferredPath) + s.Equal(log, interpreter.log) + s.Equal("", interpreter.rExecutable.String()) + s.Equal("", interpreter.version) + s.Equal("", interpreter.lockfileRelPath.String()) + s.Equal(false, interpreter.lockfileExists) + s.Equal(false, interpreter.initialized) + + // New should return some failures for interface calls + path, err := interpreter.GetRExecutable() + s.Equal("", path.String()) + s.ErrorIs(err, NotYetInitialized) + + version, err := interpreter.GetRVersion() + s.Equal("", version) + s.ErrorIs(err, NotYetInitialized) + + lockFilePath, exists, err := interpreter.GetLockFilePath() + s.Equal("", lockFilePath.String()) + s.Equal(false, exists) + s.ErrorIs(err, NotYetInitialized) + + err = interpreter.CreateLockfile(util.NewAbsolutePath("abc/renv.lock", nil)) + s.ErrorIs(err, NotYetInitialized) +} + +type OutputTestData struct { + versionOutput string + expectedVersion string + pathsLockfileOutput string + expectedLockfilePath string +} + +func getOutputTestData() []OutputTestData { + data := []OutputTestData{ + // typical output from command `r --version` + {`R version 4.3.0 (2023-04-21) -- "Already Tomorrow" +Copyright (C) 2023 The R Foundation for Statistical Computing +Platform: x86_64-apple-darwin20 (64-bit) + +R is free software and comes with ABSOLUTELY NO WARRANTY. +You are welcome to redistribute it under the terms of the +GNU General Public License versions 2 or 3. +For more information about these matters see +https://www.gnu.org/licenses/. +`, + "4.3.0", + // success output from renv.lock + `[1] "/Users/billsager/dev/publishing-client-another/test/sample-content/shinyapp/renv.lock"`, + "../../test/sample-content/shinyapp/renv.lock", + }, + // output when there is a warning + {`WARNING: ignoring environment value of R_HOME +R version 4.3.3 (2024-02-29) -- "Angel Food Cake" +Copyright (C) 2024 The R Foundation for Statistical Computing +Platform: x86_64-apple-darwin20 (64-bit) + +R is free software and comes with ABSOLUTELY NO WARRANTY. +You are welcome to redistribute it under the terms of the +GNU General Public License versions 2 or 3. +For more information about these matters see +https://www.gnu.org/licenses/.`, + "4.3.3", + // success output from renv.lock + `[1] "/Users/billsager/dev/publishing-client-another/test/sample-content/shinyapp/renv.lock"`, + "../../test/sample-content/shinyapp/renv.lock", + }, + + // output when there are multiple warnings + // as well as closely matching version strings + {`WARNING: ignoring environment value of R_HOME +WARNING: your mom is calling +WARNING: time to stand +Somewhere below is the correct R version 4.3.* that we're looking for +R version 4.3.3 (2024-02-29) -- "Angel Food Cake" +Copyright (C) 2024 The R Foundation for Statistical Computing +Platform: x86_64-apple-darwin20 (64-bit) + +R is free software and comes with ABSOLUTELY NO WARRANTY. +You are welcome to redistribute it under the terms of the +GNU General Public License versions 2 or 3. +For more information about these matters see +https://www.gnu.org/licenses/.`, + "4.3.3", + // success output from renv.lock + `[1] "/Users/billsager/dev/publishing-client-another/test/sample-content/shinyapp/renv.lock"`, + "../../test/sample-content/shinyapp/renv.lock", + }, + + // test output where version exists in multiple locations + // we want to get it from the first location + {` +R version 4.3.3 (2024-02-29) -- "Angel Food Cake" +Copyright (C) 2024 The R Foundation for Statistical Computing +Platform: x86_64-apple-darwin20 (64-bit) +R version 4.1.1 (2023-12-29) -- "Fantasy Island" + +R is free software and comes with ABSOLUTELY NO WARRANTY. +You are welcome to redistribute it under the terms of the +GNU General Public License versions 2 or 3. +For more information about these matters see +https://www.gnu.org/licenses/.`, + "4.3.3", + // success output from renv.lock + `[1] "/Users/billsager/dev/publishing-client-another/test/sample-content/shinyapp/renv.lock"`, + "../../test/sample-content/shinyapp/renv.lock", + }, + // typical output from command `r --version` + {`R version 4.3.0 (2023-04-21) -- "Already Tomorrow" + Copyright (C) 2023 The R Foundation for Statistical Computing + Platform: x86_64-apple-darwin20 (64-bit) + + R is free software and comes with ABSOLUTELY NO WARRANTY. + You are welcome to redistribute it under the terms of the + GNU General Public License versions 2 or 3. + For more information about these matters see + https://www.gnu.org/licenses/. + `, + "4.3.0", + // error output from R regarding renv + `Error in loadNamespace(x) : there is no package called ‘renv’"`, + "renv.lock", + }, + } + return data +} + +func (s *RSuite) TestGetRVersionFromExecutable() { + for _, tc := range getOutputTestData() { + s.SetupTest() + log := logging.New() + rPath := s.cwd.Join("bin", "R") + rPath.Dir().MkdirAll(0777) + rPath.WriteFile(nil, 0777) + rInterpreter := NewRInterpreter(s.cwd, rPath.Path, log) + executor := executortest.NewMockExecutor() + executor.On("RunCommand", mock.Anything, []string{"--version"}, mock.Anything, mock.Anything).Return([]byte(tc.versionOutput), nil, nil) + executor.On("RunCommand", mock.Anything, []string{"-s", "-e", "renv::paths$lockfile()"}, mock.Anything, mock.Anything).Return([]byte(tc.pathsLockfileOutput), nil, nil) + + interpreter := rInterpreter.(*defaultRInterpreter) + interpreter.executor = executor + err := rInterpreter.Init() + s.NoError(err) + + rExecutable, err := rInterpreter.GetRExecutable() + s.NoError(err) + s.Equal(true, strings.Contains(rExecutable.String(), rPath.String())) + + version, err := rInterpreter.GetRVersion() + s.NoError(err) + s.Equal(tc.expectedVersion, version) + + lockFile, _, err := rInterpreter.GetLockFilePath() + s.NoError(err) + s.Equal(tc.expectedLockfilePath, lockFile.String()) + } +} + +func (s *RSuite) TestGetRVersionFromExecutableWindows() { + // R on Windows emits version information on stderr + for _, tc := range getOutputTestData() { + s.SetupTest() + log := logging.New() + rPath := s.cwd.Join("bin", "R") + rPath.Dir().MkdirAll(0777) + rPath.WriteFile(nil, 0777) + rInterpreter := NewRInterpreter(s.cwd, rPath.Path, log) + executor := executortest.NewMockExecutor() + executor.On("RunCommand", mock.Anything, []string{"--version"}, mock.Anything, mock.Anything).Return(nil, []byte(tc.versionOutput), nil) + executor.On("RunCommand", mock.Anything, []string{"-s", "-e", "renv::paths$lockfile()"}, mock.Anything, mock.Anything).Return(nil, []byte(tc.pathsLockfileOutput), nil) + + interpreter := rInterpreter.(*defaultRInterpreter) + interpreter.executor = executor + interpreter.fs = s.cwd.Fs() + err := rInterpreter.Init() + s.NoError(err) + + rExecutable, err := rInterpreter.GetRExecutable() + s.NoError(err) + s.Equal(true, strings.Contains(rExecutable.String(), rPath.String())) + + version, err := rInterpreter.GetRVersion() + s.NoError(err) + s.Equal(tc.expectedVersion, version) + + lockFile, _, err := rInterpreter.GetLockFilePath() + s.NoError(err) + s.Equal(tc.expectedLockfilePath, lockFile.String()) + } +} + +type RExecutableValidTestData struct { + initialized bool + rExecutable util.AbsolutePath + version string + expectedIsRExecutableValidResult bool +} + +func getRExecutableValidTestData() []RExecutableValidTestData { + data := []RExecutableValidTestData{ + {initialized: false, rExecutable: util.AbsolutePath{}, version: "", expectedIsRExecutableValidResult: false}, + {initialized: false, rExecutable: util.NewAbsolutePath("abc", nil), version: "", expectedIsRExecutableValidResult: false}, + {initialized: false, rExecutable: util.AbsolutePath{}, version: "1.2.3", expectedIsRExecutableValidResult: false}, + {initialized: true, rExecutable: util.NewAbsolutePath("abc", nil), version: "", expectedIsRExecutableValidResult: false}, + {initialized: true, rExecutable: util.AbsolutePath{}, version: "1.2.3", expectedIsRExecutableValidResult: false}, + {initialized: true, rExecutable: util.NewAbsolutePath("abc", nil), version: "1.2.3", expectedIsRExecutableValidResult: true}, + } + return data +} + +// Test some internal methods to confirm expected logic + +// Make sure the combos don't allow a valid RExecutable to be wrongly reported +func (s *RSuite) TestIsRExecutableValid() { + log := logging.New() + i := NewRInterpreter(s.cwd, util.Path{}, log) + interpreter := i.(*defaultRInterpreter) + s.Equal(false, interpreter.IsRExecutableValid()) + + interpreter.initialized = true + interpreter.rExecutable = util.AbsolutePath{} + for _, tc := range getRExecutableValidTestData() { + interpreter.initialized = tc.initialized + interpreter.rExecutable = tc.rExecutable + interpreter.version = tc.version + s.Equal(tc.expectedIsRExecutableValidResult, interpreter.IsRExecutableValid()) + } +} + +// Validate when we pass in a path that exists and is valid, we get it back +func (s *RSuite) TestResolveRExecutableWhenPassedInPathExistsAndIsValid() { + log := logging.New() + + rPath := s.cwd.Join("bin", "R") + rPath.Dir().MkdirAll(0777) + rPath.WriteFile([]byte(nil), 0777) + + i := NewRInterpreter(s.cwd, rPath.Path, log) + interpreter := i.(*defaultRInterpreter) + executor := executortest.NewMockExecutor() + executor.On("RunCommand", mock.Anything, []string{"--version"}, mock.Anything, mock.Anything).Return([]byte("R version 4.3.0 (2023-04-21)"), nil, nil) + interpreter.executor = executor + interpreter.initialized = true + interpreter.fs = s.cwd.Fs() + + err := interpreter.resolveRExecutable() + s.NoError(err) + s.Equal(rPath.String(), interpreter.rExecutable.String()) + s.Equal("4.3.0", interpreter.version) +} + +// Validate when we pass in a path that does not exist +// we fall through to pulling from PATH which exists and is valid +func (s *RSuite) TestResolveRExecutableWhenPassedInPathDoesNotExistButPathValid() { + log := logging.New() + + // on path + rPath := s.cwd.Join("some", "R") + rPath.Dir().MkdirAll(0777) + rPath.WriteFile(nil, 0777) + pathLooker := util.NewMockPathLooker() + pathLooker.On("LookPath", "R").Return(rPath.String(), nil) + + i := NewRInterpreter(s.cwd, util.NewPath("/bin/R2", s.cwd.Fs()), log) + interpreter := i.(*defaultRInterpreter) + executor := executortest.NewMockExecutor() + executor.On("RunCommand", mock.Anything, []string{"--version"}, mock.Anything, mock.Anything).Return([]byte("R version 4.3.0 (2023-04-21)"), nil, nil) + + interpreter.executor = executor + interpreter.initialized = true + interpreter.pathLooker = pathLooker + interpreter.fs = s.cwd.Fs() + + err := interpreter.resolveRExecutable() + s.NoError(err) + s.Equal(true, interpreter.IsRExecutableValid()) + s.Equal(rPath.String(), interpreter.rExecutable.String()) + s.Equal("4.3.0", interpreter.version) +} + +// Validate when we pass in a path that exists but is not valid, +// we fail to find R +func (s *RSuite) TestResolveRExecutableWhenPassedInPathExistsButNotValid() { + log := logging.New() + + // on path + pathLooker := util.NewMockPathLooker() + pathLooker.On("LookPath", "R").Return("/some/R", nil) + rPath := s.cwd.Join("some", "R") + rPath.Dir().MkdirAll(0777) + rPath.WriteFile(nil, 0777) + + i := NewRInterpreter(s.cwd, util.NewPath(rPath.String(), s.cwd.Fs()), log) + interpreter := i.(*defaultRInterpreter) + executor := executortest.NewMockExecutor() + executor.On("RunCommand", mock.Anything, []string{"--version"}, mock.Anything, mock.Anything).Return([]byte("bad command"), nil, nil) + + interpreter.executor = executor + interpreter.initialized = true + interpreter.pathLooker = pathLooker + interpreter.fs = s.cwd.Fs() + + err := interpreter.resolveRExecutable() + s.Error(err) + s.Equal("couldn't parse R version from command output (/Users/billsager/dev/publishing-client-another/internal/interpreters/some/R --version)", err.Error()) + s.Equal(false, interpreter.IsRExecutableValid()) +} + +// Validate when we do not pass in a value and +// have R on the path that exists but is not valid +func (s *RSuite) TestResolveRExecutableWhenPathContainsRButNotValid() { + log := logging.New() + + if runtime.GOOS == "windows" { + s.T().Skip("This test does not run on Windows") + } + pathLooker := util.NewMockPathLooker() + pathLooker.On("LookPath", "R").Return("/some/R", nil) + rPath := s.cwd.Join("some", "R") + rPath.Dir().MkdirAll(0777) + rPath.WriteFile([]byte(nil), 0777) + + i := NewRInterpreter(s.cwd, util.Path{}, log) + interpreter := i.(*defaultRInterpreter) + interpreter.pathLooker = pathLooker + interpreter.initialized = true + executor := executortest.NewMockExecutor() + executor.On("RunCommand", mock.Anything, []string{"--version"}, mock.Anything, mock.Anything).Return([]byte("Invalid stuff"), nil, nil) + interpreter.executor = executor + interpreter.fs = s.cwd.Fs() + + err := interpreter.resolveRExecutable() + s.Error(err) + s.Equal("Unable to detect any R interpreters", err.Error()) +} + +// Validate that we find the lock file that R specifies +// with default name if it exists +func (s *RSuite) TestResolveRenvLockFileWithRSpecifyingDefaultNameAndExists() { + log := logging.New() + + if runtime.GOOS == "windows" { + s.T().Skip("This test does not run on Windows") + } + rPath := s.cwd.Join("renv.lock") + rPath.Dir().MkdirAll(0777) + rPath.WriteFile(nil, 0777) + + i := NewRInterpreter(s.cwd, util.Path{}, log) + interpreter := i.(*defaultRInterpreter) + interpreter.initialized = true + interpreter.fs = s.cwd.Fs() + executor := executortest.NewMockExecutor() + outputLine := fmt.Sprintf(`[1] "%s"`, rPath.String()) + executor.On("RunCommand", mock.Anything, []string{"-s", "-e", "renv::paths$lockfile()"}, mock.Anything, mock.Anything).Return([]byte(outputLine), nil, nil) + interpreter.executor = executor + + err := interpreter.resolveRenvLockFile("does_not_matter") + s.NoError(err) + s.Equal("renv.lock", interpreter.lockfileRelPath.String()) + s.Equal(true, interpreter.lockfileExists) +} + +// Validate that we don't find the lock file that R specifies +// with default name if it doesn't exist +func (s *RSuite) TestResolveRenvLockFileWithRSpecifyingDefaultNameAndDoesNotExist() { + log := logging.New() + + if runtime.GOOS == "windows" { + s.T().Skip("This test does not run on Windows") + } + + i := NewRInterpreter(s.cwd, util.Path{}, log) + interpreter := i.(*defaultRInterpreter) + interpreter.initialized = true + executor := executortest.NewMockExecutor() + executor.On("RunCommand", mock.Anything, []string{"-s", "-e", "renv::paths$lockfile()"}, mock.Anything, mock.Anything).Return([]byte(`[1] "internal/interpreters/renv.lock"`), nil, nil) + interpreter.executor = executor + interpreter.fs = s.cwd.Fs() + + err := interpreter.resolveRenvLockFile("does_not_matter") + s.NoError(err) + s.Equal("internal/interpreters/renv.lock", interpreter.lockfileRelPath.String()) + s.Equal(false, interpreter.lockfileExists) +} + +// Validate that we find the lock file that R specifies +// with special name if it exists +func (s *RSuite) TestResolveRenvLockFileWithRSpecialNameAndExists() { + log := logging.New() + + if runtime.GOOS == "windows" { + s.T().Skip("This test does not run on Windows") + } + rPath := s.cwd.Join("renv-project222.lock") + rPath.Dir().MkdirAll(0777) + rPath.WriteFile([]byte(nil), 0777) + + i := NewRInterpreter(s.cwd, util.Path{}, log) + interpreter := i.(*defaultRInterpreter) + executor := executortest.NewMockExecutor() + outputLine := fmt.Sprintf(`[1] "%s"`, rPath.String()) + executor.On("RunCommand", mock.Anything, []string{"-s", "-e", "renv::paths$lockfile()"}, mock.Anything, mock.Anything).Return([]byte(outputLine), nil, nil) + interpreter.executor = executor + interpreter.fs = s.cwd.Fs() + + err := interpreter.resolveRenvLockFile("does_not_matter") + s.NoError(err) + s.Equal("renv-project222.lock", interpreter.lockfileRelPath.String()) + s.Equal(true, interpreter.lockfileExists) +} diff --git a/internal/publish/bundle.go b/internal/publish/bundle.go index 0274666ca..80847da36 100644 --- a/internal/publish/bundle.go +++ b/internal/publish/bundle.go @@ -11,6 +11,7 @@ import ( "github.com/posit-dev/publisher/internal/events" "github.com/posit-dev/publisher/internal/inspect" "github.com/posit-dev/publisher/internal/inspect/dependencies/renv" + "github.com/posit-dev/publisher/internal/interpreters" "github.com/posit-dev/publisher/internal/logging" "github.com/posit-dev/publisher/internal/types" "github.com/posit-dev/publisher/internal/util" @@ -94,7 +95,7 @@ func (p *defaultPublisher) createAndUploadBundle( if p.Config.R != nil { filename := p.Config.R.PackageFile if filename == "" { - filename = inspect.DefaultRenvLockfile + filename = interpreters.DefaultRenvLockfile } p.log.Debug("R configuration present", "filename", filename) lockfile, err := renv.ReadLockfile(p.Dir.Join(filename)) diff --git a/internal/publish/publish.go b/internal/publish/publish.go index cd7a76cab..3efc7a20f 100644 --- a/internal/publish/publish.go +++ b/internal/publish/publish.go @@ -75,12 +75,15 @@ func NewFromState(s *state.State, rExecutable util.Path, emitter events.Emitter, } emitter = events.NewDataEmitter(dataMap, emitter) } + + packageManager, err := renv.NewPackageMapper(s.Dir, rExecutable, log) + return &defaultPublisher{ State: s, log: log, emitter: emitter, - rPackageMapper: renv.NewPackageMapper(s.Dir, rExecutable), - }, nil + rPackageMapper: packageManager, + }, err } func logAppInfo(w io.Writer, accountURL string, contentID types.ContentID, log logging.Logger, publishingErr error) { diff --git a/internal/publish/publish_test.go b/internal/publish/publish_test.go index 038cafbe6..83ffdb748 100644 --- a/internal/publish/publish_test.go +++ b/internal/publish/publish_test.go @@ -115,6 +115,7 @@ func (s *PublishSuite) SetupTest() { func (s *PublishSuite) TestNewFromState() { stateStore := state.Empty() + stateStore.Dir = s.cwd publisher, err := NewFromState(stateStore, util.Path{}, events.NewNullEmitter(), logging.New()) s.NoError(err) s.Equal(stateStore, publisher.(*defaultPublisher).State) diff --git a/internal/publish/r_package_descriptions.go b/internal/publish/r_package_descriptions.go index 22ac6be6d..28bbbc285 100644 --- a/internal/publish/r_package_descriptions.go +++ b/internal/publish/r_package_descriptions.go @@ -9,7 +9,7 @@ import ( "github.com/posit-dev/publisher/internal/bundles" "github.com/posit-dev/publisher/internal/events" - "github.com/posit-dev/publisher/internal/inspect" + "github.com/posit-dev/publisher/internal/interpreters" "github.com/posit-dev/publisher/internal/logging" "github.com/posit-dev/publisher/internal/types" ) @@ -32,7 +32,7 @@ func (p *defaultPublisher) getRPackages() (bundles.PackageMap, error) { lockfileString := p.Config.R.PackageFile if lockfileString == "" { - lockfileString = inspect.DefaultRenvLockfile + lockfileString = interpreters.DefaultRenvLockfile } lockfilePath := p.Dir.Join(lockfileString) diff --git a/internal/services/api/get_config_r_packages.go b/internal/services/api/get_config_r_packages.go index 745cbc18f..1c973dfbc 100644 --- a/internal/services/api/get_config_r_packages.go +++ b/internal/services/api/get_config_r_packages.go @@ -10,8 +10,8 @@ import ( "github.com/gorilla/mux" "github.com/posit-dev/publisher/internal/config" - "github.com/posit-dev/publisher/internal/inspect" "github.com/posit-dev/publisher/internal/inspect/dependencies/renv" + "github.com/posit-dev/publisher/internal/interpreters" "github.com/posit-dev/publisher/internal/logging" "github.com/posit-dev/publisher/internal/util" ) @@ -55,7 +55,7 @@ func (h *getConfigRPackagesHandler) ServeHTTP(w http.ResponseWriter, req *http.R } packageFilename := cfg.R.PackageFile if packageFilename == "" { - packageFilename = inspect.DefaultRenvLockfile + packageFilename = interpreters.DefaultRenvLockfile } path := projectDir.Join(packageFilename) diff --git a/internal/services/api/post_packages_r_scan.go b/internal/services/api/post_packages_r_scan.go index 8f7b536e0..3474aa806 100644 --- a/internal/services/api/post_packages_r_scan.go +++ b/internal/services/api/post_packages_r_scan.go @@ -9,7 +9,7 @@ import ( "net/http" "path/filepath" - "github.com/posit-dev/publisher/internal/inspect" + "github.com/posit-dev/publisher/internal/interpreters" "github.com/posit-dev/publisher/internal/logging" "github.com/posit-dev/publisher/internal/util" ) @@ -31,8 +31,6 @@ func NewPostPackagesRScanHandler(base util.AbsolutePath, log logging.Logger) *Po } } -var rInspectorFactory = inspect.NewRInspector - func (h *PostPackagesRScanHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { projectDir, _, err := ProjectDirFromRequest(h.base, w, req, h.log) if err != nil { @@ -48,7 +46,7 @@ func (h *PostPackagesRScanHandler) ServeHTTP(w http.ResponseWriter, req *http.Re return } if b.SaveName == "" { - b.SaveName = inspect.DefaultRenvLockfile + b.SaveName = interpreters.DefaultRenvLockfile } // Can't call ValidateFilename on b.SaveName because // it may contain slashes. @@ -60,8 +58,14 @@ func (h *PostPackagesRScanHandler) ServeHTTP(w http.ResponseWriter, req *http.Re } lockfileAbsPath := projectDir.Join(path.String()) rPath := util.NewPath(b.R, nil) - inspector := rInspectorFactory(projectDir, rPath, h.log) - err = inspector.CreateLockfile(lockfileAbsPath) + + rInterpreter := interpreters.NewRInterpreter(projectDir, rPath, h.log) + err = rInterpreter.Init() + if err != nil { + InternalError(w, req, h.log, err) + return + } + err = rInterpreter.CreateLockfile(lockfileAbsPath) if err != nil { InternalError(w, req, h.log, err) return diff --git a/internal/services/api/post_packages_r_scan_test.go b/internal/services/api/post_packages_r_scan_test.go index dccebe20e..84399ad5f 100644 --- a/internal/services/api/post_packages_r_scan_test.go +++ b/internal/services/api/post_packages_r_scan_test.go @@ -10,11 +10,12 @@ import ( "strings" "testing" - "github.com/posit-dev/publisher/internal/inspect" + "github.com/posit-dev/publisher/internal/interpreters" "github.com/posit-dev/publisher/internal/logging" "github.com/posit-dev/publisher/internal/util" "github.com/posit-dev/publisher/internal/util/utiltest" "github.com/spf13/afero" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" ) @@ -27,7 +28,6 @@ func TestPostPackagesRScanSuite(t *testing.T) { } func (s *PostPackagesRScanSuite) SetupTest() { - rInspectorFactory = inspect.NewRInspector } func (s *PostPackagesRScanSuite) TestNewPostPackagesRScanHandler() { @@ -44,20 +44,22 @@ func (s *PostPackagesRScanSuite) TestServeHTTP() { req, err := http.NewRequest("POST", "/api/packages/r/scan", body) s.NoError(err) - base := util.NewAbsolutePath("/project", afero.NewMemMapFs()) + fs := afero.NewMemMapFs() + base := util.NewAbsolutePath("/project", fs) err = base.MkdirAll(0777) s.NoError(err) - destPath := base.Join("renv.lock") + + lockFilePath := base.Join("renv.lock") log := logging.New() h := NewPostPackagesRScanHandler(base, log) - rInspectorFactory = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) inspect.RInspector { + interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { s.Equal(base, baseDir) s.Equal(util.NewPath("/opt/R/bin/R", nil), rExec) - - i := inspect.NewMockRInspector() - i.On("CreateLockfile", destPath).Return(nil) + i := interpreters.NewMockRInterpreter() + i.On("Init").Return(nil) + i.On("CreateLockfile", lockFilePath).Return(nil) return i } @@ -74,14 +76,13 @@ func (s *PostPackagesRScanSuite) TestServeHTTPEmptyBody() { base := util.NewAbsolutePath("/project", afero.NewMemMapFs()) err = base.MkdirAll(0777) s.NoError(err) - destPath := base.Join("renv.lock") log := logging.New() h := NewPostPackagesRScanHandler(base, log) - rInspectorFactory = func(util.AbsolutePath, util.Path, logging.Logger) inspect.RInspector { - i := inspect.NewMockRInspector() - i.On("CreateLockfile", destPath).Return(nil) + interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { + i := interpreters.NewMockRInterpreter() + i.On("CreateLockfile", mock.Anything).Return(nil) return i } @@ -95,20 +96,22 @@ func (s *PostPackagesRScanSuite) TestServeHTTPWithSaveName() { req, err := http.NewRequest("POST", "/api/packages/r/scan", body) s.NoError(err) - base := util.NewAbsolutePath("/project", afero.NewMemMapFs()) + fs := afero.NewMemMapFs() + base := util.NewAbsolutePath("/project", fs) err = base.MkdirAll(0777) s.NoError(err) destPath := base.Join("my_renv.lock") log := logging.New() - h := NewPostPackagesRScanHandler(base, log) - rInspectorFactory = func(util.AbsolutePath, util.Path, logging.Logger) inspect.RInspector { - i := inspect.NewMockRInspector() - i.On("CreateLockfile", destPath).Return(nil) + interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { + i := interpreters.NewMockRInterpreter() + i.On("CreateLockfile", util.NewAbsolutePath(destPath.String(), fs)).Return(nil) return i } + h := NewPostPackagesRScanHandler(base, log) + h.ServeHTTP(rec, req) s.Equal(http.StatusNoContent, rec.Result().StatusCode) } @@ -119,7 +122,8 @@ func (s *PostPackagesRScanSuite) TestServeHTTPWithSaveNameInSubdir() { req, err := http.NewRequest("POST", "/api/packages/r/scan", body) s.NoError(err) - base := util.NewAbsolutePath("/project", afero.NewMemMapFs()) + fs := afero.NewMemMapFs() + base := util.NewAbsolutePath("/project", fs) err = base.MkdirAll(0777) s.NoError(err) destPath := base.Join(".renv", "profiles", "staging", "renv.lock") @@ -127,8 +131,8 @@ func (s *PostPackagesRScanSuite) TestServeHTTPWithSaveNameInSubdir() { log := logging.New() h := NewPostPackagesRScanHandler(base, log) - rInspectorFactory = func(util.AbsolutePath, util.Path, logging.Logger) inspect.RInspector { - i := inspect.NewMockRInspector() + interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { + i := interpreters.NewMockRInterpreter() i.On("CreateLockfile", destPath).Return(nil) return i } @@ -151,8 +155,9 @@ func (s *PostPackagesRScanSuite) TestServeHTTPErr() { h := NewPostPackagesRScanHandler(base, log) testError := errors.New("test error from ScanRequirements") - rInspectorFactory = func(util.AbsolutePath, util.Path, logging.Logger) inspect.RInspector { - i := inspect.NewMockRInspector() + interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { + i := interpreters.NewMockRInterpreter() + i.On("Init").Return(nil) i.On("CreateLockfile", destPath).Return(testError) return i } @@ -166,24 +171,25 @@ func (s *PostPackagesRScanSuite) TestServeHTTPSubdir() { body := strings.NewReader(`{"saveName":""}`) // Scanning a subdirectory two levels down - base := util.NewAbsolutePath("/project", afero.NewMemMapFs()) + fs := afero.NewMemMapFs() + base := util.NewAbsolutePath("/project", fs) projectDir := base.Join("subproject", "subdir") err := projectDir.MkdirAll(0777) s.NoError(err) relProjectDir, err := projectDir.Rel(base) s.NoError(err) + destPath := projectDir.Join("renv.lock") dirParam := url.QueryEscape(relProjectDir.String()) req, err := http.NewRequest("POST", "/api/packages/r/scan?dir="+dirParam, body) s.NoError(err) - destPath := projectDir.Join("renv.lock") - h := NewPostPackagesRScanHandler(base, logging.New()) - rInspectorFactory = func(base util.AbsolutePath, r util.Path, log logging.Logger) inspect.RInspector { - s.Equal(projectDir, base) - i := inspect.NewMockRInspector() + interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { + s.Equal(projectDir, baseDir) + i := interpreters.NewMockRInterpreter() + i.On("Init").Return(nil) i.On("CreateLockfile", destPath).Return(nil) return i } From 3b57cb72515447d41d901606847a89e8007a67f4 Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Tue, 17 Dec 2024 14:41:29 -0800 Subject: [PATCH 02/28] fix linting errors --- internal/initialize/initialize.go | 20 -------------------- internal/interpreters/r.go | 6 +++--- 2 files changed, 3 insertions(+), 23 deletions(-) diff --git a/internal/initialize/initialize.go b/internal/initialize/initialize.go index 08aee8342..b5ab22604 100644 --- a/internal/initialize/initialize.go +++ b/internal/initialize/initialize.go @@ -10,7 +10,6 @@ import ( "github.com/posit-dev/publisher/internal/config" "github.com/posit-dev/publisher/internal/inspect" "github.com/posit-dev/publisher/internal/inspect/detectors" - "github.com/posit-dev/publisher/internal/interpreters" "github.com/posit-dev/publisher/internal/logging" "github.com/posit-dev/publisher/internal/util" ) @@ -101,25 +100,6 @@ func requiresPython(cfg *config.Config, base util.AbsolutePath) (bool, error) { return exists, nil } -func requiresR(cfg *config.Config, rInterpreter interpreters.RInterpreter) (bool, error) { - if cfg.R != nil { - // InferType returned an R configuration for us to fill in. - return true, nil - } - if cfg.Type != config.ContentTypeHTML && !cfg.Type.IsPythonContent() { - // Presence of renv.lock implies R is needed, - // unless we're deploying pre-rendered Rmd or Quarto - // (where there will usually be a source file and - // associated lockfile in the directory) - _, exists, err := rInterpreter.GetLockFilePath() - if err != nil { - return false, err - } - return exists, nil - } - return false, nil -} - func normalizeConfig( cfg *config.Config, base util.AbsolutePath, diff --git a/internal/interpreters/r.go b/internal/interpreters/r.go index 0e4666e3d..23b2b36ea 100644 --- a/internal/interpreters/r.go +++ b/internal/interpreters/r.go @@ -20,9 +20,9 @@ import ( const DefaultRenvLockfile = "renv.lock" -var NotYetInitialized = types.NewAgentError(types.ErrorRExecNotFound, errors.New("Not yet initialized"), nil) -var MissingRError = types.NewAgentError(types.ErrorRExecNotFound, errors.New("Unable to detect any R interpreters"), nil) -var InvalidR = types.NewAgentError(types.ErrorRExecNotFound, errors.New("R executable is invalid"), nil) +var NotYetInitialized = types.NewAgentError(types.ErrorRExecNotFound, errors.New("not yet initialized"), nil) +var MissingRError = types.NewAgentError(types.ErrorRExecNotFound, errors.New("unable to detect any R interpreters"), nil) +var InvalidR = types.NewAgentError(types.ErrorRExecNotFound, errors.New("r executable is invalid"), nil) type RInterpreter interface { Init() error From 9be66c0fe44d250869eb4b61aed6083521ca7e1d Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Tue, 17 Dec 2024 15:35:45 -0800 Subject: [PATCH 03/28] fix unit tests in CI --- internal/initialize/initialize_test.go | 10 ++++++++++ internal/inspect/r_test.go | 10 ++++++++++ internal/interpreters/r_mock.go | 8 ++++---- internal/interpreters/r_test.go | 19 +++++++++++-------- 4 files changed, 35 insertions(+), 12 deletions(-) diff --git a/internal/initialize/initialize_test.go b/internal/initialize/initialize_test.go index b3622a7d3..c474c7a27 100644 --- a/internal/initialize/initialize_test.go +++ b/internal/initialize/initialize_test.go @@ -8,10 +8,12 @@ import ( "github.com/posit-dev/publisher/internal/config" "github.com/posit-dev/publisher/internal/inspect" "github.com/posit-dev/publisher/internal/inspect/detectors" + "github.com/posit-dev/publisher/internal/interpreters" "github.com/posit-dev/publisher/internal/logging" "github.com/posit-dev/publisher/internal/util" "github.com/posit-dev/publisher/internal/util/utiltest" "github.com/spf13/afero" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" ) @@ -34,6 +36,14 @@ func (s *InitializeSuite) SetupTest() { s.cwd = cwd err = cwd.MkdirAll(0700) s.NoError(err) + + interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { + i := interpreters.NewMockRInterpreter() + i.On("Init").Return(nil) + i.On("RequiresR", mock.Anything).Return(false, nil) + i.On("GetLockFilePath").Return(util.RelativePath{}, false, nil) + return i + } } func (s *InitializeSuite) TestInitEmpty() { diff --git a/internal/inspect/r_test.go b/internal/inspect/r_test.go index 56b1efe26..ebaa2aeed 100644 --- a/internal/inspect/r_test.go +++ b/internal/inspect/r_test.go @@ -5,10 +5,12 @@ package inspect import ( "testing" + "github.com/posit-dev/publisher/internal/interpreters" "github.com/posit-dev/publisher/internal/logging" "github.com/posit-dev/publisher/internal/util" "github.com/posit-dev/publisher/internal/util/utiltest" "github.com/spf13/afero" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" ) @@ -27,6 +29,14 @@ func (s *RSuite) SetupTest() { s.cwd = cwd err = cwd.MkdirAll(0700) s.NoError(err) + + interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { + i := interpreters.NewMockRInterpreter() + i.On("Init").Return(nil) + i.On("RequiresR", mock.Anything).Return(false, nil) + i.On("GetLockFilePath").Return(util.RelativePath{}, false, nil) + return i + } } func (s *RSuite) TestNewRInspector() { diff --git a/internal/interpreters/r_mock.go b/internal/interpreters/r_mock.go index c6f835451..8ea8a11d5 100644 --- a/internal/interpreters/r_mock.go +++ b/internal/interpreters/r_mock.go @@ -60,19 +60,19 @@ func (m *MockRInterpreter) GetLockFilePath() (util.RelativePath, bool, error) { arg0 := args.Get(0) arg1 := args.Get(1) if arg0 == nil { - return util.RelativePath{}, false, args.Error(1) + return util.RelativePath{}, false, args.Error(2) } else { var iPath interface{} = arg0 - path, ok := iPath.(string) + path, ok := iPath.(util.RelativePath) if !ok { - path = "" + path = util.RelativePath{} } var iExists interface{} = arg1 exists, ok := iExists.(bool) if !ok { exists = false } - return util.NewRelativePath(path, nil), exists, args.Error(1) + return util.NewRelativePath(path.String(), nil), exists, args.Error(2) } } diff --git a/internal/interpreters/r_test.go b/internal/interpreters/r_test.go index e523f6bd1..57f6c8e75 100644 --- a/internal/interpreters/r_test.go +++ b/internal/interpreters/r_test.go @@ -21,6 +21,7 @@ import ( type RSuite struct { utiltest.Suite cwd util.AbsolutePath + fs afero.Fs } func TestRSuite(t *testing.T) { @@ -28,7 +29,8 @@ func TestRSuite(t *testing.T) { } func (s *RSuite) SetupTest() { - cwd, err := util.Getwd(afero.NewMemMapFs()) + s.fs = afero.NewMemMapFs() + cwd, err := util.Getwd(s.fs) s.NoError(err) s.cwd = cwd err = cwd.MkdirAll(0700) @@ -37,7 +39,7 @@ func (s *RSuite) SetupTest() { func (s *RSuite) TestNewRInterpreter() { log := logging.New() - rPath := util.NewPath("/usr/bin/R", nil) + rPath := util.NewPath("/usr/bin/R", s.fs) i := NewRInterpreter(s.cwd, rPath, log) interpreter := i.(*defaultRInterpreter) s.Equal(rPath, interpreter.preferredPath) @@ -174,6 +176,7 @@ func (s *RSuite) TestGetRVersionFromExecutable() { rPath := s.cwd.Join("bin", "R") rPath.Dir().MkdirAll(0777) rPath.WriteFile(nil, 0777) + rInterpreter := NewRInterpreter(s.cwd, rPath.Path, log) executor := executortest.NewMockExecutor() executor.On("RunCommand", mock.Anything, []string{"--version"}, mock.Anything, mock.Anything).Return([]byte(tc.versionOutput), nil, nil) @@ -238,14 +241,14 @@ type RExecutableValidTestData struct { expectedIsRExecutableValidResult bool } -func getRExecutableValidTestData() []RExecutableValidTestData { +func getRExecutableValidTestData(fs afero.Fs) []RExecutableValidTestData { data := []RExecutableValidTestData{ {initialized: false, rExecutable: util.AbsolutePath{}, version: "", expectedIsRExecutableValidResult: false}, - {initialized: false, rExecutable: util.NewAbsolutePath("abc", nil), version: "", expectedIsRExecutableValidResult: false}, + {initialized: false, rExecutable: util.NewAbsolutePath("abc", fs), version: "", expectedIsRExecutableValidResult: false}, {initialized: false, rExecutable: util.AbsolutePath{}, version: "1.2.3", expectedIsRExecutableValidResult: false}, - {initialized: true, rExecutable: util.NewAbsolutePath("abc", nil), version: "", expectedIsRExecutableValidResult: false}, + {initialized: true, rExecutable: util.NewAbsolutePath("abc", fs), version: "", expectedIsRExecutableValidResult: false}, {initialized: true, rExecutable: util.AbsolutePath{}, version: "1.2.3", expectedIsRExecutableValidResult: false}, - {initialized: true, rExecutable: util.NewAbsolutePath("abc", nil), version: "1.2.3", expectedIsRExecutableValidResult: true}, + {initialized: true, rExecutable: util.NewAbsolutePath("abc", fs), version: "1.2.3", expectedIsRExecutableValidResult: true}, } return data } @@ -261,7 +264,7 @@ func (s *RSuite) TestIsRExecutableValid() { interpreter.initialized = true interpreter.rExecutable = util.AbsolutePath{} - for _, tc := range getRExecutableValidTestData() { + for _, tc := range getRExecutableValidTestData(s.fs) { interpreter.initialized = tc.initialized interpreter.rExecutable = tc.rExecutable interpreter.version = tc.version @@ -373,7 +376,7 @@ func (s *RSuite) TestResolveRExecutableWhenPathContainsRButNotValid() { err := interpreter.resolveRExecutable() s.Error(err) - s.Equal("Unable to detect any R interpreters", err.Error()) + s.Equal("unable to detect any R interpreters", err.Error()) } // Validate that we find the lock file that R specifies From b933e771ff69c614ad04afdda2b0864faa410bf7 Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Tue, 17 Dec 2024 15:41:00 -0800 Subject: [PATCH 04/28] trial fix for CI differences --- internal/interpreters/r_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/interpreters/r_test.go b/internal/interpreters/r_test.go index 57f6c8e75..1b539e27c 100644 --- a/internal/interpreters/r_test.go +++ b/internal/interpreters/r_test.go @@ -196,8 +196,10 @@ func (s *RSuite) TestGetRVersionFromExecutable() { s.Equal(tc.expectedVersion, version) lockFile, _, err := rInterpreter.GetLockFilePath() + absLockFile := util.NewAbsolutePath(lockFile.String(), s.fs) s.NoError(err) - s.Equal(tc.expectedLockfilePath, lockFile.String()) + + s.Equal(tc.expectedLockfilePath, absLockFile.String()) } } From 20ecfcf9a89f0eda5d2fce168ddffc7080eacdbb Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Tue, 17 Dec 2024 15:45:53 -0800 Subject: [PATCH 05/28] trial fix for CI differences --- internal/interpreters/r_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/interpreters/r_test.go b/internal/interpreters/r_test.go index 1b539e27c..56a605f44 100644 --- a/internal/interpreters/r_test.go +++ b/internal/interpreters/r_test.go @@ -196,10 +196,11 @@ func (s *RSuite) TestGetRVersionFromExecutable() { s.Equal(tc.expectedVersion, version) lockFile, _, err := rInterpreter.GetLockFilePath() - absLockFile := util.NewAbsolutePath(lockFile.String(), s.fs) s.NoError(err) + absLockFile := util.NewAbsolutePath(lockFile.String(), s.fs) + absExpectedLockFile := util.NewAbsolutePath(tc.expectedLockfilePath, s.fs) - s.Equal(tc.expectedLockfilePath, absLockFile.String()) + s.Equal(absExpectedLockFile.String(), absLockFile.String()) } } From 18929beae5603888aa0b06859ce412fd41cabb84 Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Tue, 17 Dec 2024 15:54:01 -0800 Subject: [PATCH 06/28] trial fix for CI differences --- internal/interpreters/r_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/interpreters/r_test.go b/internal/interpreters/r_test.go index 56a605f44..82bab8e6c 100644 --- a/internal/interpreters/r_test.go +++ b/internal/interpreters/r_test.go @@ -90,8 +90,8 @@ https://www.gnu.org/licenses/. `, "4.3.0", // success output from renv.lock - `[1] "/Users/billsager/dev/publishing-client-another/test/sample-content/shinyapp/renv.lock"`, - "../../test/sample-content/shinyapp/renv.lock", + `[1] "/test/sample-content/shinyapp/renv.lock"`, + "/test/sample-content/shinyapp/renv.lock", }, // output when there is a warning {`WARNING: ignoring environment value of R_HOME @@ -106,8 +106,8 @@ For more information about these matters see https://www.gnu.org/licenses/.`, "4.3.3", // success output from renv.lock - `[1] "/Users/billsager/dev/publishing-client-another/test/sample-content/shinyapp/renv.lock"`, - "../../test/sample-content/shinyapp/renv.lock", + `[1] "/test/sample-content/shinyapp/renv.lock"`, + "/test/sample-content/shinyapp/renv.lock", }, // output when there are multiple warnings @@ -127,8 +127,8 @@ For more information about these matters see https://www.gnu.org/licenses/.`, "4.3.3", // success output from renv.lock - `[1] "/Users/billsager/dev/publishing-client-another/test/sample-content/shinyapp/renv.lock"`, - "../../test/sample-content/shinyapp/renv.lock", + `[1] "/test/sample-content/shinyapp/renv.lock"`, + "/test/sample-content/shinyapp/renv.lock", }, // test output where version exists in multiple locations @@ -146,8 +146,8 @@ For more information about these matters see https://www.gnu.org/licenses/.`, "4.3.3", // success output from renv.lock - `[1] "/Users/billsager/dev/publishing-client-another/test/sample-content/shinyapp/renv.lock"`, - "../../test/sample-content/shinyapp/renv.lock", + `[1] "/test/sample-content/shinyapp/renv.lock"`, + "/test/sample-content/shinyapp/renv.lock", }, // typical output from command `r --version` {`R version 4.3.0 (2023-04-21) -- "Already Tomorrow" From 7d15f4826c85561de48de3d436b9157a783b53e5 Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Tue, 17 Dec 2024 15:58:24 -0800 Subject: [PATCH 07/28] trial fix for CI differences --- internal/interpreters/r_test.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/internal/interpreters/r_test.go b/internal/interpreters/r_test.go index 82bab8e6c..0abb27567 100644 --- a/internal/interpreters/r_test.go +++ b/internal/interpreters/r_test.go @@ -170,6 +170,10 @@ https://www.gnu.org/licenses/.`, } func (s *RSuite) TestGetRVersionFromExecutable() { + if runtime.GOOS == "windows" { + s.T().Skip("This test does not run on Windows") + } + for _, tc := range getOutputTestData() { s.SetupTest() log := logging.New() @@ -205,6 +209,9 @@ func (s *RSuite) TestGetRVersionFromExecutable() { } func (s *RSuite) TestGetRVersionFromExecutableWindows() { + if runtime.GOOS != "windows" { + s.T().Skip("This test only runs on Windows") + } // R on Windows emits version information on stderr for _, tc := range getOutputTestData() { s.SetupTest() @@ -233,7 +240,10 @@ func (s *RSuite) TestGetRVersionFromExecutableWindows() { lockFile, _, err := rInterpreter.GetLockFilePath() s.NoError(err) - s.Equal(tc.expectedLockfilePath, lockFile.String()) + absLockFile := util.NewAbsolutePath(lockFile.String(), s.fs) + absExpectedLockFile := util.NewAbsolutePath(tc.expectedLockfilePath, s.fs) + + s.Equal(absExpectedLockFile.String(), absLockFile.String()) } } From c66b0661f347c2b477772ae964559ed0c67b2c97 Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Tue, 17 Dec 2024 16:07:45 -0800 Subject: [PATCH 08/28] trial fix for CI differences --- internal/interpreters/r_test.go | 2 +- internal/publish/publish_test.go | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/internal/interpreters/r_test.go b/internal/interpreters/r_test.go index 0abb27567..487dd4b74 100644 --- a/internal/interpreters/r_test.go +++ b/internal/interpreters/r_test.go @@ -360,7 +360,7 @@ func (s *RSuite) TestResolveRExecutableWhenPassedInPathExistsButNotValid() { err := interpreter.resolveRExecutable() s.Error(err) - s.Equal("couldn't parse R version from command output (/Users/billsager/dev/publishing-client-another/internal/interpreters/some/R --version)", err.Error()) + s.Equal(true, strings.Contains(err.Error(), "couldn't parse R version from command output")) s.Equal(false, interpreter.IsRExecutableValid()) } diff --git a/internal/publish/publish_test.go b/internal/publish/publish_test.go index 83ffdb748..e3c1bfd69 100644 --- a/internal/publish/publish_test.go +++ b/internal/publish/publish_test.go @@ -16,6 +16,7 @@ import ( "github.com/posit-dev/publisher/internal/deployment" "github.com/posit-dev/publisher/internal/events" "github.com/posit-dev/publisher/internal/inspect/dependencies/renv" + "github.com/posit-dev/publisher/internal/interpreters" "github.com/posit-dev/publisher/internal/logging" "github.com/posit-dev/publisher/internal/logging/loggingtest" "github.com/posit-dev/publisher/internal/project" @@ -111,6 +112,14 @@ func (s *PublishSuite) SetupTest() { cwd.Join("requirements.txt").WriteFile([]byte("flask\n"), 0600) cwd.Join("renv.lock").WriteFile([]byte(renvLockContent), 0600) + interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { + i := interpreters.NewMockRInterpreter() + i.On("Init").Return(nil) + i.On("RequiresR", mock.Anything).Return(false, nil) + i.On("GetLockFilePath").Return(util.RelativePath{}, false, nil) + return i + } + } func (s *PublishSuite) TestNewFromState() { From 811e4af0e7c6723dece598aebe4c8e1e22043036 Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Wed, 18 Dec 2024 09:26:23 -0800 Subject: [PATCH 09/28] wait longer for title prompt in CI e2e test --- test/vscode-ui/test/specs/fastapi.spec.ts | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/test/vscode-ui/test/specs/fastapi.spec.ts b/test/vscode-ui/test/specs/fastapi.spec.ts index 54709e3af..cddf92a20 100644 --- a/test/vscode-ui/test/specs/fastapi.spec.ts +++ b/test/vscode-ui/test/specs/fastapi.spec.ts @@ -53,11 +53,26 @@ describe("VS Code Extension UI Test", () => { const simplepy = browser.$(`aria/simple.py`); await simplepy.click(); - const titleMessage = browser.$("#quickInput_message"); - await expect(titleMessage).toHaveText( - "Enter a title for your content or application. (Press 'Enter' to confirm or 'Escape' to cancel)", + const elem = await $("#quickInput_message"); + await browser.waitUntil( + async function () { + return ( + (await elem.getText()) === + "Enter a title for your content or application. (Press 'Enter' to confirm or 'Escape' to cancel)" + ); + }, + { + timeout: 60000, + timeoutMsg: + "expected Title prompt to be visible after 60 seconds of inspection processing", + }, ); + // const titleMessage = browser.$("#quickInput_message"); + // await expect(titleMessage).toHaveText( + // "Enter a title for your content or application. (Press 'Enter' to confirm or 'Escape' to cancel)", + // ); + await input.setValue("my fastapi app"); await browser.keys("\uE007"); // set server url From 78e4468e913c319ff0be70833132407a75dea673 Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Wed, 18 Dec 2024 10:01:10 -0800 Subject: [PATCH 10/28] lazy load the renv lock file location (and wait to execute `renv` until needed) --- internal/interpreters/r.go | 59 +++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 27 deletions(-) diff --git a/internal/interpreters/r.go b/internal/interpreters/r.go index 23b2b36ea..63f252786 100644 --- a/internal/interpreters/r.go +++ b/internal/interpreters/r.go @@ -37,15 +37,16 @@ type defaultRInterpreter struct { executor executor.Executor pathLooker util.PathLooker - base util.AbsolutePath - preferredPath util.Path - rExecutable util.AbsolutePath - version string - lockfileRelPath util.RelativePath - lockfileExists bool - log logging.Logger - initialized bool - fs afero.Fs + base util.AbsolutePath + preferredPath util.Path + rExecutable util.AbsolutePath + version string + lockfileRelPath util.RelativePath + lockfileExists bool + lockfileInitialized bool + log logging.Logger + initialized bool + fs afero.Fs } var _ RInterpreter = &defaultRInterpreter{} @@ -59,15 +60,16 @@ func RInterpreterFactory(base util.AbsolutePath, executor: executor.NewExecutor(), pathLooker: util.NewPathLooker(), - base: base, - preferredPath: rExecutableParam, - rExecutable: util.AbsolutePath{}, - version: "", - lockfileRelPath: util.RelativePath{}, - lockfileExists: false, - log: log, - initialized: false, - fs: nil, + base: base, + preferredPath: rExecutableParam, + rExecutable: util.AbsolutePath{}, + version: "", + lockfileRelPath: util.RelativePath{}, + lockfileExists: false, + lockfileInitialized: false, + log: log, + initialized: false, + fs: nil, } } @@ -76,6 +78,10 @@ func RInterpreterFactory(base util.AbsolutePath, // towards the perferredPath, but otherwise, first one on path. If the // executable is not a valid R interpreter, then will not be set. // 2. Seeds the version of the rExecutable being used, if set. +// +// We lazy load the lock file information, since it requires a call into +// renv, which can be slow to be started (package initialization or something). +// When this occurs, we'll do the following steps. // 3. Seeds the renv lock file for the rExecutable being used or if not found // then the path to the default lock file // 4. Seeds the existance of the lock file at the lockfileRelPath @@ -93,12 +99,6 @@ func (i *defaultRInterpreter) Init() error { return err } - // This will set lockfileRelPath and lockfileExists for us - err = i.resolveRenvLockFile(i.rExecutable.String()) - if err != nil { - return err - } - return nil } @@ -126,10 +126,15 @@ func (i *defaultRInterpreter) GetLockFilePath() (relativePath util.RelativePath, if !i.initialized { return util.RelativePath{}, false, NotYetInitialized } - if i.initialized { - return i.lockfileRelPath, i.lockfileExists, nil + if !i.lockfileInitialized { + // This will set lockfileRelPath and lockfileExists for us + err = i.resolveRenvLockFile(i.rExecutable.String()) + if err != nil { + return util.RelativePath{}, false, err + } + i.lockfileInitialized = true } - return util.RelativePath{}, false, NotYetInitialized + return i.lockfileRelPath, i.lockfileExists, nil } func (i *defaultRInterpreter) IsRExecutableValid() bool { From 1b1ee2f465f5958726270c64f1143c0804f8db12 Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Wed, 18 Dec 2024 12:00:26 -0800 Subject: [PATCH 11/28] handle R executable not found - without unit tests --- internal/initialize/initialize.go | 1 + internal/inspect/r.go | 19 ++++++++- internal/interpreters/r.go | 70 ++++++++++++++++++++----------- 3 files changed, 63 insertions(+), 27 deletions(-) diff --git a/internal/initialize/initialize.go b/internal/initialize/initialize.go index b5ab22604..b0f0f27ae 100644 --- a/internal/initialize/initialize.go +++ b/internal/initialize/initialize.go @@ -152,6 +152,7 @@ func normalizeConfig( rInspector, err := RInspectorFactory(base, rExecutable, log) if err != nil { + log.Debug("Error while creating the RInspector", "error", err.Error()) return err } needR, err := rInspector.RequiresR(cfg) diff --git a/internal/inspect/r.go b/internal/inspect/r.go index fba9504b2..a921992a9 100644 --- a/internal/inspect/r.go +++ b/internal/inspect/r.go @@ -7,6 +7,7 @@ import ( "github.com/posit-dev/publisher/internal/executor" "github.com/posit-dev/publisher/internal/interpreters" "github.com/posit-dev/publisher/internal/logging" + "github.com/posit-dev/publisher/internal/types" "github.com/posit-dev/publisher/internal/util" ) @@ -28,6 +29,8 @@ var _ RInspector = &defaultRInspector{} func NewRInspector(base util.AbsolutePath, rExecutable util.Path, log logging.Logger) (RInspector, error) { rInterpreter := interpreters.NewRInterpreter(base, rExecutable, log) + // No error returned if there is no R interpreter found. + // That can be expected when retrieving the RExecutable err := rInterpreter.Init() return &defaultRInspector{ @@ -44,12 +47,24 @@ func NewRInspector(base util.AbsolutePath, rExecutable util.Path, log logging.Lo func (i *defaultRInspector) InspectR() (*config.R, error) { _, err := i.rInterpreter.GetRExecutable() if err != nil { - i.log.Debug("Error retrieving R Executable", "error", err) + if _, ok := types.IsAgentErrorOf(err, types.ErrorRExecNotFound); ok { + // we have no R on the system. That's ok. + i.log.Debug("Inspecting with no accessible R Executable.") + } else { + i.log.Debug("Error retrieving R Executable", "error", err) + } } version, err := i.rInterpreter.GetRVersion() if err != nil { - i.log.Debug("Error retrieving R Version", "error", err) + if _, ok := types.IsAgentErrorOf(err, types.ErrorRExecNotFound); ok { + // we have no R on the system. That's ok. + i.log.Debug("No R Version, since we have no accessible R Executable.") + } else { + i.log.Debug("Error retrieving R Version", "error", err) + } } + // GetLockFilePath will handle if there is no RVersion available. It defaults to `renv.lock` + // and checks for the presence of it. packageFile, _, err := i.rInterpreter.GetLockFilePath() if err != nil { i.log.Debug("Error retrieving R package lock file", "error", err) diff --git a/internal/interpreters/r.go b/internal/interpreters/r.go index 63f252786..6acff64ac 100644 --- a/internal/interpreters/r.go +++ b/internal/interpreters/r.go @@ -94,8 +94,14 @@ func (i *defaultRInterpreter) Init() error { i.initialized = true // This will set the rExecutable and version for us + // Only fatal, unexpected errors will be returned. + // We will handle MissingRError internally, as this is a valid environment err := i.resolveRExecutable() if err != nil { + if _, ok := types.IsAgentErrorOf(err, types.ErrorRExecNotFound); ok { + // suppress the error, this is valid. + return nil + } return err } @@ -128,6 +134,7 @@ func (i *defaultRInterpreter) GetLockFilePath() (relativePath util.RelativePath, } if !i.lockfileInitialized { // This will set lockfileRelPath and lockfileExists for us + // and does not require an R Executable to be available (but it is better if it is) err = i.resolveRenvLockFile(i.rExecutable.String()) if err != nil { return util.RelativePath{}, false, err @@ -152,6 +159,8 @@ func (i *defaultRInterpreter) IsRExecutableValid() bool { func (i *defaultRInterpreter) resolveRExecutable() error { var rExecutable = util.AbsolutePath{} + // return MissingRError + // Passed in path to executable if i.preferredPath.String() != "" { // User-provided R executable @@ -198,26 +207,32 @@ func (i *defaultRInterpreter) resolveRExecutable() error { } } - // If we still don't have one, then we ca + // If we still don't have one, then it will need + // to be handled, but is a totally valid environment without R if rExecutable.Path.String() == "" { + i.log.Debug("R executable not found, proceeding without working R environment.") return MissingRError } - // Need to validate the executable, so let's ask it for the version - i.log.Debug("Validating path to R executable found", "path", rExecutable) - // Ensure the R is actually runnable. - version, err := i.ValidateRExecutable(rExecutable.String()) - if err == nil { - i.log.Debug("Successful validation for R executable", "rExecutable", rExecutable) - } else { - i.log.Debug("R executable from PATH is invalid.", "rExecutable", rExecutable, "error", err) - return err - } - - // all is good! - i.rExecutable = util.NewAbsolutePath(rExecutable.String(), i.fs) - i.version = version - return nil + // temp + i.log.Debug("R executable not found, proceeding without working R environment.") + return MissingRError + + // // Need to validate the executable, so let's ask it for the version + // i.log.Debug("Validating path to R executable found", "path", rExecutable) + // // Ensure the R is actually runnable. + // version, err := i.ValidateRExecutable(rExecutable.String()) + // if err == nil { + // i.log.Debug("Successful validation for R executable", "rExecutable", rExecutable) + // } else { + // i.log.Debug("R executable from PATH is invalid.", "rExecutable", rExecutable, "error", err) + // return err + // } + + // // all is good! + // i.rExecutable = util.NewAbsolutePath(rExecutable.String(), i.fs) + // i.version = version + // return nil } // We assume if we can get a version from the rExecutable passed in, that it @@ -269,16 +284,21 @@ func (i *defaultRInterpreter) getRVersionFromRExecutable(rExecutable string) (st // // Return path, existence and any error if encountered. func (i *defaultRInterpreter) resolveRenvLockFile(rExecutable string) error { - lockfilePath, err := i.getRenvLockfilePathFromRExecutable(rExecutable) - if err == nil { - i.log.Debug("renv lockfile found via R executable", "renv_lock", lockfilePath) - } else { - // we'll handle the error by looking elsewhere - - i.log.Debug("Unable to get renv lockfile path via R executable", "error", err.Error()) - // we'll default if we can't get it from R executable + var lockfilePath util.AbsolutePath + + if i.IsRExecutableValid() { + lockfilePath, err := i.getRenvLockfilePathFromRExecutable(rExecutable) + if err == nil { + i.log.Debug("renv lockfile found via R executable", "renv_lock", lockfilePath) + } else { + // we'll handle the error by looking elsewhere + i.log.Debug("Unable to get renv lockfile path via R executable", "error", err.Error()) + } + } + // if we still don't have a path, we'll default if we can't get it from R executable + if lockfilePath.Path.String() == "" { lockfilePath = i.base.Join(DefaultRenvLockfile) - i.log.Debug("using default renv lockfile", "lockfilePath", lockfilePath) + i.log.Debug("looking for default renv lockfile", "lockfilePath", lockfilePath) } lockfileRelPath, err := lockfilePath.Rel(i.base) From ef9d391416ccf62f2898687ecbab95ab77e97c35 Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Wed, 18 Dec 2024 12:00:47 -0800 Subject: [PATCH 12/28] return CI test back to original to see if passes --- test/vscode-ui/test/specs/fastapi.spec.ts | 36 +++++++++++------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/test/vscode-ui/test/specs/fastapi.spec.ts b/test/vscode-ui/test/specs/fastapi.spec.ts index cddf92a20..a3b9319db 100644 --- a/test/vscode-ui/test/specs/fastapi.spec.ts +++ b/test/vscode-ui/test/specs/fastapi.spec.ts @@ -53,26 +53,26 @@ describe("VS Code Extension UI Test", () => { const simplepy = browser.$(`aria/simple.py`); await simplepy.click(); - const elem = await $("#quickInput_message"); - await browser.waitUntil( - async function () { - return ( - (await elem.getText()) === - "Enter a title for your content or application. (Press 'Enter' to confirm or 'Escape' to cancel)" - ); - }, - { - timeout: 60000, - timeoutMsg: - "expected Title prompt to be visible after 60 seconds of inspection processing", - }, - ); - - // const titleMessage = browser.$("#quickInput_message"); - // await expect(titleMessage).toHaveText( - // "Enter a title for your content or application. (Press 'Enter' to confirm or 'Escape' to cancel)", + // const elem = await $("#quickInput_message"); + // await browser.waitUntil( + // async function () { + // return ( + // (await elem.getText()) === + // "Enter a title for your content or application. (Press 'Enter' to confirm or 'Escape' to cancel)" + // ); + // }, + // { + // timeout: 60000, + // timeoutMsg: + // "expected Title prompt to be visible after 60 seconds of inspection processing", + // }, // ); + const titleMessage = browser.$("#quickInput_message"); + await expect(titleMessage).toHaveText( + "Enter a title for your content or application. (Press 'Enter' to confirm or 'Escape' to cancel)", + ); + await input.setValue("my fastapi app"); await browser.keys("\uE007"); // set server url From 2af2ace21317092b45caccbc3e832d232bcd9f78 Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Wed, 18 Dec 2024 12:50:16 -0800 Subject: [PATCH 13/28] remove temp debug code and adjust unit tests --- internal/interpreters/r.go | 39 ++++++++++++++------------------- internal/interpreters/r_test.go | 5 ++++- 2 files changed, 21 insertions(+), 23 deletions(-) diff --git a/internal/interpreters/r.go b/internal/interpreters/r.go index 6acff64ac..677dbc369 100644 --- a/internal/interpreters/r.go +++ b/internal/interpreters/r.go @@ -159,8 +159,6 @@ func (i *defaultRInterpreter) IsRExecutableValid() bool { func (i *defaultRInterpreter) resolveRExecutable() error { var rExecutable = util.AbsolutePath{} - // return MissingRError - // Passed in path to executable if i.preferredPath.String() != "" { // User-provided R executable @@ -214,25 +212,21 @@ func (i *defaultRInterpreter) resolveRExecutable() error { return MissingRError } - // temp - i.log.Debug("R executable not found, proceeding without working R environment.") - return MissingRError - - // // Need to validate the executable, so let's ask it for the version - // i.log.Debug("Validating path to R executable found", "path", rExecutable) - // // Ensure the R is actually runnable. - // version, err := i.ValidateRExecutable(rExecutable.String()) - // if err == nil { - // i.log.Debug("Successful validation for R executable", "rExecutable", rExecutable) - // } else { - // i.log.Debug("R executable from PATH is invalid.", "rExecutable", rExecutable, "error", err) - // return err - // } - - // // all is good! - // i.rExecutable = util.NewAbsolutePath(rExecutable.String(), i.fs) - // i.version = version - // return nil + // Need to validate the executable, so let's ask it for the version + i.log.Debug("Validating path to R executable found", "path", rExecutable) + // Ensure the R is actually runnable. + version, err := i.ValidateRExecutable(rExecutable.String()) + if err == nil { + i.log.Debug("Successful validation for R executable", "rExecutable", rExecutable) + } else { + i.log.Debug("R executable from PATH is invalid.", "rExecutable", rExecutable, "error", err) + return err + } + + // all is good! + i.rExecutable = util.NewAbsolutePath(rExecutable.String(), i.fs) + i.version = version + return nil } // We assume if we can get a version from the rExecutable passed in, that it @@ -285,9 +279,10 @@ func (i *defaultRInterpreter) getRVersionFromRExecutable(rExecutable string) (st // Return path, existence and any error if encountered. func (i *defaultRInterpreter) resolveRenvLockFile(rExecutable string) error { var lockfilePath util.AbsolutePath + var err error if i.IsRExecutableValid() { - lockfilePath, err := i.getRenvLockfilePathFromRExecutable(rExecutable) + lockfilePath, err = i.getRenvLockfilePathFromRExecutable(rExecutable) if err == nil { i.log.Debug("renv lockfile found via R executable", "renv_lock", lockfilePath) } else { diff --git a/internal/interpreters/r_test.go b/internal/interpreters/r_test.go index 487dd4b74..86b4e5114 100644 --- a/internal/interpreters/r_test.go +++ b/internal/interpreters/r_test.go @@ -438,7 +438,7 @@ func (s *RSuite) TestResolveRenvLockFileWithRSpecifyingDefaultNameAndDoesNotExis err := interpreter.resolveRenvLockFile("does_not_matter") s.NoError(err) - s.Equal("internal/interpreters/renv.lock", interpreter.lockfileRelPath.String()) + s.Equal("renv.lock", interpreter.lockfileRelPath.String()) s.Equal(false, interpreter.lockfileExists) } @@ -461,6 +461,9 @@ func (s *RSuite) TestResolveRenvLockFileWithRSpecialNameAndExists() { executor.On("RunCommand", mock.Anything, []string{"-s", "-e", "renv::paths$lockfile()"}, mock.Anything, mock.Anything).Return([]byte(outputLine), nil, nil) interpreter.executor = executor interpreter.fs = s.cwd.Fs() + interpreter.initialized = true + interpreter.rExecutable = util.NewAbsolutePath("does_not_matter/R", interpreter.fs) + interpreter.version = "does_not_matter" err := interpreter.resolveRenvLockFile("does_not_matter") s.NoError(err) From d77b562fdfc7898eb86bc76538ff9e7abd9dc710 Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Wed, 18 Dec 2024 17:27:17 -0800 Subject: [PATCH 14/28] additional unit tests --- internal/initialize/initialize_test.go | 112 ++++++++++++++++--- internal/inspect/r_test.go | 141 ++++++++++++++++++++++-- internal/interpreters/r.go | 12 ++- internal/interpreters/r_mock.go | 8 +- internal/interpreters/r_test.go | 144 ++++++++++++++++++++++++- 5 files changed, 385 insertions(+), 32 deletions(-) diff --git a/internal/initialize/initialize_test.go b/internal/initialize/initialize_test.go index c474c7a27..e3cada89c 100644 --- a/internal/initialize/initialize_test.go +++ b/internal/initialize/initialize_test.go @@ -17,6 +17,8 @@ import ( "github.com/stretchr/testify/suite" ) +// TODO = initialize not currently testing R project + type InitializeSuite struct { utiltest.Suite cwd util.AbsolutePath @@ -59,7 +61,7 @@ func (s *InitializeSuite) TestInitEmpty() { s.Equal("My App", cfg.Title) } -func (s *InitializeSuite) createAppPy() { +func (s *InitializeSuite) createAppPy() util.AbsolutePath { appPath := s.cwd.Join("app.py") err := appPath.WriteFile([]byte(` from flask import Flask @@ -67,6 +69,55 @@ func (s *InitializeSuite) createAppPy() { app.run() `), 0666) s.NoError(err) + return appPath +} + +func (s *InitializeSuite) createAppR() util.AbsolutePath { + appPath := s.cwd.Join("app.R") + err := appPath.WriteFile([]byte(` +library(shiny) + +# Define UI for application that draws a histogram +ui <- fluidPage( + + # Application title + titlePanel("Old Faithful Geyser Data"), + + # Sidebar with a slider input for number of bins + sidebarLayout( + sidebarPanel( + sliderInput("bins", + "Number of bins:", + min = 1, + max = 50, + value = 30) + ), + + # Show a plot of the generated distribution + mainPanel( + plotOutput("distPlot") + ) + ) +) + +# Define server logic required to draw a histogram +server <- function(input, output) { + + output$distPlot <- renderPlot({ + # generate bins based on input$bins from ui.R + x <- faithful[, 2] + bins <- seq(min(x), max(x), length.out = input$bins + 1) + + # draw the histogram with the specified number of bins + hist(x, breaks = bins, col = 'darkgray', border = 'white') + }) +} + +# Run the application +shinyApp(ui = ui, server = server) + `), 0666) + s.NoError(err) + return appPath } func (s *InitializeSuite) createHTML() { @@ -98,10 +149,26 @@ func makeMockPythonInspector(util.AbsolutePath, util.Path, logging.Logger) inspe return pyInspector } +var expectedRConfig = &config.R{ + Version: "1.2.3", + PackageManager: "renv", + PackageFile: "renv.lock", +} + +func setupMockRInspector(requiredRReturnValue bool, requiredRError error) func(util.AbsolutePath, util.Path, logging.Logger) (inspect.RInspector, error) { + return func(util.AbsolutePath, util.Path, logging.Logger) (inspect.RInspector, error) { + rInspector := inspect.NewMockRInspector() + rInspector.On("InspectR").Return(expectedRConfig, nil) + rInspector.On("RequiresR").Return(requiredRReturnValue, requiredRError) + return rInspector, nil + } +} + func (s *InitializeSuite) TestInitInferredType() { log := logging.New() s.createAppPy() PythonInspectorFactory = makeMockPythonInspector + RInspectorFactory = setupMockRInspector(false, nil) configName := "" cfg, err := Init(s.cwd, configName, util.Path{}, util.Path{}, log) s.NoError(err) @@ -118,6 +185,7 @@ func (s *InitializeSuite) TestInitRequirementsFile() { s.createHTML() s.createRequirementsFile() PythonInspectorFactory = makeMockPythonInspector + RInspectorFactory = setupMockRInspector(false, nil) configName := "" cfg, err := Init(s.cwd, configName, util.Path{}, util.Path{}, log) s.NoError(err) @@ -133,6 +201,7 @@ func (s *InitializeSuite) TestInitIfNeededWhenNeeded() { log := logging.New() s.createAppPy() PythonInspectorFactory = makeMockPythonInspector + RInspectorFactory = setupMockRInspector(false, nil) configName := "" err := InitIfNeeded(s.cwd, configName, log) s.NoError(err) @@ -160,6 +229,9 @@ func (s *InitializeSuite) TestInitIfNeededWhenNotNeeded() { PythonInspectorFactory = func(util.AbsolutePath, util.Path, logging.Logger) inspect.PythonInspector { return &inspect.MockPythonInspector{} } + RInspectorFactory = func(util.AbsolutePath, util.Path, logging.Logger) (inspect.RInspector, error) { + return &inspect.MockRInspector{}, nil + } err := InitIfNeeded(s.cwd, configName, log) s.NoError(err) newConfig, err := config.FromFile(configPath) @@ -169,25 +241,39 @@ func (s *InitializeSuite) TestInitIfNeededWhenNotNeeded() { func (s *InitializeSuite) TestGetPossibleConfigs() { log := logging.New() - s.createAppPy() + appPy := s.createAppPy() + exist, err := appPy.Exists() + s.NoError(err) + s.Equal(true, exist) + appR := s.createAppR() + exist, err = appR.Exists() + s.NoError(err) + s.Equal(true, exist) - err := s.cwd.Join("index.html").WriteFile([]byte(``), 0666) + err = s.cwd.Join("index.html").WriteFile([]byte(``), 0666) s.NoError(err) PythonInspectorFactory = makeMockPythonInspector + RInspectorFactory = setupMockRInspector(true, nil) + configs, err := GetPossibleConfigs(s.cwd, util.Path{}, util.Path{}, util.RelativePath{}, log) s.NoError(err) - s.Len(configs, 2) - s.Equal(config.ContentTypePythonFlask, configs[0].Type) - s.Equal("app.py", configs[0].Entrypoint) - s.Equal([]string{"/app.py", "/requirements.txt"}, configs[0].Files) - s.Equal(expectedPyConfig, configs[0].Python) - - s.Equal(config.ContentTypeHTML, configs[1].Type) - s.Equal("index.html", configs[1].Entrypoint) - s.Equal([]string{"/index.html"}, configs[1].Files) - s.Nil(configs[1].Python) + s.Len(configs, 3) + s.Equal(config.ContentTypeRShiny, configs[0].Type) + s.Equal("app.R", configs[0].Entrypoint) + s.Equal([]string{"/app.R", "/renv.lock"}, configs[0].Files) + s.Equal(expectedRConfig, configs[0].R) + + s.Equal(config.ContentTypePythonFlask, configs[1].Type) + s.Equal("app.py", configs[1].Entrypoint) + s.Equal([]string{"/app.py", "/requirements.txt", "/renv.lock"}, configs[1].Files) + s.Equal(expectedPyConfig, configs[1].Python) + + s.Equal(config.ContentTypeHTML, configs[2].Type) + s.Equal("index.html", configs[2].Entrypoint) + s.Equal([]string{"/index.html", "/renv.lock"}, configs[2].Files) + s.Nil(configs[2].Python) } func (s *InitializeSuite) TestGetPossibleConfigsEmpty() { diff --git a/internal/inspect/r_test.go b/internal/inspect/r_test.go index ebaa2aeed..a0df77d82 100644 --- a/internal/inspect/r_test.go +++ b/internal/inspect/r_test.go @@ -3,14 +3,16 @@ package inspect // Copyright (C) 2023 by Posit Software, PBC. import ( + "errors" "testing" + "github.com/posit-dev/publisher/internal/config" "github.com/posit-dev/publisher/internal/interpreters" "github.com/posit-dev/publisher/internal/logging" + "github.com/posit-dev/publisher/internal/types" "github.com/posit-dev/publisher/internal/util" "github.com/posit-dev/publisher/internal/util/utiltest" "github.com/spf13/afero" - "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" ) @@ -29,19 +31,19 @@ func (s *RSuite) SetupTest() { s.cwd = cwd err = cwd.MkdirAll(0700) s.NoError(err) +} + +func (s *RSuite) TestNewRInspector() { + log := logging.New() + rPath := util.NewPath("/usr/bin/R", nil) interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { i := interpreters.NewMockRInterpreter() i.On("Init").Return(nil) - i.On("RequiresR", mock.Anything).Return(false, nil) - i.On("GetLockFilePath").Return(util.RelativePath{}, false, nil) + // i.On("RequiresR", mock.Anything).Return(false, nil) + // i.On("GetLockFilePath").Return(util.RelativePath{}, false, nil) return i } -} - -func (s *RSuite) TestNewRInspector() { - log := logging.New() - rPath := util.NewPath("/usr/bin/R", nil) i, err := NewRInspector(s.cwd, rPath, log) s.NoError(err) @@ -50,3 +52,126 @@ func (s *RSuite) TestNewRInspector() { s.Equal(s.cwd, inspector.base) s.Equal(log, inspector.log) } + +func (s *RSuite) TestInspectWithRFound() { + var relPath util.RelativePath + + interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { + i := interpreters.NewMockRInterpreter() + i.On("Init").Return(nil) + i.On("GetRExecutable").Return(util.NewAbsolutePath("R", baseDir.Fs()), nil) + i.On("GetRVersion").Return("1.2.3", nil) + relPath = util.NewRelativePath(baseDir.Join("renv.lock").String(), baseDir.Fs()) + i.On("GetLockFilePath").Return(relPath, true, nil) + return i + } + log := logging.New() + i, err := NewRInspector(s.cwd, util.Path{}, log) + s.NoError(err) + + inspect, err := i.InspectR() + s.NoError(err) + s.Equal("renv", inspect.PackageManager) + s.Equal("1.2.3", inspect.Version) + s.Equal(relPath.String(), inspect.PackageFile) +} + +func (s *RSuite) TestInspectWithNoRFound() { + interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { + i := interpreters.NewMockRInterpreter() + rExecNotFoundError := types.NewAgentError(types.ErrorRExecNotFound, errors.New("info"), nil) + i.On("Init").Return(nil) + i.On("GetRExecutable").Return(util.AbsolutePath{}, nil) + i.On("GetRVersion").Return("", rExecNotFoundError) + // i.On("RequiresR", mock.Anything).Return(false, nil) + relPath := util.NewRelativePath(baseDir.Join("renv_2222.lock").String(), baseDir.Fs()) + i.On("GetLockFilePath").Return(relPath, false, nil) + return i + } + log := logging.New() + i, err := NewRInspector(s.cwd, util.Path{}, log) + s.NoError(err) + + inspect, err := i.InspectR() + s.NoError(err) + s.Equal("renv", inspect.PackageManager) + s.Equal("", inspect.Version) + s.Equal(s.cwd.Join("renv_2222.lock").String(), inspect.PackageFile) +} + +func (s *RSuite) TestRequiresRWithEmptyCfgAndLockfileExists() { + interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { + i := interpreters.NewMockRInterpreter() + relPath := util.NewRelativePath(s.cwd.Join("renv.lock").String(), s.cwd.Fs()) + i.On("GetLockFilePath").Return(relPath, true, nil) + return i + } + + log := logging.New() + i, err := NewRInspector(s.cwd, util.Path{}, log) + s.NoError(err) + + cfg := &config.Config{} + + require, err := i.RequiresR(cfg) + s.NoError(err) + s.Equal(true, require) +} + +func (s *RSuite) TestRequiresRWithEmptyCfgAndLockfileDoesNotExists() { + interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { + i := interpreters.NewMockRInterpreter() + relPath := util.NewRelativePath(s.cwd.Join("renv.lock").String(), s.cwd.Fs()) + i.On("GetLockFilePath").Return(relPath, false, nil) + return i + } + + log := logging.New() + i, err := NewRInspector(s.cwd, util.Path{}, log) + s.NoError(err) + + cfg := &config.Config{} + + require, err := i.RequiresR(cfg) + s.NoError(err) + s.Equal(false, require) +} + +func (s *RSuite) TestRequiresRWithRCfg() { + log := logging.New() + i, err := NewRInspector(s.cwd, util.Path{}, log) + s.NoError(err) + + cfg := &config.Config{ + R: &config.R{}, + } + require, err := i.RequiresR(cfg) + s.NoError(err) + s.Equal(true, require) +} + +func (s *RSuite) TestRequiresRNoRButWithTypeAsPython() { + log := logging.New() + i, err := NewRInspector(s.cwd, util.Path{}, log) + s.NoError(err) + + cfg := &config.Config{ + Type: config.ContentTypePythonFastAPI, + } + require, err := i.RequiresR(cfg) + s.NoError(err) + s.Equal(false, require) +} + +func (s *RSuite) TestRequiresRNoRButWithTypeEqualContentTypeHTML() { + log := logging.New() + i, err := NewRInspector(s.cwd, util.Path{}, log) + s.NoError(err) + + cfg := &config.Config{ + Type: config.ContentTypeHTML, + } + require, err := i.RequiresR(cfg) + s.NoError(err) + s.Equal(false, require) +} diff --git a/internal/interpreters/r.go b/internal/interpreters/r.go index 677dbc369..714d880df 100644 --- a/internal/interpreters/r.go +++ b/internal/interpreters/r.go @@ -364,10 +364,14 @@ func (i *defaultRInterpreter) CreateLockfile(lockfilePath util.AbsolutePath) err if err != nil { return err } - - escaped := strings.ReplaceAll(lockfilePath.String(), `\`, `\\`) - code := fmt.Sprintf(`renv::snapshot(lockfile="%s")`, escaped) - args := []string{"-s", "-e", code} + var cmd string + if lockfilePath.String() == "" { + cmd = "renv::snapshot()" + } else { + escaped := strings.ReplaceAll(lockfilePath.String(), `\`, `\\`) + cmd = fmt.Sprintf(`renv::snapshot(lockfile="%s")`, escaped) + } + args := []string{"-s", "-e", cmd} stdout, stderr, err := i.executor.RunCommand(rExecutable.String(), args, i.base, i.log) i.log.Debug("renv::snapshot()", "out", string(stdout), "err", string(stderr)) return err diff --git a/internal/interpreters/r_mock.go b/internal/interpreters/r_mock.go index 8ea8a11d5..9510fdffe 100644 --- a/internal/interpreters/r_mock.go +++ b/internal/interpreters/r_mock.go @@ -3,8 +3,6 @@ package interpreters // Copyright (C) 2024 by Posit Software, PBC. import ( - "errors" - "github.com/posit-dev/publisher/internal/util" "github.com/stretchr/testify/mock" ) @@ -35,7 +33,7 @@ func (m *MockRInterpreter) GetRExecutable() (util.AbsolutePath, error) { if path, ok := i.(string); ok { return util.NewAbsolutePath(path, nil), args.Error(1) } else { - return util.AbsolutePath{}, errors.New("invalid string argument") + return util.AbsolutePath{}, args.Error(1) } } } @@ -50,7 +48,7 @@ func (m *MockRInterpreter) GetRVersion() (string, error) { if version, ok := i.(string); ok { return version, args.Error(1) } else { - return "", errors.New("invalid string argument") + return "", args.Error(1) } } } @@ -72,7 +70,7 @@ func (m *MockRInterpreter) GetLockFilePath() (util.RelativePath, bool, error) { if !ok { exists = false } - return util.NewRelativePath(path.String(), nil), exists, args.Error(2) + return path, exists, args.Error(2) } } diff --git a/internal/interpreters/r_test.go b/internal/interpreters/r_test.go index 86b4e5114..fc098d63b 100644 --- a/internal/interpreters/r_test.go +++ b/internal/interpreters/r_test.go @@ -3,6 +3,7 @@ package interpreters // Copyright (C) 2023 by Posit Software, PBC. import ( + "errors" "fmt" "runtime" "strings" @@ -10,6 +11,7 @@ import ( "github.com/posit-dev/publisher/internal/executor/executortest" "github.com/posit-dev/publisher/internal/logging" + "github.com/posit-dev/publisher/internal/types" "github.com/posit-dev/publisher/internal/util" "github.com/posit-dev/publisher/internal/util/utiltest" "github.com/spf13/afero" @@ -68,6 +70,48 @@ func (s *RSuite) TestNewRInterpreter() { s.ErrorIs(err, NotYetInitialized) } +func (s *RSuite) TestInit() { + log := logging.New() + + rPath := s.cwd.Join("bin", "R") + rPath.Dir().MkdirAll(0777) + rPath.WriteFile([]byte(nil), 0777) + + i := NewRInterpreter(s.cwd, rPath.Path, log) + interpreter := i.(*defaultRInterpreter) + executor := executortest.NewMockExecutor() + executor.On("RunCommand", mock.Anything, []string{"--version"}, mock.Anything, mock.Anything).Return([]byte("R version 4.3.0 (2023-04-21)"), nil, nil) + executor.On("RunCommand", mock.Anything, []string{"-s", "-e", "renv::paths$lockfile()"}, mock.Anything, mock.Anything).Return([]byte(`[1] "/test/sample-content/shinyapp/renv.lock"`), nil, nil).Once() + interpreter.executor = executor + interpreter.initialized = true + interpreter.fs = s.cwd.Fs() + + err := i.Init() + s.NoError(err) + + s.Equal(rPath.String(), interpreter.rExecutable.String()) + s.Equal("4.3.0", interpreter.version) + s.Equal("", interpreter.lockfileRelPath.String()) + s.Equal(false, interpreter.lockfileExists) + s.Equal(true, interpreter.initialized) + + // Now we lazy load the lock file path + lockFilePath, exists, err := interpreter.GetLockFilePath() + absLockFile := util.NewAbsolutePath(lockFilePath.String(), s.fs) + absExpectedLockFile := util.NewAbsolutePath("/test/sample-content/shinyapp/renv.lock", s.fs) + s.Equal(absExpectedLockFile.String(), absLockFile.String()) + s.Equal(false, exists) + s.NoError(err) + + // Make sure calling doesn't re-invoke discovery. We test this by having the R renv:: command + // only return once. If it is called more than that, the test code will panic. + lockFilePath, exists, err = interpreter.GetLockFilePath() + absLockFile = util.NewAbsolutePath(lockFilePath.String(), s.fs) + s.Equal(absExpectedLockFile.String(), absLockFile.String()) + s.Equal(false, exists) + s.NoError(err) +} + type OutputTestData struct { versionOutput string expectedVersion string @@ -285,6 +329,22 @@ func (s *RSuite) TestIsRExecutableValid() { } } +func (s *RSuite) TestResolveRExecutableWhenNotFoundOrInvalid() { + log := logging.New() + + i := NewRInterpreter(s.cwd, util.Path{}, log) + interpreter := i.(*defaultRInterpreter) + executor := executortest.NewMockExecutor() + executor.On("RunCommand", mock.Anything, []string{"--version"}, mock.Anything, mock.Anything).Return([]byte("R version 4.3.0 (2023-04-21)"), nil, nil) + executor.On("ValidateRExecutable").Return("", errors.New("an error")) + interpreter.executor = executor + interpreter.initialized = true + interpreter.fs = s.cwd.Fs() + + err := interpreter.resolveRExecutable() + s.Error(err) +} + // Validate when we pass in a path that exists and is valid, we get it back func (s *RSuite) TestResolveRExecutableWhenPassedInPathExistsAndIsValid() { log := logging.New() @@ -367,11 +427,11 @@ func (s *RSuite) TestResolveRExecutableWhenPassedInPathExistsButNotValid() { // Validate when we do not pass in a value and // have R on the path that exists but is not valid func (s *RSuite) TestResolveRExecutableWhenPathContainsRButNotValid() { - log := logging.New() - if runtime.GOOS == "windows" { s.T().Skip("This test does not run on Windows") } + + log := logging.New() pathLooker := util.NewMockPathLooker() pathLooker.On("LookPath", "R").Return("/some/R", nil) rPath := s.cwd.Join("some", "R") @@ -392,6 +452,39 @@ func (s *RSuite) TestResolveRExecutableWhenPathContainsRButNotValid() { s.Equal("unable to detect any R interpreters", err.Error()) } +// Validate if unable to run R executable, get an error in +func (s *RSuite) TestGetRVersionFromRExecutableWithInvalidR() { + + log := logging.New() + i := NewRInterpreter(s.cwd, util.Path{}, log) + interpreter := i.(*defaultRInterpreter) + interpreter.initialized = true + executor := executortest.NewMockExecutor() + executor.On("RunCommand", mock.Anything, []string{"--version"}, mock.Anything, mock.Anything).Return([]byte(""), nil, errors.New("problem")) + interpreter.executor = executor + interpreter.fs = s.cwd.Fs() + + _, err := interpreter.getRVersionFromRExecutable("does-not-matter") + s.Error(err) + + err = interpreter.resolveRenvLockFile("does-not-matter") + s.NoError(err) +} + +func (s *RSuite) TestResolveRenvLockFileWithInvalidR() { + log := logging.New() + i := NewRInterpreter(s.cwd, util.Path{}, log) + interpreter := i.(*defaultRInterpreter) + interpreter.initialized = true + executor := executortest.NewMockExecutor() + executor.On("RunCommand", mock.Anything, []string{"--version"}, mock.Anything, mock.Anything).Return([]byte(""), nil, errors.New("problem")) + interpreter.executor = executor + interpreter.fs = s.cwd.Fs() + + err := interpreter.resolveRenvLockFile("does-not-matter") + s.NoError(err) +} + // Validate that we find the lock file that R specifies // with default name if it exists func (s *RSuite) TestResolveRenvLockFileWithRSpecifyingDefaultNameAndExists() { @@ -470,3 +563,50 @@ func (s *RSuite) TestResolveRenvLockFileWithRSpecialNameAndExists() { s.Equal("renv-project222.lock", interpreter.lockfileRelPath.String()) s.Equal(true, interpreter.lockfileExists) } + +func (s *RSuite) TestCreateLockfileWithInvalidR() { + log := logging.New() + i := NewRInterpreter(s.cwd, util.Path{}, log) + interpreter := i.(*defaultRInterpreter) + interpreter.initialized = true + interpreter.fs = s.cwd.Fs() + + err := interpreter.CreateLockfile(util.NewAbsolutePath("abc/xxy/renv.lock", s.cwd.Fs())) + s.Error(err) + _, ok := types.IsAgentErrorOf(err, types.ErrorRExecNotFound) + s.Equal(true, ok) +} + +func (s *RSuite) TestCreateLockfileWithNonEmptyPath() { + log := logging.New() + i := NewRInterpreter(s.cwd, util.Path{}, log) + interpreter := i.(*defaultRInterpreter) + interpreter.initialized = true + interpreter.rExecutable = util.NewAbsolutePath("/usr/bin/R", s.cwd.Fs()) + interpreter.version = "1.2.3" + executor := executortest.NewMockExecutor() + executor.On("RunCommand", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]byte("success"), nil, nil) + interpreter.executor = executor + interpreter.fs = s.cwd.Fs() + + err := i.CreateLockfile(util.NewAbsolutePath("abc/xxy/renv.lock", s.cwd.Fs())) + s.NoError(err) +} + +func (s *RSuite) TestCreateLockfileWithEmptyPath() { + log := logging.New() + i := NewRInterpreter(s.cwd, util.Path{}, log) + interpreter := i.(*defaultRInterpreter) + interpreter.initialized = true + interpreter.rExecutable = util.NewAbsolutePath("/usr/bin/R", s.cwd.Fs()) + interpreter.version = "1.2.3" + executor := executortest.NewMockExecutor() + executor.On("RunCommand", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]byte("success"), nil, nil) + interpreter.executor = executor + interpreter.fs = s.cwd.Fs() + + err := i.CreateLockfile(util.AbsolutePath{}) + s.NoError(err) +} + +// Confirm that we don't resolve lock file on init From 4e65fff15c753391ec4bf67063b0fc03c4e39ede Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Wed, 18 Dec 2024 17:35:17 -0800 Subject: [PATCH 15/28] reset CI test file --- test/vscode-ui/test/specs/fastapi.spec.ts | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/test/vscode-ui/test/specs/fastapi.spec.ts b/test/vscode-ui/test/specs/fastapi.spec.ts index a3b9319db..54709e3af 100644 --- a/test/vscode-ui/test/specs/fastapi.spec.ts +++ b/test/vscode-ui/test/specs/fastapi.spec.ts @@ -53,21 +53,6 @@ describe("VS Code Extension UI Test", () => { const simplepy = browser.$(`aria/simple.py`); await simplepy.click(); - // const elem = await $("#quickInput_message"); - // await browser.waitUntil( - // async function () { - // return ( - // (await elem.getText()) === - // "Enter a title for your content or application. (Press 'Enter' to confirm or 'Escape' to cancel)" - // ); - // }, - // { - // timeout: 60000, - // timeoutMsg: - // "expected Title prompt to be visible after 60 seconds of inspection processing", - // }, - // ); - const titleMessage = browser.$("#quickInput_message"); await expect(titleMessage).toHaveText( "Enter a title for your content or application. (Press 'Enter' to confirm or 'Escape' to cancel)", From c56914df690875a9ef30ddfc7dfd2bc81f15c926 Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Thu, 19 Dec 2024 08:19:57 -0800 Subject: [PATCH 16/28] remove unneeded comment --- internal/interpreters/r_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/interpreters/r_test.go b/internal/interpreters/r_test.go index fc098d63b..337f4decb 100644 --- a/internal/interpreters/r_test.go +++ b/internal/interpreters/r_test.go @@ -608,5 +608,3 @@ func (s *RSuite) TestCreateLockfileWithEmptyPath() { err := i.CreateLockfile(util.AbsolutePath{}) s.NoError(err) } - -// Confirm that we don't resolve lock file on init From 4b79ef2d7b741ffab4834c4d50b4efd4994cd75b Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Thu, 19 Dec 2024 22:41:17 -0800 Subject: [PATCH 17/28] adjust testability to use dependency injection --- cmd/publisher/commands/deploy.go | 3 +- cmd/publisher/commands/init.go | 3 +- cmd/publisher/commands/redeploy.go | 3 +- internal/initialize/initialize.go | 95 +++++++--- internal/initialize/initialize_test.go | 142 ++++++++++----- .../dependencies/renv/available_packages.go | 12 +- .../renv/available_packages_test.go | 42 +++-- .../dependencies/renv/manifest_packages.go | 2 +- internal/inspect/detectors/all.go | 2 + internal/inspect/python.go | 2 + internal/inspect/r.go | 18 +- internal/inspect/r_test.go | 78 +++++--- internal/interpreters/r.go | 114 +++++++----- internal/interpreters/r_test.go | 170 ++++++++---------- internal/publish/publish_test.go | 16 +- internal/services/api/api_service.go | 2 +- internal/services/api/post_inspect.go | 8 +- internal/services/api/post_packages_r_scan.go | 15 +- .../services/api/post_packages_r_scan_test.go | 90 +++++++--- 19 files changed, 533 insertions(+), 284 deletions(-) diff --git a/cmd/publisher/commands/deploy.go b/cmd/publisher/commands/deploy.go index 74c27f562..2e27996aa 100644 --- a/cmd/publisher/commands/deploy.go +++ b/cmd/publisher/commands/deploy.go @@ -53,7 +53,8 @@ func (cmd *DeployCmd) Run(args *cli_types.CommonArgs, ctx *cli_types.CLIContext) return err } } - err = initialize.InitIfNeeded(absPath, cmd.ConfigName, ctx.Logger) + i := initialize.NewDefaultInitialize() + err = i.InitIfNeeded(absPath, cmd.ConfigName, ctx.Logger) if err != nil { return err } diff --git a/cmd/publisher/commands/init.go b/cmd/publisher/commands/init.go index ded08850c..13a747c1c 100644 --- a/cmd/publisher/commands/init.go +++ b/cmd/publisher/commands/init.go @@ -52,7 +52,8 @@ func (cmd *InitCommand) Run(args *cli_types.CommonArgs, ctx *cli_types.CLIContex if cmd.ConfigName == "" { cmd.ConfigName = config.DefaultConfigName } - cfg, err := initialize.Init(absPath, cmd.ConfigName, cmd.Python, cmd.R, ctx.Logger) + i := initialize.NewDefaultInitialize() + cfg, err := i.Init(absPath, cmd.ConfigName, cmd.Python, cmd.R, ctx.Logger) if err != nil { return err } diff --git a/cmd/publisher/commands/redeploy.go b/cmd/publisher/commands/redeploy.go index dbaf2b2cc..28e2a4c5f 100644 --- a/cmd/publisher/commands/redeploy.go +++ b/cmd/publisher/commands/redeploy.go @@ -33,7 +33,8 @@ func (cmd *RedeployCmd) Run(args *cli_types.CommonArgs, ctx *cli_types.CLIContex } ctx.Logger = events.NewCLILogger(args.Verbose, os.Stderr) - err = initialize.InitIfNeeded(absPath, cmd.ConfigName, ctx.Logger) + i := initialize.NewDefaultInitialize() + err = i.InitIfNeeded(absPath, cmd.ConfigName, ctx.Logger) if err != nil { return err } diff --git a/internal/initialize/initialize.go b/internal/initialize/initialize.go index b0f0f27ae..346a2f00f 100644 --- a/internal/initialize/initialize.go +++ b/internal/initialize/initialize.go @@ -10,13 +10,66 @@ import ( "github.com/posit-dev/publisher/internal/config" "github.com/posit-dev/publisher/internal/inspect" "github.com/posit-dev/publisher/internal/inspect/detectors" + "github.com/posit-dev/publisher/internal/interpreters" "github.com/posit-dev/publisher/internal/logging" "github.com/posit-dev/publisher/internal/util" ) -var ContentDetectorFactory = detectors.NewContentTypeDetector -var PythonInspectorFactory = inspect.NewPythonInspector -var RInspectorFactory = inspect.NewRInspector +type Initialize interface { + Init(base util.AbsolutePath, configName string, python util.Path, rExecutable util.Path, log logging.Logger) (*config.Config, error) + InitIfNeeded(path util.AbsolutePath, configName string, log logging.Logger) error + GetPossibleConfigs(base util.AbsolutePath, python util.Path, rExecutable util.Path, entrypoint util.RelativePath, log logging.Logger) ([]*config.Config, error) + + normalizeConfig(cfg *config.Config, base util.AbsolutePath, python util.Path, rExecutable util.Path, entrypoint util.RelativePath, log logging.Logger) error +} + +type defaultInitialize struct { + contentTypeDetectorFactory detectors.ContentTypeDetectorFactory + pythonInspectorFactory inspect.PythonInspectorFactory + rInspectorFactory inspect.RInspectorFactory + rInterpreterFactory interpreters.RInterpreterFactory +} + +var _ Initialize = &defaultInitialize{} + +func NewDefaultInitialize() Initialize { + return NewInitialize(nil, nil, nil, nil) +} + +func NewInitialize( + contentTypeDetectorFactoryOverride detectors.ContentTypeDetectorFactory, + pythonInspectorFactoryOverride inspect.PythonInspectorFactory, + rInspectorFactoryOverride inspect.RInspectorFactory, + rInterpreterFactoryOverride interpreters.RInterpreterFactory, +) Initialize { + initialize := &defaultInitialize{ + contentTypeDetectorFactory: nil, + pythonInspectorFactory: nil, + rInspectorFactory: nil, + rInterpreterFactory: nil, + } + if contentTypeDetectorFactoryOverride == nil { + initialize.contentTypeDetectorFactory = detectors.NewContentTypeDetector + } else { + initialize.contentTypeDetectorFactory = contentTypeDetectorFactoryOverride + } + if pythonInspectorFactoryOverride == nil { + initialize.pythonInspectorFactory = inspect.NewPythonInspector + } else { + initialize.pythonInspectorFactory = pythonInspectorFactoryOverride + } + if rInspectorFactoryOverride == nil { + initialize.rInspectorFactory = inspect.NewRInspector + } else { + initialize.rInspectorFactory = rInspectorFactoryOverride + } + if rInterpreterFactoryOverride == nil { + initialize.rInterpreterFactory = interpreters.NewRInterpreter + } else { + initialize.rInterpreterFactory = rInterpreterFactoryOverride + } + return initialize +} var errNoDeployableContent = fmt.Errorf("no deployable content was detected") @@ -24,9 +77,9 @@ const initialComment = ` Configuration file generated by Posit Publisher. Please review and modify as needed. See the documentation for more options: https://github.com/posit-dev/publisher/blob/main/docs/configuration.md` -func inspectProject(base util.AbsolutePath, python util.Path, rExecutable util.Path, log logging.Logger) (*config.Config, error) { +func (i *defaultInitialize) inspectProject(base util.AbsolutePath, python util.Path, rExecutable util.Path, log logging.Logger) (*config.Config, error) { log.Info("Detecting deployment type and entrypoint...", "path", base.String()) - typeDetector := ContentDetectorFactory(log) + typeDetector := i.contentTypeDetectorFactory(log) configs, err := typeDetector.InferType(base, util.RelativePath{}) if err != nil { @@ -47,12 +100,12 @@ func inspectProject(base util.AbsolutePath, python util.Path, rExecutable util.P cfg.Title = base.Base() } - needPython, err := requiresPython(cfg, base) + needPython, err := i.requiresPython(cfg, base) if err != nil { return nil, err } if needPython { - inspector := PythonInspectorFactory(base, python, log) + inspector := i.pythonInspectorFactory(base, python, log) pyConfig, err := inspector.InspectPython() if err != nil { return nil, err @@ -60,7 +113,7 @@ func inspectProject(base util.AbsolutePath, python util.Path, rExecutable util.P cfg.Python = pyConfig } - rInspector, err := RInspectorFactory(base, rExecutable, log) + rInspector, err := i.rInspectorFactory(base, rExecutable, log, i.rInterpreterFactory, nil) if err != nil { return nil, err } @@ -84,7 +137,7 @@ func inspectProject(base util.AbsolutePath, python util.Path, rExecutable util.P return cfg, nil } -func requiresPython(cfg *config.Config, base util.AbsolutePath) (bool, error) { +func (i *defaultInitialize) requiresPython(cfg *config.Config, base util.AbsolutePath) (bool, error) { if cfg.Python != nil && cfg.Python.Version == "" { // InferType returned a python configuration for us to fill in. return true, nil @@ -100,7 +153,7 @@ func requiresPython(cfg *config.Config, base util.AbsolutePath) (bool, error) { return exists, nil } -func normalizeConfig( +func (i *defaultInitialize) normalizeConfig( cfg *config.Config, base util.AbsolutePath, python util.Path, @@ -133,14 +186,14 @@ func normalizeConfig( log.Debug("Inspector populate files list", "total_files", len(cfg.Files)) } - needPython, err := requiresPython(cfg, base) + needPython, err := i.requiresPython(cfg, base) if err != nil { log.Debug("Error while determining Python as a requirement", "error", err.Error()) return err } if needPython { log.Debug("Determined that Python is required") - inspector := PythonInspectorFactory(base, python, log) + inspector := i.pythonInspectorFactory(base, python, log) pyConfig, err := inspector.InspectPython() if err != nil { log.Debug("Error while inspecting to generate a Python based configuration", "error", err.Error()) @@ -150,7 +203,7 @@ func normalizeConfig( cfg.Files = append(cfg.Files, fmt.Sprint("/", cfg.Python.PackageFile)) } - rInspector, err := RInspectorFactory(base, rExecutable, log) + rInspector, err := i.rInspectorFactory(base, rExecutable, log, i.rInterpreterFactory, nil) if err != nil { log.Debug("Error while creating the RInspector", "error", err.Error()) return err @@ -174,7 +227,7 @@ func normalizeConfig( return nil } -func GetPossibleConfigs( +func (i *defaultInitialize) GetPossibleConfigs( base util.AbsolutePath, python util.Path, rExecutable util.Path, @@ -182,14 +235,14 @@ func GetPossibleConfigs( log logging.Logger) ([]*config.Config, error) { log.Info("Detecting deployment type and entrypoint...", "path", base.String()) - typeDetector := ContentDetectorFactory(log) + typeDetector := i.contentTypeDetectorFactory(log) configs, err := typeDetector.InferType(base, entrypoint) if err != nil { return nil, fmt.Errorf("error detecting content type: %w", err) } for _, cfg := range configs { - err = normalizeConfig(cfg, base, python, rExecutable, entrypoint, log) + err = i.normalizeConfig(cfg, base, python, rExecutable, entrypoint, log) if err != nil { return nil, err } @@ -197,15 +250,15 @@ func GetPossibleConfigs( return configs, nil } -func Init(base util.AbsolutePath, configName string, python util.Path, rExecutable util.Path, log logging.Logger) (*config.Config, error) { +func (i *defaultInitialize) Init(base util.AbsolutePath, configName string, python util.Path, rExecutable util.Path, log logging.Logger) (*config.Config, error) { if configName == "" { configName = config.DefaultConfigName } - cfg, err := inspectProject(base, python, rExecutable, log) + cfg, err := i.inspectProject(base, python, rExecutable, log) if err != nil { return nil, err } - err = normalizeConfig(cfg, base, python, rExecutable, util.RelativePath{}, log) + err = i.normalizeConfig(cfg, base, python, rExecutable, util.RelativePath{}, log) if err != nil { return nil, err } @@ -218,7 +271,7 @@ func Init(base util.AbsolutePath, configName string, python util.Path, rExecutab } // InitIfNeeded runs an auto-initialize if the specified config file does not exist. -func InitIfNeeded(path util.AbsolutePath, configName string, log logging.Logger) error { +func (i *defaultInitialize) InitIfNeeded(path util.AbsolutePath, configName string, log logging.Logger) error { configPath := config.GetConfigPath(path, configName) exists, err := configPath.Exists() if err != nil { @@ -226,7 +279,7 @@ func InitIfNeeded(path util.AbsolutePath, configName string, log logging.Logger) } if !exists { log.Info("Configuration file does not exist; creating it", "path", configPath.String()) - _, err = Init(path, configName, util.Path{}, util.Path{}, log) + _, err = i.Init(path, configName, util.Path{}, util.Path{}, log) if err != nil { return err } diff --git a/internal/initialize/initialize_test.go b/internal/initialize/initialize_test.go index e3cada89c..dcd34a395 100644 --- a/internal/initialize/initialize_test.go +++ b/internal/initialize/initialize_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/posit-dev/publisher/internal/config" + "github.com/posit-dev/publisher/internal/executor" "github.com/posit-dev/publisher/internal/inspect" "github.com/posit-dev/publisher/internal/inspect/detectors" "github.com/posit-dev/publisher/internal/interpreters" @@ -28,24 +29,36 @@ func TestInitializeSuite(t *testing.T) { suite.Run(t, new(InitializeSuite)) } -func (s *InitializeSuite) SetupTest() { - // Restore default factories for each test - ContentDetectorFactory = detectors.NewContentTypeDetector - PythonInspectorFactory = inspect.NewPythonInspector +func setupMockRInspector(requiredRReturnValue bool, requiredRError error) inspect.RInspectorFactory { + return func(base util.AbsolutePath, rExecutable util.Path, log logging.Logger, rInterpreterFactoryOverride interpreters.RInterpreterFactory, cmdExecutorOverride executor.Executor) (inspect.RInspector, error) { + rInspector := inspect.NewMockRInspector() + rInspector.On("InspectR").Return(expectedRConfig, nil) + rInspector.On("RequiresR").Return(requiredRReturnValue, requiredRError) + return rInspector, nil + } +} + +func setupNewRInterpreterMock( + base util.AbsolutePath, + rExecutableParam util.Path, + log logging.Logger, + cmdExecutorOverride executor.Executor, + pathLookerOverride util.PathLooker, + existsFuncOverride interpreters.ExistsFunc, +) (interpreters.RInterpreter, error) { + i := interpreters.NewMockRInterpreter() + i.On("Init").Return(nil) + i.On("RequiresR", mock.Anything).Return(false, nil) + i.On("GetLockFilePath").Return(util.RelativePath{}, false, nil) + return i, nil +} +func (s *InitializeSuite) SetupTest() { cwd, err := util.Getwd(afero.NewMemMapFs()) s.NoError(err) s.cwd = cwd err = cwd.MkdirAll(0700) s.NoError(err) - - interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { - i := interpreters.NewMockRInterpreter() - i.On("Init").Return(nil) - i.On("RequiresR", mock.Anything).Return(false, nil) - i.On("GetLockFilePath").Return(util.RelativePath{}, false, nil) - return i - } } func (s *InitializeSuite) TestInitEmpty() { @@ -55,7 +68,14 @@ func (s *InitializeSuite) TestInitEmpty() { err := path.Mkdir(0777) s.NoError(err) - cfg, err := Init(path, "", util.Path{}, util.Path{}, log) + i := NewInitialize( + detectors.NewContentTypeDetector, + inspect.NewPythonInspector, + inspect.NewRInspector, + setupNewRInterpreterMock, + ) + + cfg, err := i.Init(path, "", util.Path{}, util.Path{}, log) s.Nil(err) s.Equal(config.ContentTypeUnknown, cfg.Type) s.Equal("My App", cfg.Title) @@ -155,22 +175,19 @@ var expectedRConfig = &config.R{ PackageFile: "renv.lock", } -func setupMockRInspector(requiredRReturnValue bool, requiredRError error) func(util.AbsolutePath, util.Path, logging.Logger) (inspect.RInspector, error) { - return func(util.AbsolutePath, util.Path, logging.Logger) (inspect.RInspector, error) { - rInspector := inspect.NewMockRInspector() - rInspector.On("InspectR").Return(expectedRConfig, nil) - rInspector.On("RequiresR").Return(requiredRReturnValue, requiredRError) - return rInspector, nil - } -} - func (s *InitializeSuite) TestInitInferredType() { log := logging.New() s.createAppPy() - PythonInspectorFactory = makeMockPythonInspector - RInspectorFactory = setupMockRInspector(false, nil) + + i := NewInitialize( + detectors.NewContentTypeDetector, + makeMockPythonInspector, + setupMockRInspector(false, nil), + setupNewRInterpreterMock, + ) + configName := "" - cfg, err := Init(s.cwd, configName, util.Path{}, util.Path{}, log) + cfg, err := i.Init(s.cwd, configName, util.Path{}, util.Path{}, log) s.NoError(err) configPath := config.GetConfigPath(s.cwd, configName) cfg2, err := config.FromFile(configPath) @@ -184,10 +201,16 @@ func (s *InitializeSuite) TestInitRequirementsFile() { log := logging.New() s.createHTML() s.createRequirementsFile() - PythonInspectorFactory = makeMockPythonInspector - RInspectorFactory = setupMockRInspector(false, nil) + + i := NewInitialize( + detectors.NewContentTypeDetector, + makeMockPythonInspector, + setupMockRInspector(false, nil), + setupNewRInterpreterMock, + ) + configName := "" - cfg, err := Init(s.cwd, configName, util.Path{}, util.Path{}, log) + cfg, err := i.Init(s.cwd, configName, util.Path{}, util.Path{}, log) s.NoError(err) configPath := config.GetConfigPath(s.cwd, configName) cfg2, err := config.FromFile(configPath) @@ -200,10 +223,16 @@ func (s *InitializeSuite) TestInitRequirementsFile() { func (s *InitializeSuite) TestInitIfNeededWhenNeeded() { log := logging.New() s.createAppPy() - PythonInspectorFactory = makeMockPythonInspector - RInspectorFactory = setupMockRInspector(false, nil) + + i := NewInitialize( + detectors.NewContentTypeDetector, + makeMockPythonInspector, + setupMockRInspector(false, nil), + setupNewRInterpreterMock, + ) + configName := "" - err := InitIfNeeded(s.cwd, configName, log) + err := i.InitIfNeeded(s.cwd, configName, log) s.NoError(err) configPath := config.GetConfigPath(s.cwd, configName) cfg, err := config.FromFile(configPath) @@ -226,13 +255,21 @@ func (s *InitializeSuite) TestInitIfNeededWhenNotNeeded() { cfg.WriteFile(configPath) cfg.FillDefaults() - PythonInspectorFactory = func(util.AbsolutePath, util.Path, logging.Logger) inspect.PythonInspector { + pythonInspectorFactory := func(util.AbsolutePath, util.Path, logging.Logger) inspect.PythonInspector { return &inspect.MockPythonInspector{} } - RInspectorFactory = func(util.AbsolutePath, util.Path, logging.Logger) (inspect.RInspector, error) { + rInspectorFactory := func(util.AbsolutePath, util.Path, logging.Logger, interpreters.RInterpreterFactory, executor.Executor) (inspect.RInspector, error) { return &inspect.MockRInspector{}, nil } - err := InitIfNeeded(s.cwd, configName, log) + + i := NewInitialize( + detectors.NewContentTypeDetector, + pythonInspectorFactory, + rInspectorFactory, + setupNewRInterpreterMock, + ) + + err := i.InitIfNeeded(s.cwd, configName, log) s.NoError(err) newConfig, err := config.FromFile(configPath) s.NoError(err) @@ -253,10 +290,14 @@ func (s *InitializeSuite) TestGetPossibleConfigs() { err = s.cwd.Join("index.html").WriteFile([]byte(``), 0666) s.NoError(err) - PythonInspectorFactory = makeMockPythonInspector - RInspectorFactory = setupMockRInspector(true, nil) + i := NewInitialize( + detectors.NewContentTypeDetector, + makeMockPythonInspector, + setupMockRInspector(true, nil), + setupNewRInterpreterMock, + ) - configs, err := GetPossibleConfigs(s.cwd, util.Path{}, util.Path{}, util.RelativePath{}, log) + configs, err := i.GetPossibleConfigs(s.cwd, util.Path{}, util.Path{}, util.RelativePath{}, log) s.NoError(err) s.Len(configs, 3) @@ -279,7 +320,14 @@ func (s *InitializeSuite) TestGetPossibleConfigs() { func (s *InitializeSuite) TestGetPossibleConfigsEmpty() { log := logging.New() - configs, err := GetPossibleConfigs(s.cwd, util.Path{}, util.Path{}, util.RelativePath{}, log) + i := NewInitialize( + detectors.NewContentTypeDetector, + inspect.NewPythonInspector, + inspect.NewRInspector, + setupNewRInterpreterMock, + ) + + configs, err := i.GetPossibleConfigs(s.cwd, util.Path{}, util.Path{}, util.RelativePath{}, log) s.NoError(err) s.Len(configs, 1) @@ -292,8 +340,15 @@ func (s *InitializeSuite) TestGetPossibleConfigsWithMissingEntrypoint() { log := logging.New() s.createAppPy() + i := NewInitialize( + detectors.NewContentTypeDetector, + inspect.NewPythonInspector, + inspect.NewRInspector, + setupNewRInterpreterMock, + ) + entrypoint := util.NewRelativePath("nonexistent.py", s.cwd.Fs()) - configs, err := GetPossibleConfigs(s.cwd, util.Path{}, util.Path{}, entrypoint, log) + configs, err := i.GetPossibleConfigs(s.cwd, util.Path{}, util.Path{}, entrypoint, log) s.NoError(err) s.Len(configs, 1) @@ -308,8 +363,15 @@ func (s *InitializeSuite) TestNormalizeConfigHandlesUnknownConfigs() { cfg := config.New() cfg.Type = config.ContentTypeUnknown + i := NewInitialize( + detectors.NewContentTypeDetector, + inspect.NewPythonInspector, + inspect.NewRInspector, + setupNewRInterpreterMock, + ) + ep := util.NewRelativePath("notreal.py", s.cwd.Fs()) - normalizeConfig(cfg, s.cwd, util.Path{}, util.Path{}, ep, log) + i.normalizeConfig(cfg, s.cwd, util.Path{}, util.Path{}, ep, log) // Entrypoint is set from the relative path passed to normalizeConfig s.Equal("notreal.py", cfg.Entrypoint) diff --git a/internal/inspect/dependencies/renv/available_packages.go b/internal/inspect/dependencies/renv/available_packages.go index 3b1b279a6..79df4779a 100644 --- a/internal/inspect/dependencies/renv/available_packages.go +++ b/internal/inspect/dependencies/renv/available_packages.go @@ -30,10 +30,16 @@ type defaultAvailablePackagesLister struct { rExecutor executor.Executor } -func NewAvailablePackageLister(base util.AbsolutePath, rExecutable util.Path, log logging.Logger) (*defaultAvailablePackagesLister, error) { +func NewAvailablePackageLister(base util.AbsolutePath, rExecutable util.Path, log logging.Logger, rInterpreterFactoryOverride interpreters.RInterpreterFactory, cmdExecutorOverride executor.Executor) (*defaultAvailablePackagesLister, error) { - rInterpreter := interpreters.NewRInterpreter(base, rExecutable, log) - err := rInterpreter.Init() + var rInterpreter interpreters.RInterpreter + var err error + + if rInterpreterFactoryOverride != nil { + rInterpreter, err = rInterpreterFactoryOverride(base, rExecutable, log, cmdExecutorOverride, nil, nil) + } else { + rInterpreter, err = interpreters.NewRInterpreter(base, rExecutable, log, nil, nil, nil) + } return &defaultAvailablePackagesLister{ base: base, diff --git a/internal/inspect/dependencies/renv/available_packages_test.go b/internal/inspect/dependencies/renv/available_packages_test.go index 238a83255..69ab136c8 100644 --- a/internal/inspect/dependencies/renv/available_packages_test.go +++ b/internal/inspect/dependencies/renv/available_packages_test.go @@ -6,6 +6,7 @@ import ( "runtime" "testing" + "github.com/posit-dev/publisher/internal/executor" "github.com/posit-dev/publisher/internal/executor/executortest" "github.com/posit-dev/publisher/internal/interpreters" "github.com/posit-dev/publisher/internal/logging" @@ -36,14 +37,21 @@ func (s *AvailablePackagesSuite) SetupTest() { } func (s *AvailablePackagesSuite) TestListAvailablePackages() { - interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { + setupMockRInterpreter := func( + base util.AbsolutePath, + rExecutableParam util.Path, + log logging.Logger, + cmdExecutorOverride executor.Executor, + pathLookerOverride util.PathLooker, + existsFuncOverride interpreters.ExistsFunc, + ) (interpreters.RInterpreter, error) { i := interpreters.NewMockRInterpreter() i.On("Init").Return(nil) i.On("GetRExecutable").Return("R", nil) - return i + return i, nil } - lister, err := NewAvailablePackageLister(s.base, util.Path{}, s.log) + lister, err := NewAvailablePackageLister(s.base, util.Path{}, s.log, setupMockRInterpreter, nil) s.NoError(err) rExecutablePath := util.NewAbsolutePath("R", nil) @@ -71,14 +79,21 @@ BioCbooks https://bioconductor.org/packages/3.18/books ` func (s *AvailablePackagesSuite) TestGetBioconductorRepos() { - interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { + setupMockRInterpreter := func( + base util.AbsolutePath, + rExecutableParam util.Path, + log logging.Logger, + cmdExecutorOverride executor.Executor, + pathLookerOverride util.PathLooker, + existsFuncOverride interpreters.ExistsFunc, + ) (interpreters.RInterpreter, error) { i := interpreters.NewMockRInterpreter() i.On("Init").Return(nil) i.On("GetRExecutable").Return("R", nil) - return i + return i, nil } - lister, _ := NewAvailablePackageLister(s.base, util.Path{}, s.log) + lister, _ := NewAvailablePackageLister(s.base, util.Path{}, s.log, setupMockRInterpreter, nil) rExecutablePath := util.NewAbsolutePath("R", nil) executor := executortest.NewMockExecutor() @@ -105,15 +120,22 @@ func (s *AvailablePackagesSuite) TestGetLibPaths() { s.T().Skip() } - interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { + setupMockRInterpreter := func( + base util.AbsolutePath, + rExecutableParam util.Path, + log logging.Logger, + cmdExecutorOverride executor.Executor, + pathLookerOverride util.PathLooker, + existsFuncOverride interpreters.ExistsFunc, + ) (interpreters.RInterpreter, error) { i := interpreters.NewMockRInterpreter() i.On("Init").Return(nil) i.On("GetRExecutable").Return("R", nil) - return i + return i, nil } rExecutablePath := util.NewAbsolutePath("R", nil) - lister, err := NewAvailablePackageLister(s.base, util.NewPath("R", nil), s.log) + lister, err := NewAvailablePackageLister(s.base, util.NewPath("R", nil), s.log, setupMockRInterpreter, nil) s.NoError(err) executor := executortest.NewMockExecutor() @@ -135,7 +157,7 @@ func (s *AvailablePackagesSuite) TestGetLibPathsWindows() { if runtime.GOOS != "windows" { s.T().Skip() } - lister, err := NewAvailablePackageLister(s.base, util.Path{}, s.log) + lister, err := NewAvailablePackageLister(s.base, util.Path{}, s.log, nil, nil) s.NoError(err) rExecutablePath := util.NewAbsolutePath("R", nil) diff --git a/internal/inspect/dependencies/renv/manifest_packages.go b/internal/inspect/dependencies/renv/manifest_packages.go index 6d9d87ce0..23fecacee 100644 --- a/internal/inspect/dependencies/renv/manifest_packages.go +++ b/internal/inspect/dependencies/renv/manifest_packages.go @@ -27,7 +27,7 @@ type defaultPackageMapper struct { func NewPackageMapper(base util.AbsolutePath, rExecutable util.Path, log logging.Logger) (*defaultPackageMapper, error) { - lister, err := NewAvailablePackageLister(base, rExecutable, log) + lister, err := NewAvailablePackageLister(base, rExecutable, log, nil, nil) return &defaultPackageMapper{ lister: lister, diff --git a/internal/inspect/detectors/all.go b/internal/inspect/detectors/all.go index 7520c2bcd..8656c0e88 100644 --- a/internal/inspect/detectors/all.go +++ b/internal/inspect/detectors/all.go @@ -17,6 +17,8 @@ type ContentTypeDetector struct { detectors []ContentTypeInferer } +type ContentTypeDetectorFactory func(log logging.Logger) *ContentTypeDetector + func NewContentTypeDetector(log logging.Logger) *ContentTypeDetector { return &ContentTypeDetector{ log: log, diff --git a/internal/inspect/python.go b/internal/inspect/python.go index 4994fe9fd..ee5578968 100644 --- a/internal/inspect/python.go +++ b/internal/inspect/python.go @@ -42,6 +42,8 @@ const PythonRequirementsFilename = "requirements.txt" var pythonVersionCache = make(map[string]string) +type PythonInspectorFactory func(base util.AbsolutePath, pythonPath util.Path, log logging.Logger) PythonInspector + func NewPythonInspector(base util.AbsolutePath, pythonPath util.Path, log logging.Logger) PythonInspector { return &defaultPythonInspector{ executor: executor.NewExecutor(), diff --git a/internal/inspect/r.go b/internal/inspect/r.go index a921992a9..b94d78f5f 100644 --- a/internal/inspect/r.go +++ b/internal/inspect/r.go @@ -26,12 +26,20 @@ type defaultRInspector struct { var _ RInspector = &defaultRInspector{} -func NewRInspector(base util.AbsolutePath, rExecutable util.Path, log logging.Logger) (RInspector, error) { +type RInspectorFactory func(base util.AbsolutePath, rExecutable util.Path, log logging.Logger, rInterpreterFactory interpreters.RInterpreterFactory, cmdExecutorOverride executor.Executor) (RInspector, error) - rInterpreter := interpreters.NewRInterpreter(base, rExecutable, log) - // No error returned if there is no R interpreter found. - // That can be expected when retrieving the RExecutable - err := rInterpreter.Init() +func NewRInspector(base util.AbsolutePath, rExecutable util.Path, log logging.Logger, rInterpreterFactoryOverride interpreters.RInterpreterFactory, cmdExecutorOverride executor.Executor) (RInspector, error) { + + var rInterpreter interpreters.RInterpreter + var err error + + if rInterpreterFactoryOverride != nil { + rInterpreter, err = rInterpreterFactoryOverride(base, rExecutable, log, cmdExecutorOverride, nil, nil) + } else { + // No error returned if there is no R interpreter found. + // That can be expected when retrieving the RExecutable + rInterpreter, err = interpreters.NewRInterpreter(base, rExecutable, log, nil, nil, nil) + } return &defaultRInspector{ base: base, diff --git a/internal/inspect/r_test.go b/internal/inspect/r_test.go index a0df77d82..bea3b6758 100644 --- a/internal/inspect/r_test.go +++ b/internal/inspect/r_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/posit-dev/publisher/internal/config" + "github.com/posit-dev/publisher/internal/executor" "github.com/posit-dev/publisher/internal/interpreters" "github.com/posit-dev/publisher/internal/logging" "github.com/posit-dev/publisher/internal/types" @@ -37,15 +38,22 @@ func (s *RSuite) TestNewRInspector() { log := logging.New() rPath := util.NewPath("/usr/bin/R", nil) - interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { + setupMockRInterpreter := func( + base util.AbsolutePath, + rExecutableParam util.Path, + log logging.Logger, + cmdExecutorOverride executor.Executor, + pathLookerOverride util.PathLooker, + existsFuncOverride interpreters.ExistsFunc, + ) (interpreters.RInterpreter, error) { i := interpreters.NewMockRInterpreter() i.On("Init").Return(nil) // i.On("RequiresR", mock.Anything).Return(false, nil) // i.On("GetLockFilePath").Return(util.RelativePath{}, false, nil) - return i + return i, nil } - i, err := NewRInspector(s.cwd, rPath, log) + i, err := NewRInspector(s.cwd, rPath, log, setupMockRInterpreter, nil) s.NoError(err) inspector := i.(*defaultRInspector) @@ -56,17 +64,24 @@ func (s *RSuite) TestNewRInspector() { func (s *RSuite) TestInspectWithRFound() { var relPath util.RelativePath - interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { + setupMockRInterpreter := func( + base util.AbsolutePath, + rExecutableParam util.Path, + log logging.Logger, + cmdExecutorOverride executor.Executor, + pathLookerOverride util.PathLooker, + existsFuncOverride interpreters.ExistsFunc, + ) (interpreters.RInterpreter, error) { i := interpreters.NewMockRInterpreter() i.On("Init").Return(nil) - i.On("GetRExecutable").Return(util.NewAbsolutePath("R", baseDir.Fs()), nil) + i.On("GetRExecutable").Return(util.NewAbsolutePath("R", s.cwd.Fs()), nil) i.On("GetRVersion").Return("1.2.3", nil) - relPath = util.NewRelativePath(baseDir.Join("renv.lock").String(), baseDir.Fs()) + relPath = util.NewRelativePath(s.cwd.Join("renv.lock").String(), s.cwd.Fs()) i.On("GetLockFilePath").Return(relPath, true, nil) - return i + return i, nil } log := logging.New() - i, err := NewRInspector(s.cwd, util.Path{}, log) + i, err := NewRInspector(s.cwd, util.Path{}, log, setupMockRInterpreter, nil) s.NoError(err) inspect, err := i.InspectR() @@ -77,19 +92,26 @@ func (s *RSuite) TestInspectWithRFound() { } func (s *RSuite) TestInspectWithNoRFound() { - interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { + setupMockRInterpreter := func( + base util.AbsolutePath, + rExecutableParam util.Path, + log logging.Logger, + cmdExecutorOverride executor.Executor, + pathLookerOverride util.PathLooker, + existsFuncOverride interpreters.ExistsFunc, + ) (interpreters.RInterpreter, error) { i := interpreters.NewMockRInterpreter() rExecNotFoundError := types.NewAgentError(types.ErrorRExecNotFound, errors.New("info"), nil) i.On("Init").Return(nil) i.On("GetRExecutable").Return(util.AbsolutePath{}, nil) i.On("GetRVersion").Return("", rExecNotFoundError) // i.On("RequiresR", mock.Anything).Return(false, nil) - relPath := util.NewRelativePath(baseDir.Join("renv_2222.lock").String(), baseDir.Fs()) + relPath := util.NewRelativePath(s.cwd.Join("renv_2222.lock").String(), s.cwd.Fs()) i.On("GetLockFilePath").Return(relPath, false, nil) - return i + return i, nil } log := logging.New() - i, err := NewRInspector(s.cwd, util.Path{}, log) + i, err := NewRInspector(s.cwd, util.Path{}, log, setupMockRInterpreter, nil) s.NoError(err) inspect, err := i.InspectR() @@ -100,15 +122,22 @@ func (s *RSuite) TestInspectWithNoRFound() { } func (s *RSuite) TestRequiresRWithEmptyCfgAndLockfileExists() { - interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { + setupMockRInterpreter := func( + base util.AbsolutePath, + rExecutableParam util.Path, + log logging.Logger, + cmdExecutorOverride executor.Executor, + pathLookerOverride util.PathLooker, + existsFuncOverride interpreters.ExistsFunc, + ) (interpreters.RInterpreter, error) { i := interpreters.NewMockRInterpreter() relPath := util.NewRelativePath(s.cwd.Join("renv.lock").String(), s.cwd.Fs()) i.On("GetLockFilePath").Return(relPath, true, nil) - return i + return i, nil } log := logging.New() - i, err := NewRInspector(s.cwd, util.Path{}, log) + i, err := NewRInspector(s.cwd, util.Path{}, log, setupMockRInterpreter, nil) s.NoError(err) cfg := &config.Config{} @@ -119,15 +148,22 @@ func (s *RSuite) TestRequiresRWithEmptyCfgAndLockfileExists() { } func (s *RSuite) TestRequiresRWithEmptyCfgAndLockfileDoesNotExists() { - interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { + setupMockRInterpreter := func( + base util.AbsolutePath, + rExecutableParam util.Path, + log logging.Logger, + cmdExecutorOverride executor.Executor, + pathLookerOverride util.PathLooker, + existsFuncOverride interpreters.ExistsFunc, + ) (interpreters.RInterpreter, error) { i := interpreters.NewMockRInterpreter() relPath := util.NewRelativePath(s.cwd.Join("renv.lock").String(), s.cwd.Fs()) i.On("GetLockFilePath").Return(relPath, false, nil) - return i + return i, nil } log := logging.New() - i, err := NewRInspector(s.cwd, util.Path{}, log) + i, err := NewRInspector(s.cwd, util.Path{}, log, setupMockRInterpreter, nil) s.NoError(err) cfg := &config.Config{} @@ -139,7 +175,7 @@ func (s *RSuite) TestRequiresRWithEmptyCfgAndLockfileDoesNotExists() { func (s *RSuite) TestRequiresRWithRCfg() { log := logging.New() - i, err := NewRInspector(s.cwd, util.Path{}, log) + i, err := NewRInspector(s.cwd, util.Path{}, log, nil, nil) s.NoError(err) cfg := &config.Config{ @@ -152,7 +188,7 @@ func (s *RSuite) TestRequiresRWithRCfg() { func (s *RSuite) TestRequiresRNoRButWithTypeAsPython() { log := logging.New() - i, err := NewRInspector(s.cwd, util.Path{}, log) + i, err := NewRInspector(s.cwd, util.Path{}, log, nil, nil) s.NoError(err) cfg := &config.Config{ @@ -165,7 +201,7 @@ func (s *RSuite) TestRequiresRNoRButWithTypeAsPython() { func (s *RSuite) TestRequiresRNoRButWithTypeEqualContentTypeHTML() { log := logging.New() - i, err := NewRInspector(s.cwd, util.Path{}, log) + i, err := NewRInspector(s.cwd, util.Path{}, log, nil, nil) s.NoError(err) cfg := &config.Config{ diff --git a/internal/interpreters/r.go b/internal/interpreters/r.go index 714d880df..3f2dade5e 100644 --- a/internal/interpreters/r.go +++ b/internal/interpreters/r.go @@ -20,13 +20,16 @@ import ( const DefaultRenvLockfile = "renv.lock" -var NotYetInitialized = types.NewAgentError(types.ErrorRExecNotFound, errors.New("not yet initialized"), nil) var MissingRError = types.NewAgentError(types.ErrorRExecNotFound, errors.New("unable to detect any R interpreters"), nil) -var InvalidR = types.NewAgentError(types.ErrorRExecNotFound, errors.New("r executable is invalid"), nil) -type RInterpreter interface { - Init() error +func newInvalidRError(desc string, err error) *types.AgentError { + errorDesc := fmt.Sprintf("r executable is invalid: %s. Error: %s", desc, err) + return types.NewAgentError(types.ErrorRExecNotFound, errors.New(errorDesc), nil) +} +type ExistsFunc func(p util.Path) (bool, error) + +type RInterpreter interface { GetRExecutable() (util.AbsolutePath, error) GetRVersion() (string, error) GetLockFilePath() (util.RelativePath, bool, error) @@ -34,8 +37,9 @@ type RInterpreter interface { } type defaultRInterpreter struct { - executor executor.Executor - pathLooker util.PathLooker + cmdExecutor executor.Executor + pathLooker util.PathLooker + existsFunc ExistsFunc base util.AbsolutePath preferredPath util.Path @@ -45,20 +49,32 @@ type defaultRInterpreter struct { lockfileExists bool lockfileInitialized bool log logging.Logger - initialized bool fs afero.Fs } -var _ RInterpreter = &defaultRInterpreter{} - -var NewRInterpreter = RInterpreterFactory +type RInterpreterFactory func( + base util.AbsolutePath, + rExecutableParam util.Path, + log logging.Logger, + cmdExecutorOverride executor.Executor, + pathLookerOverride util.PathLooker, + existsFuncOverride ExistsFunc, +) (RInterpreter, error) -func RInterpreterFactory(base util.AbsolutePath, - rExecutableParam util.Path, log logging.Logger) RInterpreter { +var _ RInterpreter = &defaultRInterpreter{} - return &defaultRInterpreter{ - executor: executor.NewExecutor(), - pathLooker: util.NewPathLooker(), +func NewRInterpreter( + base util.AbsolutePath, + rExecutableParam util.Path, + log logging.Logger, + cmdExecutorOverride executor.Executor, + pathLookerOverride util.PathLooker, + existsFuncOverride ExistsFunc, +) (RInterpreter, error) { + interpreter := &defaultRInterpreter{ + cmdExecutor: nil, + pathLooker: nil, + existsFunc: nil, base: base, preferredPath: rExecutableParam, @@ -68,9 +84,31 @@ func RInterpreterFactory(base util.AbsolutePath, lockfileExists: false, lockfileInitialized: false, log: log, - initialized: false, fs: nil, } + if cmdExecutorOverride != nil { + interpreter.cmdExecutor = cmdExecutorOverride + } else { + interpreter.cmdExecutor = executor.NewExecutor() + } + if pathLookerOverride != nil { + interpreter.pathLooker = pathLookerOverride + } else { + interpreter.pathLooker = util.NewPathLooker() + } + if existsFuncOverride != nil { + interpreter.existsFunc = existsFuncOverride + } else { + interpreter.existsFunc = func(p util.Path) (bool, error) { + return p.Exists() + } + } + + err := interpreter.init() + if err != nil { + return nil, err + } + return interpreter, nil } // Initializes the attributes within the defaultRInterpreter structure @@ -88,11 +126,7 @@ func RInterpreterFactory(base util.AbsolutePath, // // Errors are taken care of internally and determine the setting or non-setting // of the attributes. -func (i *defaultRInterpreter) Init() error { - // entire flow doesn't need to occur to be initialized. Just want to be sure - // some of it was attempted to run - i.initialized = true - +func (i *defaultRInterpreter) init() error { // This will set the rExecutable and version for us // Only fatal, unexpected errors will be returned. // We will handle MissingRError internally, as this is a valid environment @@ -109,19 +143,13 @@ func (i *defaultRInterpreter) Init() error { } func (i *defaultRInterpreter) GetRExecutable() (util.AbsolutePath, error) { - if !i.initialized { - return util.AbsolutePath{}, NotYetInitialized - } if i.IsRExecutableValid() { return i.rExecutable, nil } - return util.AbsolutePath{}, MissingRError + return i.rExecutable, MissingRError } func (i *defaultRInterpreter) GetRVersion() (string, error) { - if !i.initialized { - return "", NotYetInitialized - } if i.IsRExecutableValid() { return i.version, nil } @@ -129,9 +157,6 @@ func (i *defaultRInterpreter) GetRVersion() (string, error) { } func (i *defaultRInterpreter) GetLockFilePath() (relativePath util.RelativePath, exists bool, err error) { - if !i.initialized { - return util.RelativePath{}, false, NotYetInitialized - } if !i.lockfileInitialized { // This will set lockfileRelPath and lockfileExists for us // and does not require an R Executable to be available (but it is better if it is) @@ -145,7 +170,7 @@ func (i *defaultRInterpreter) GetLockFilePath() (relativePath util.RelativePath, } func (i *defaultRInterpreter) IsRExecutableValid() bool { - return i.initialized && i.rExecutable.Path.String() != "" && i.version != "" + return i.rExecutable.Path.String() != "" && i.version != "" } // Determine which R Executable to use by: @@ -162,7 +187,7 @@ func (i *defaultRInterpreter) resolveRExecutable() error { // Passed in path to executable if i.preferredPath.String() != "" { // User-provided R executable - exists, err := i.preferredPath.Exists() + exists, err := i.existsFunc(i.preferredPath) if err != nil { i.log.Warn("Unable to check existence of preferred R interpreter. Proceeding with discovery.", "preferredPath", i.preferredPath, "error", err) } else { @@ -186,7 +211,7 @@ func (i *defaultRInterpreter) resolveRExecutable() error { i.log.Debug("Found R executable from PATH", "path", path) tempRPath := util.NewPath(path, i.fs) // make sure it exists - exists, err := tempRPath.Exists() + exists, err := i.existsFunc(tempRPath) if err != nil { i.log.Warn("Unable to check existence of R interpreter on PATH. Proceeding with discovery.", "path", path, "error", err) } else { @@ -232,13 +257,12 @@ func (i *defaultRInterpreter) resolveRExecutable() error { // We assume if we can get a version from the rExecutable passed in, that it // is really an R Executable. func (i *defaultRInterpreter) ValidateRExecutable(rExecutable string) (string, error) { - if !i.initialized { - return "", NotYetInitialized - } version, err := i.getRVersionFromRExecutable(rExecutable) if err != nil { - i.log.Debug("could not run R executable", "rExecutable", rExecutable, "error", err) - return "", err + desc := fmt.Sprintf("could not run R executable. rExecutable: %s", rExecutable) + i.log.Error(desc, "error", err) + aerr := newInvalidRError(desc, err) + return "", aerr } return version, nil } @@ -252,7 +276,7 @@ func (i *defaultRInterpreter) getRVersionFromRExecutable(rExecutable string) (st i.log.Info("Getting R version", "r", rExecutable) args := []string{"--version"} - output, stderr, err := i.executor.RunCommand(rExecutable, args, util.AbsolutePath{}, i.log) + output, stderr, err := i.cmdExecutor.RunCommand(rExecutable, args, util.AbsolutePath{}, i.log) if err != nil { return "", err } @@ -302,7 +326,7 @@ func (i *defaultRInterpreter) resolveRenvLockFile(rExecutable string) error { return err } - lockfileExists, err := lockfilePath.Exists() + lockfileExists, err := i.existsFunc(lockfilePath.Path) if err != nil { i.log.Debug("Error while confirming existence of renv lock file", "renv_lock", lockfilePath, "error", err.Error()) return err @@ -322,7 +346,7 @@ func (i *defaultRInterpreter) getRenvLockfilePathFromRExecutable(rExecutable str i.log.Info("Getting renv lockfile path from R executable", "r", rExecutable) args := []string{"-s", "-e", "renv::paths$lockfile()"} - output, stderr, err := i.executor.RunCommand(rExecutable, args, i.base, i.log) + output, stderr, err := i.cmdExecutor.RunCommand(rExecutable, args, i.base, i.log) if err != nil { if _, ok := err.(*exec.ExitError); ok { i.log.Warn("Couldn't detect lockfile path from R executable (renv::paths$lockfile()); is renv installed?") @@ -350,10 +374,6 @@ func (i *defaultRInterpreter) getRenvLockfilePathFromRExecutable(rExecutable str // CreateLockfile creates a lockfile at the specified path // by invoking R to run `renv::snapshot()`. func (i *defaultRInterpreter) CreateLockfile(lockfilePath util.AbsolutePath) error { - if !i.initialized { - return NotYetInitialized - } - rExecutable, err := i.GetRExecutable() if err != nil { return err @@ -372,7 +392,7 @@ func (i *defaultRInterpreter) CreateLockfile(lockfilePath util.AbsolutePath) err cmd = fmt.Sprintf(`renv::snapshot(lockfile="%s")`, escaped) } args := []string{"-s", "-e", cmd} - stdout, stderr, err := i.executor.RunCommand(rExecutable.String(), args, i.base, i.log) + stdout, stderr, err := i.cmdExecutor.RunCommand(rExecutable.String(), args, i.base, i.log) i.log.Debug("renv::snapshot()", "out", string(stdout), "err", string(stderr)) return err } diff --git a/internal/interpreters/r_test.go b/internal/interpreters/r_test.go index 337f4decb..c31a8beaf 100644 --- a/internal/interpreters/r_test.go +++ b/internal/interpreters/r_test.go @@ -42,7 +42,11 @@ func (s *RSuite) SetupTest() { func (s *RSuite) TestNewRInterpreter() { log := logging.New() rPath := util.NewPath("/usr/bin/R", s.fs) - i := NewRInterpreter(s.cwd, rPath, log) + + pathLooker := util.NewMockPathLooker() + pathLooker.On("LookPath", "R").Return("", nil) + + i, _ := NewRInterpreter(s.cwd, rPath, log, nil, pathLooker, nil) interpreter := i.(*defaultRInterpreter) s.Equal(rPath, interpreter.preferredPath) s.Equal(log, interpreter.log) @@ -50,24 +54,6 @@ func (s *RSuite) TestNewRInterpreter() { s.Equal("", interpreter.version) s.Equal("", interpreter.lockfileRelPath.String()) s.Equal(false, interpreter.lockfileExists) - s.Equal(false, interpreter.initialized) - - // New should return some failures for interface calls - path, err := interpreter.GetRExecutable() - s.Equal("", path.String()) - s.ErrorIs(err, NotYetInitialized) - - version, err := interpreter.GetRVersion() - s.Equal("", version) - s.ErrorIs(err, NotYetInitialized) - - lockFilePath, exists, err := interpreter.GetLockFilePath() - s.Equal("", lockFilePath.String()) - s.Equal(false, exists) - s.ErrorIs(err, NotYetInitialized) - - err = interpreter.CreateLockfile(util.NewAbsolutePath("abc/renv.lock", nil)) - s.ErrorIs(err, NotYetInitialized) } func (s *RSuite) TestInit() { @@ -77,23 +63,19 @@ func (s *RSuite) TestInit() { rPath.Dir().MkdirAll(0777) rPath.WriteFile([]byte(nil), 0777) - i := NewRInterpreter(s.cwd, rPath.Path, log) - interpreter := i.(*defaultRInterpreter) executor := executortest.NewMockExecutor() executor.On("RunCommand", mock.Anything, []string{"--version"}, mock.Anything, mock.Anything).Return([]byte("R version 4.3.0 (2023-04-21)"), nil, nil) executor.On("RunCommand", mock.Anything, []string{"-s", "-e", "renv::paths$lockfile()"}, mock.Anything, mock.Anything).Return([]byte(`[1] "/test/sample-content/shinyapp/renv.lock"`), nil, nil).Once() - interpreter.executor = executor - interpreter.initialized = true - interpreter.fs = s.cwd.Fs() - err := i.Init() + i, err := NewRInterpreter(s.cwd, rPath.Path, log, executor, nil, nil) s.NoError(err) + interpreter := i.(*defaultRInterpreter) + interpreter.fs = s.cwd.Fs() s.Equal(rPath.String(), interpreter.rExecutable.String()) s.Equal("4.3.0", interpreter.version) s.Equal("", interpreter.lockfileRelPath.String()) s.Equal(false, interpreter.lockfileExists) - s.Equal(true, interpreter.initialized) // Now we lazy load the lock file path lockFilePath, exists, err := interpreter.GetLockFilePath() @@ -225,16 +207,19 @@ func (s *RSuite) TestGetRVersionFromExecutable() { rPath.Dir().MkdirAll(0777) rPath.WriteFile(nil, 0777) - rInterpreter := NewRInterpreter(s.cwd, rPath.Path, log) executor := executortest.NewMockExecutor() executor.On("RunCommand", mock.Anything, []string{"--version"}, mock.Anything, mock.Anything).Return([]byte(tc.versionOutput), nil, nil) executor.On("RunCommand", mock.Anything, []string{"-s", "-e", "renv::paths$lockfile()"}, mock.Anything, mock.Anything).Return([]byte(tc.pathsLockfileOutput), nil, nil) - interpreter := rInterpreter.(*defaultRInterpreter) - interpreter.executor = executor - err := rInterpreter.Init() + rInterpreter, err := NewRInterpreter(s.cwd, rPath.Path, log, executor, nil, nil) s.NoError(err) + interpreter := rInterpreter.(*defaultRInterpreter) + interpreter.existsFunc = func(util.Path) (bool, error) { + return true, nil + } + // interpreter.version = "fake" + rExecutable, err := rInterpreter.GetRExecutable() s.NoError(err) s.Equal(true, strings.Contains(rExecutable.String(), rPath.String())) @@ -263,16 +248,16 @@ func (s *RSuite) TestGetRVersionFromExecutableWindows() { rPath := s.cwd.Join("bin", "R") rPath.Dir().MkdirAll(0777) rPath.WriteFile(nil, 0777) - rInterpreter := NewRInterpreter(s.cwd, rPath.Path, log) + executor := executortest.NewMockExecutor() executor.On("RunCommand", mock.Anything, []string{"--version"}, mock.Anything, mock.Anything).Return(nil, []byte(tc.versionOutput), nil) executor.On("RunCommand", mock.Anything, []string{"-s", "-e", "renv::paths$lockfile()"}, mock.Anything, mock.Anything).Return(nil, []byte(tc.pathsLockfileOutput), nil) + rInterpreter, err := NewRInterpreter(s.cwd, rPath.Path, log, executor, nil, nil) + s.NoError(err) + interpreter := rInterpreter.(*defaultRInterpreter) - interpreter.executor = executor interpreter.fs = s.cwd.Fs() - err := rInterpreter.Init() - s.NoError(err) rExecutable, err := rInterpreter.GetRExecutable() s.NoError(err) @@ -315,14 +300,19 @@ func getRExecutableValidTestData(fs afero.Fs) []RExecutableValidTestData { // Make sure the combos don't allow a valid RExecutable to be wrongly reported func (s *RSuite) TestIsRExecutableValid() { log := logging.New() - i := NewRInterpreter(s.cwd, util.Path{}, log) + + // need to add path looker.. should we just allow a callback prior to init? + pathLooker := util.NewMockPathLooker() + pathLooker.On("LookPath", "R").Return("", nil) + + // interpreter.pathLooker = pathLooker + + i, _ := NewRInterpreter(s.cwd, util.Path{}, log, nil, pathLooker, nil) interpreter := i.(*defaultRInterpreter) s.Equal(false, interpreter.IsRExecutableValid()) - interpreter.initialized = true interpreter.rExecutable = util.AbsolutePath{} for _, tc := range getRExecutableValidTestData(s.fs) { - interpreter.initialized = tc.initialized interpreter.rExecutable = tc.rExecutable interpreter.version = tc.version s.Equal(tc.expectedIsRExecutableValidResult, interpreter.IsRExecutableValid()) @@ -332,13 +322,12 @@ func (s *RSuite) TestIsRExecutableValid() { func (s *RSuite) TestResolveRExecutableWhenNotFoundOrInvalid() { log := logging.New() - i := NewRInterpreter(s.cwd, util.Path{}, log) - interpreter := i.(*defaultRInterpreter) executor := executortest.NewMockExecutor() executor.On("RunCommand", mock.Anything, []string{"--version"}, mock.Anything, mock.Anything).Return([]byte("R version 4.3.0 (2023-04-21)"), nil, nil) executor.On("ValidateRExecutable").Return("", errors.New("an error")) - interpreter.executor = executor - interpreter.initialized = true + + i, _ := NewRInterpreter(s.cwd, util.Path{}, log, executor, nil, nil) + interpreter := i.(*defaultRInterpreter) interpreter.fs = s.cwd.Fs() err := interpreter.resolveRExecutable() @@ -353,12 +342,11 @@ func (s *RSuite) TestResolveRExecutableWhenPassedInPathExistsAndIsValid() { rPath.Dir().MkdirAll(0777) rPath.WriteFile([]byte(nil), 0777) - i := NewRInterpreter(s.cwd, rPath.Path, log) - interpreter := i.(*defaultRInterpreter) executor := executortest.NewMockExecutor() executor.On("RunCommand", mock.Anything, []string{"--version"}, mock.Anything, mock.Anything).Return([]byte("R version 4.3.0 (2023-04-21)"), nil, nil) - interpreter.executor = executor - interpreter.initialized = true + + i, _ := NewRInterpreter(s.cwd, rPath.Path, log, executor, nil, nil) + interpreter := i.(*defaultRInterpreter) interpreter.fs = s.cwd.Fs() err := interpreter.resolveRExecutable() @@ -379,13 +367,11 @@ func (s *RSuite) TestResolveRExecutableWhenPassedInPathDoesNotExistButPathValid( pathLooker := util.NewMockPathLooker() pathLooker.On("LookPath", "R").Return(rPath.String(), nil) - i := NewRInterpreter(s.cwd, util.NewPath("/bin/R2", s.cwd.Fs()), log) - interpreter := i.(*defaultRInterpreter) executor := executortest.NewMockExecutor() executor.On("RunCommand", mock.Anything, []string{"--version"}, mock.Anything, mock.Anything).Return([]byte("R version 4.3.0 (2023-04-21)"), nil, nil) - interpreter.executor = executor - interpreter.initialized = true + i, _ := NewRInterpreter(s.cwd, util.NewPath("/bin/R2", s.cwd.Fs()), log, executor, nil, nil) + interpreter := i.(*defaultRInterpreter) interpreter.pathLooker = pathLooker interpreter.fs = s.cwd.Fs() @@ -408,13 +394,12 @@ func (s *RSuite) TestResolveRExecutableWhenPassedInPathExistsButNotValid() { rPath.Dir().MkdirAll(0777) rPath.WriteFile(nil, 0777) - i := NewRInterpreter(s.cwd, util.NewPath(rPath.String(), s.cwd.Fs()), log) - interpreter := i.(*defaultRInterpreter) executor := executortest.NewMockExecutor() executor.On("RunCommand", mock.Anything, []string{"--version"}, mock.Anything, mock.Anything).Return([]byte("bad command"), nil, nil) - interpreter.executor = executor - interpreter.initialized = true + i, _ := NewRInterpreter(s.cwd, util.NewPath(rPath.String(), s.cwd.Fs()), log, executor, nil, nil) + interpreter := i.(*defaultRInterpreter) + interpreter.pathLooker = pathLooker interpreter.fs = s.cwd.Fs() @@ -438,13 +423,13 @@ func (s *RSuite) TestResolveRExecutableWhenPathContainsRButNotValid() { rPath.Dir().MkdirAll(0777) rPath.WriteFile([]byte(nil), 0777) - i := NewRInterpreter(s.cwd, util.Path{}, log) - interpreter := i.(*defaultRInterpreter) - interpreter.pathLooker = pathLooker - interpreter.initialized = true executor := executortest.NewMockExecutor() executor.On("RunCommand", mock.Anything, []string{"--version"}, mock.Anything, mock.Anything).Return([]byte("Invalid stuff"), nil, nil) - interpreter.executor = executor + + i, _ := NewRInterpreter(s.cwd, util.Path{}, log, executor, nil, nil) + interpreter := i.(*defaultRInterpreter) + interpreter.pathLooker = pathLooker + interpreter.fs = s.cwd.Fs() err := interpreter.resolveRExecutable() @@ -456,12 +441,12 @@ func (s *RSuite) TestResolveRExecutableWhenPathContainsRButNotValid() { func (s *RSuite) TestGetRVersionFromRExecutableWithInvalidR() { log := logging.New() - i := NewRInterpreter(s.cwd, util.Path{}, log) - interpreter := i.(*defaultRInterpreter) - interpreter.initialized = true + executor := executortest.NewMockExecutor() executor.On("RunCommand", mock.Anything, []string{"--version"}, mock.Anything, mock.Anything).Return([]byte(""), nil, errors.New("problem")) - interpreter.executor = executor + + i, _ := NewRInterpreter(s.cwd, util.Path{}, log, executor, nil, nil) + interpreter := i.(*defaultRInterpreter) interpreter.fs = s.cwd.Fs() _, err := interpreter.getRVersionFromRExecutable("does-not-matter") @@ -473,12 +458,12 @@ func (s *RSuite) TestGetRVersionFromRExecutableWithInvalidR() { func (s *RSuite) TestResolveRenvLockFileWithInvalidR() { log := logging.New() - i := NewRInterpreter(s.cwd, util.Path{}, log) - interpreter := i.(*defaultRInterpreter) - interpreter.initialized = true + executor := executortest.NewMockExecutor() executor.On("RunCommand", mock.Anything, []string{"--version"}, mock.Anything, mock.Anything).Return([]byte(""), nil, errors.New("problem")) - interpreter.executor = executor + + i, _ := NewRInterpreter(s.cwd, util.Path{}, log, executor, nil, nil) + interpreter := i.(*defaultRInterpreter) interpreter.fs = s.cwd.Fs() err := interpreter.resolveRenvLockFile("does-not-matter") @@ -497,14 +482,14 @@ func (s *RSuite) TestResolveRenvLockFileWithRSpecifyingDefaultNameAndExists() { rPath.Dir().MkdirAll(0777) rPath.WriteFile(nil, 0777) - i := NewRInterpreter(s.cwd, util.Path{}, log) - interpreter := i.(*defaultRInterpreter) - interpreter.initialized = true - interpreter.fs = s.cwd.Fs() executor := executortest.NewMockExecutor() outputLine := fmt.Sprintf(`[1] "%s"`, rPath.String()) executor.On("RunCommand", mock.Anything, []string{"-s", "-e", "renv::paths$lockfile()"}, mock.Anything, mock.Anything).Return([]byte(outputLine), nil, nil) - interpreter.executor = executor + executor.On("RunCommand", mock.Anything, []string{"--version"}, mock.Anything, mock.Anything).Return([]byte(""), nil, errors.New("problem")) + + i, _ := NewRInterpreter(s.cwd, util.Path{}, log, executor, nil, nil) + interpreter := i.(*defaultRInterpreter) + interpreter.fs = s.cwd.Fs() err := interpreter.resolveRenvLockFile("does_not_matter") s.NoError(err) @@ -521,12 +506,12 @@ func (s *RSuite) TestResolveRenvLockFileWithRSpecifyingDefaultNameAndDoesNotExis s.T().Skip("This test does not run on Windows") } - i := NewRInterpreter(s.cwd, util.Path{}, log) - interpreter := i.(*defaultRInterpreter) - interpreter.initialized = true executor := executortest.NewMockExecutor() executor.On("RunCommand", mock.Anything, []string{"-s", "-e", "renv::paths$lockfile()"}, mock.Anything, mock.Anything).Return([]byte(`[1] "internal/interpreters/renv.lock"`), nil, nil) - interpreter.executor = executor + executor.On("RunCommand", mock.Anything, []string{"--version"}, mock.Anything, mock.Anything).Return([]byte(""), nil, errors.New("problem")) + + i, _ := NewRInterpreter(s.cwd, util.Path{}, log, executor, nil, nil) + interpreter := i.(*defaultRInterpreter) interpreter.fs = s.cwd.Fs() err := interpreter.resolveRenvLockFile("does_not_matter") @@ -547,14 +532,15 @@ func (s *RSuite) TestResolveRenvLockFileWithRSpecialNameAndExists() { rPath.Dir().MkdirAll(0777) rPath.WriteFile([]byte(nil), 0777) - i := NewRInterpreter(s.cwd, util.Path{}, log) - interpreter := i.(*defaultRInterpreter) executor := executortest.NewMockExecutor() outputLine := fmt.Sprintf(`[1] "%s"`, rPath.String()) executor.On("RunCommand", mock.Anything, []string{"-s", "-e", "renv::paths$lockfile()"}, mock.Anything, mock.Anything).Return([]byte(outputLine), nil, nil) - interpreter.executor = executor + executor.On("RunCommand", mock.Anything, []string{"--version"}, mock.Anything, mock.Anything).Return([]byte(""), nil, errors.New("problem")) + + i, _ := NewRInterpreter(s.cwd, util.Path{}, log, executor, nil, nil) + interpreter := i.(*defaultRInterpreter) + interpreter.fs = s.cwd.Fs() - interpreter.initialized = true interpreter.rExecutable = util.NewAbsolutePath("does_not_matter/R", interpreter.fs) interpreter.version = "does_not_matter" @@ -566,10 +552,10 @@ func (s *RSuite) TestResolveRenvLockFileWithRSpecialNameAndExists() { func (s *RSuite) TestCreateLockfileWithInvalidR() { log := logging.New() - i := NewRInterpreter(s.cwd, util.Path{}, log) + i, _ := NewRInterpreter(s.cwd, util.Path{}, log, nil, nil, nil) interpreter := i.(*defaultRInterpreter) - interpreter.initialized = true interpreter.fs = s.cwd.Fs() + interpreter.rExecutable = util.AbsolutePath{} err := interpreter.CreateLockfile(util.NewAbsolutePath("abc/xxy/renv.lock", s.cwd.Fs())) s.Error(err) @@ -579,14 +565,15 @@ func (s *RSuite) TestCreateLockfileWithInvalidR() { func (s *RSuite) TestCreateLockfileWithNonEmptyPath() { log := logging.New() - i := NewRInterpreter(s.cwd, util.Path{}, log) + + executor := executortest.NewMockExecutor() + executor.On("RunCommand", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]byte("success"), nil, nil) + executor.On("RunCommand", mock.Anything, []string{"--version"}, mock.Anything, mock.Anything).Return([]byte(""), nil, errors.New("problem")) + + i, _ := NewRInterpreter(s.cwd, util.Path{}, log, executor, nil, nil) interpreter := i.(*defaultRInterpreter) - interpreter.initialized = true interpreter.rExecutable = util.NewAbsolutePath("/usr/bin/R", s.cwd.Fs()) interpreter.version = "1.2.3" - executor := executortest.NewMockExecutor() - executor.On("RunCommand", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]byte("success"), nil, nil) - interpreter.executor = executor interpreter.fs = s.cwd.Fs() err := i.CreateLockfile(util.NewAbsolutePath("abc/xxy/renv.lock", s.cwd.Fs())) @@ -595,14 +582,15 @@ func (s *RSuite) TestCreateLockfileWithNonEmptyPath() { func (s *RSuite) TestCreateLockfileWithEmptyPath() { log := logging.New() - i := NewRInterpreter(s.cwd, util.Path{}, log) + + executor := executortest.NewMockExecutor() + executor.On("RunCommand", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]byte("success"), nil, nil) + executor.On("RunCommand", mock.Anything, []string{"--version"}, mock.Anything, mock.Anything).Return([]byte(""), nil, errors.New("problem")) + + i, _ := NewRInterpreter(s.cwd, util.Path{}, log, executor, nil, nil) interpreter := i.(*defaultRInterpreter) - interpreter.initialized = true interpreter.rExecutable = util.NewAbsolutePath("/usr/bin/R", s.cwd.Fs()) interpreter.version = "1.2.3" - executor := executortest.NewMockExecutor() - executor.On("RunCommand", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]byte("success"), nil, nil) - interpreter.executor = executor interpreter.fs = s.cwd.Fs() err := i.CreateLockfile(util.AbsolutePath{}) diff --git a/internal/publish/publish_test.go b/internal/publish/publish_test.go index e3c1bfd69..a1ff58e87 100644 --- a/internal/publish/publish_test.go +++ b/internal/publish/publish_test.go @@ -16,7 +16,6 @@ import ( "github.com/posit-dev/publisher/internal/deployment" "github.com/posit-dev/publisher/internal/events" "github.com/posit-dev/publisher/internal/inspect/dependencies/renv" - "github.com/posit-dev/publisher/internal/interpreters" "github.com/posit-dev/publisher/internal/logging" "github.com/posit-dev/publisher/internal/logging/loggingtest" "github.com/posit-dev/publisher/internal/project" @@ -112,14 +111,13 @@ func (s *PublishSuite) SetupTest() { cwd.Join("requirements.txt").WriteFile([]byte("flask\n"), 0600) cwd.Join("renv.lock").WriteFile([]byte(renvLockContent), 0600) - interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { - i := interpreters.NewMockRInterpreter() - i.On("Init").Return(nil) - i.On("RequiresR", mock.Anything).Return(false, nil) - i.On("GetLockFilePath").Return(util.RelativePath{}, false, nil) - return i - } - + // setupMockRInterpreter := func() (interpreters.RInterpreter, error) { + // i := interpreters.NewMockRInterpreter() + // i.On("Init").Return(nil) + // i.On("RequiresR", mock.Anything).Return(false, nil) + // i.On("GetLockFilePath").Return(util.RelativePath{}, false, nil) + // return i, nil + // } } func (s *PublishSuite) TestNewFromState() { diff --git a/internal/services/api/api_service.go b/internal/services/api/api_service.go index 752dd658a..f0a362a73 100644 --- a/internal/services/api/api_service.go +++ b/internal/services/api/api_service.go @@ -175,7 +175,7 @@ func RouterHandlerFunc(base util.AbsolutePath, lister accounts.AccountList, log Methods(http.MethodPost) // POST /api/packages/r/scan - r.Handle(ToPath("packages", "r", "scan"), NewPostPackagesRScanHandler(base, log)). + r.Handle(ToPath("packages", "r", "scan"), NewPostPackagesRScanHandler(base, log, nil)). Methods(http.MethodPost) c := cors.AllowAll().Handler(r) diff --git a/internal/services/api/post_inspect.go b/internal/services/api/post_inspect.go index 9fa6f0690..ab01a3659 100644 --- a/internal/services/api/post_inspect.go +++ b/internal/services/api/post_inspect.go @@ -112,7 +112,9 @@ func PostInspectHandlerFunc(base util.AbsolutePath, log logging.Logger) http.Han } entrypoint := req.URL.Query().Get("entrypoint") entrypointPath := util.NewRelativePath(entrypoint, base.Fs()) - configs, err := initialize.GetPossibleConfigs(path, pythonPath, rPath, entrypointPath, log) + + i := initialize.NewDefaultInitialize() + configs, err := i.GetPossibleConfigs(path, pythonPath, rPath, entrypointPath, log) if err != nil { return err } @@ -150,7 +152,9 @@ func PostInspectHandlerFunc(base util.AbsolutePath, log logging.Logger) http.Han // Response already returned by getEntrypointPath return } - configs, err := initialize.GetPossibleConfigs(projectDir, pythonPath, rPath, entrypointPath, log) + + i := initialize.NewDefaultInitialize() + configs, err := i.GetPossibleConfigs(projectDir, pythonPath, rPath, entrypointPath, log) if err != nil { if aerr, ok := types.IsAgentErrorOf(err, types.ErrorPythonExecNotFound); ok { apiErr := types.APIErrorPythonExecNotFoundFromAgentError(*aerr) diff --git a/internal/services/api/post_packages_r_scan.go b/internal/services/api/post_packages_r_scan.go index 3474aa806..445979e6f 100644 --- a/internal/services/api/post_packages_r_scan.go +++ b/internal/services/api/post_packages_r_scan.go @@ -20,14 +20,16 @@ type PostPackagesRScanRequest struct { } type PostPackagesRScanHandler struct { - base util.AbsolutePath - log logging.Logger + base util.AbsolutePath + log logging.Logger + rInterpreterFactory interpreters.RInterpreterFactory } -func NewPostPackagesRScanHandler(base util.AbsolutePath, log logging.Logger) *PostPackagesRScanHandler { +func NewPostPackagesRScanHandler(base util.AbsolutePath, log logging.Logger, rInterpreterFactory interpreters.RInterpreterFactory) *PostPackagesRScanHandler { return &PostPackagesRScanHandler{ - base: base, - log: log, + base: base, + log: log, + rInterpreterFactory: rInterpreterFactory, } } @@ -59,8 +61,7 @@ func (h *PostPackagesRScanHandler) ServeHTTP(w http.ResponseWriter, req *http.Re lockfileAbsPath := projectDir.Join(path.String()) rPath := util.NewPath(b.R, nil) - rInterpreter := interpreters.NewRInterpreter(projectDir, rPath, h.log) - err = rInterpreter.Init() + rInterpreter, err := h.rInterpreterFactory(projectDir, rPath, h.log, nil, nil, nil) if err != nil { InternalError(w, req, h.log, err) return diff --git a/internal/services/api/post_packages_r_scan_test.go b/internal/services/api/post_packages_r_scan_test.go index 84399ad5f..a938a8035 100644 --- a/internal/services/api/post_packages_r_scan_test.go +++ b/internal/services/api/post_packages_r_scan_test.go @@ -10,6 +10,7 @@ import ( "strings" "testing" + "github.com/posit-dev/publisher/internal/executor" "github.com/posit-dev/publisher/internal/interpreters" "github.com/posit-dev/publisher/internal/logging" "github.com/posit-dev/publisher/internal/util" @@ -33,7 +34,7 @@ func (s *PostPackagesRScanSuite) SetupTest() { func (s *PostPackagesRScanSuite) TestNewPostPackagesRScanHandler() { base := util.NewAbsolutePath("/project", nil) log := logging.New() - h := NewPostPackagesRScanHandler(base, log) + h := NewPostPackagesRScanHandler(base, log, nil) s.Equal(base, h.base) s.Equal(log, h.log) } @@ -52,17 +53,23 @@ func (s *PostPackagesRScanSuite) TestServeHTTP() { lockFilePath := base.Join("renv.lock") log := logging.New() - h := NewPostPackagesRScanHandler(base, log) - interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { - s.Equal(base, baseDir) - s.Equal(util.NewPath("/opt/R/bin/R", nil), rExec) + setupMockRInterpreter := func( + base util.AbsolutePath, + rExecutableParam util.Path, + log logging.Logger, + cmdExecutorOverride executor.Executor, + pathLookerOverride util.PathLooker, + existsFuncOverride interpreters.ExistsFunc, + ) (interpreters.RInterpreter, error) { i := interpreters.NewMockRInterpreter() i.On("Init").Return(nil) i.On("CreateLockfile", lockFilePath).Return(nil) - return i + return i, nil } + h := NewPostPackagesRScanHandler(base, log, setupMockRInterpreter) + h.ServeHTTP(rec, req) s.Equal(http.StatusNoContent, rec.Result().StatusCode) } @@ -78,14 +85,22 @@ func (s *PostPackagesRScanSuite) TestServeHTTPEmptyBody() { s.NoError(err) log := logging.New() - h := NewPostPackagesRScanHandler(base, log) - interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { + setupMockRInterpreter := func( + base util.AbsolutePath, + rExecutableParam util.Path, + log logging.Logger, + cmdExecutorOverride executor.Executor, + pathLookerOverride util.PathLooker, + existsFuncOverride interpreters.ExistsFunc, + ) (interpreters.RInterpreter, error) { i := interpreters.NewMockRInterpreter() i.On("CreateLockfile", mock.Anything).Return(nil) - return i + return i, nil } + h := NewPostPackagesRScanHandler(base, log, setupMockRInterpreter) + h.ServeHTTP(rec, req) s.Equal(http.StatusNoContent, rec.Result().StatusCode) } @@ -104,13 +119,20 @@ func (s *PostPackagesRScanSuite) TestServeHTTPWithSaveName() { log := logging.New() - interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { + setupMockRInterpreter := func( + base util.AbsolutePath, + rExecutableParam util.Path, + log logging.Logger, + cmdExecutorOverride executor.Executor, + pathLookerOverride util.PathLooker, + existsFuncOverride interpreters.ExistsFunc, + ) (interpreters.RInterpreter, error) { i := interpreters.NewMockRInterpreter() i.On("CreateLockfile", util.NewAbsolutePath(destPath.String(), fs)).Return(nil) - return i + return i, nil } - h := NewPostPackagesRScanHandler(base, log) + h := NewPostPackagesRScanHandler(base, log, setupMockRInterpreter) h.ServeHTTP(rec, req) s.Equal(http.StatusNoContent, rec.Result().StatusCode) @@ -129,14 +151,22 @@ func (s *PostPackagesRScanSuite) TestServeHTTPWithSaveNameInSubdir() { destPath := base.Join(".renv", "profiles", "staging", "renv.lock") log := logging.New() - h := NewPostPackagesRScanHandler(base, log) - interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { + setupMockRInterpreter := func( + base util.AbsolutePath, + rExecutableParam util.Path, + log logging.Logger, + cmdExecutorOverride executor.Executor, + pathLookerOverride util.PathLooker, + existsFuncOverride interpreters.ExistsFunc, + ) (interpreters.RInterpreter, error) { i := interpreters.NewMockRInterpreter() i.On("CreateLockfile", destPath).Return(nil) - return i + return i, nil } + h := NewPostPackagesRScanHandler(base, log, setupMockRInterpreter) + h.ServeHTTP(rec, req) s.Equal(http.StatusNoContent, rec.Result().StatusCode) } @@ -152,16 +182,24 @@ func (s *PostPackagesRScanSuite) TestServeHTTPErr() { s.NoError(err) destPath := base.Join("renv.lock") log := logging.New() - h := NewPostPackagesRScanHandler(base, log) testError := errors.New("test error from ScanRequirements") - interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { + setupMockRInterpreter := func( + base util.AbsolutePath, + rExecutableParam util.Path, + log logging.Logger, + cmdExecutorOverride executor.Executor, + pathLookerOverride util.PathLooker, + existsFuncOverride interpreters.ExistsFunc, + ) (interpreters.RInterpreter, error) { i := interpreters.NewMockRInterpreter() i.On("Init").Return(nil) i.On("CreateLockfile", destPath).Return(testError) - return i + return i, nil } + h := NewPostPackagesRScanHandler(base, log, setupMockRInterpreter) + h.ServeHTTP(rec, req) s.Equal(http.StatusInternalServerError, rec.Result().StatusCode) } @@ -184,16 +222,22 @@ func (s *PostPackagesRScanSuite) TestServeHTTPSubdir() { req, err := http.NewRequest("POST", "/api/packages/r/scan?dir="+dirParam, body) s.NoError(err) - h := NewPostPackagesRScanHandler(base, logging.New()) - - interpreters.NewRInterpreter = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) interpreters.RInterpreter { - s.Equal(projectDir, baseDir) + setupMockRInterpreter := func( + base util.AbsolutePath, + rExecutableParam util.Path, + log logging.Logger, + cmdExecutorOverride executor.Executor, + pathLookerOverride util.PathLooker, + existsFuncOverride interpreters.ExistsFunc, + ) (interpreters.RInterpreter, error) { i := interpreters.NewMockRInterpreter() i.On("Init").Return(nil) i.On("CreateLockfile", destPath).Return(nil) - return i + return i, nil } + h := NewPostPackagesRScanHandler(base, logging.New(), setupMockRInterpreter) + h.ServeHTTP(rec, req) s.Equal(http.StatusNoContent, rec.Result().StatusCode) } From 43895209dbdac0884892254501126d633eec59c6 Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Thu, 19 Dec 2024 22:48:29 -0800 Subject: [PATCH 18/28] remove unneeded comments --- internal/inspect/r_test.go | 3 --- internal/interpreters/r_test.go | 1 - 2 files changed, 4 deletions(-) diff --git a/internal/inspect/r_test.go b/internal/inspect/r_test.go index bea3b6758..de8abd53d 100644 --- a/internal/inspect/r_test.go +++ b/internal/inspect/r_test.go @@ -48,8 +48,6 @@ func (s *RSuite) TestNewRInspector() { ) (interpreters.RInterpreter, error) { i := interpreters.NewMockRInterpreter() i.On("Init").Return(nil) - // i.On("RequiresR", mock.Anything).Return(false, nil) - // i.On("GetLockFilePath").Return(util.RelativePath{}, false, nil) return i, nil } @@ -105,7 +103,6 @@ func (s *RSuite) TestInspectWithNoRFound() { i.On("Init").Return(nil) i.On("GetRExecutable").Return(util.AbsolutePath{}, nil) i.On("GetRVersion").Return("", rExecNotFoundError) - // i.On("RequiresR", mock.Anything).Return(false, nil) relPath := util.NewRelativePath(s.cwd.Join("renv_2222.lock").String(), s.cwd.Fs()) i.On("GetLockFilePath").Return(relPath, false, nil) return i, nil diff --git a/internal/interpreters/r_test.go b/internal/interpreters/r_test.go index c31a8beaf..f33d4c507 100644 --- a/internal/interpreters/r_test.go +++ b/internal/interpreters/r_test.go @@ -218,7 +218,6 @@ func (s *RSuite) TestGetRVersionFromExecutable() { interpreter.existsFunc = func(util.Path) (bool, error) { return true, nil } - // interpreter.version = "fake" rExecutable, err := rInterpreter.GetRExecutable() s.NoError(err) From cddc15653ecaf933747207fd1aafe1b2b28b7a5f Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Thu, 19 Dec 2024 22:55:57 -0800 Subject: [PATCH 19/28] remove unneeded comments --- internal/publish/publish_test.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/internal/publish/publish_test.go b/internal/publish/publish_test.go index a1ff58e87..840282fc9 100644 --- a/internal/publish/publish_test.go +++ b/internal/publish/publish_test.go @@ -110,14 +110,6 @@ func (s *PublishSuite) SetupTest() { cwd.Join("app.py").WriteFile([]byte("import flask\n"), 0600) cwd.Join("requirements.txt").WriteFile([]byte("flask\n"), 0600) cwd.Join("renv.lock").WriteFile([]byte(renvLockContent), 0600) - - // setupMockRInterpreter := func() (interpreters.RInterpreter, error) { - // i := interpreters.NewMockRInterpreter() - // i.On("Init").Return(nil) - // i.On("RequiresR", mock.Anything).Return(false, nil) - // i.On("GetLockFilePath").Return(util.RelativePath{}, false, nil) - // return i, nil - // } } func (s *PublishSuite) TestNewFromState() { From 7775a57c20020cb98c25b82abbee190de101e53e Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Fri, 20 Dec 2024 09:05:22 -0800 Subject: [PATCH 20/28] fix unit test missing mock call --- internal/inspect/dependencies/renv/available_packages_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/inspect/dependencies/renv/available_packages_test.go b/internal/inspect/dependencies/renv/available_packages_test.go index 69ab136c8..381dc52f1 100644 --- a/internal/inspect/dependencies/renv/available_packages_test.go +++ b/internal/inspect/dependencies/renv/available_packages_test.go @@ -140,6 +140,7 @@ func (s *AvailablePackagesSuite) TestGetLibPaths() { executor := executortest.NewMockExecutor() executor.On("RunCommand", rExecutablePath.String(), mock.Anything, s.base, mock.Anything).Return([]byte(libPathsOutput), []byte{}, nil) + executor.On("RunCommand", mock.Anything, []string{"-s", "-e", "renv::paths$lockfile()"}, mock.Anything, mock.Anything).Return([]byte(`[1] "/test/sample-content/shinyapp/renv.lock"`), nil, nil).Once() lister.rExecutor = executor repos, err := lister.GetLibPaths(logging.New()) @@ -164,6 +165,7 @@ func (s *AvailablePackagesSuite) TestGetLibPathsWindows() { executor := executortest.NewMockExecutor() executor.On("RunCommand", rExecutablePath.String(), mock.Anything, s.base, mock.Anything).Return([]byte(windowsLibPathsOutput), []byte{}, nil) + executor.On("RunCommand", mock.Anything, []string{"-s", "-e", "renv::paths$lockfile()"}, mock.Anything, mock.Anything).Return([]byte(`[1] "\\test\\sample-content\\shinyapp\\renv.lock"`), nil, nil).Once() lister.rExecutor = executor repos, err := lister.GetLibPaths(logging.New()) From 12947dad64c6d2724f89d8a3bb06803edb3e1869 Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Fri, 20 Dec 2024 09:22:33 -0800 Subject: [PATCH 21/28] fix unit test missing mock call --- internal/inspect/dependencies/renv/available_packages_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/internal/inspect/dependencies/renv/available_packages_test.go b/internal/inspect/dependencies/renv/available_packages_test.go index 381dc52f1..43d26a459 100644 --- a/internal/inspect/dependencies/renv/available_packages_test.go +++ b/internal/inspect/dependencies/renv/available_packages_test.go @@ -140,7 +140,6 @@ func (s *AvailablePackagesSuite) TestGetLibPaths() { executor := executortest.NewMockExecutor() executor.On("RunCommand", rExecutablePath.String(), mock.Anything, s.base, mock.Anything).Return([]byte(libPathsOutput), []byte{}, nil) - executor.On("RunCommand", mock.Anything, []string{"-s", "-e", "renv::paths$lockfile()"}, mock.Anything, mock.Anything).Return([]byte(`[1] "/test/sample-content/shinyapp/renv.lock"`), nil, nil).Once() lister.rExecutor = executor repos, err := lister.GetLibPaths(logging.New()) @@ -164,8 +163,7 @@ func (s *AvailablePackagesSuite) TestGetLibPathsWindows() { rExecutablePath := util.NewAbsolutePath("R", nil) executor := executortest.NewMockExecutor() - executor.On("RunCommand", rExecutablePath.String(), mock.Anything, s.base, mock.Anything).Return([]byte(windowsLibPathsOutput), []byte{}, nil) - executor.On("RunCommand", mock.Anything, []string{"-s", "-e", "renv::paths$lockfile()"}, mock.Anything, mock.Anything).Return([]byte(`[1] "\\test\\sample-content\\shinyapp\\renv.lock"`), nil, nil).Once() + executor.On("RunCommand", rExecutablePath, []string{"-s", "-e", "cat(.libPaths(), sep=\"\\n\")"}, s.base, mock.Anything).Return([]byte(windowsLibPathsOutput), []byte{}, nil) lister.rExecutor = executor repos, err := lister.GetLibPaths(logging.New()) From 89643221c6a1f93b7de0c462ae24fd7f0c0e5b1b Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Fri, 20 Dec 2024 09:27:16 -0800 Subject: [PATCH 22/28] fix unit test missing mock call --- .../renv/available_packages_test.go | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/internal/inspect/dependencies/renv/available_packages_test.go b/internal/inspect/dependencies/renv/available_packages_test.go index 43d26a459..a39644a9a 100644 --- a/internal/inspect/dependencies/renv/available_packages_test.go +++ b/internal/inspect/dependencies/renv/available_packages_test.go @@ -157,11 +157,25 @@ func (s *AvailablePackagesSuite) TestGetLibPathsWindows() { if runtime.GOOS != "windows" { s.T().Skip() } - lister, err := NewAvailablePackageLister(s.base, util.Path{}, s.log, nil, nil) - s.NoError(err) + setupMockRInterpreter := func( + base util.AbsolutePath, + rExecutableParam util.Path, + log logging.Logger, + cmdExecutorOverride executor.Executor, + pathLookerOverride util.PathLooker, + existsFuncOverride interpreters.ExistsFunc, + ) (interpreters.RInterpreter, error) { + i := interpreters.NewMockRInterpreter() + i.On("Init").Return(nil) + i.On("GetRExecutable").Return("R", nil) + return i, nil + } rExecutablePath := util.NewAbsolutePath("R", nil) + lister, err := NewAvailablePackageLister(s.base, util.NewPath("R", nil), s.log, setupMockRInterpreter, nil) + s.NoError(err) + executor := executortest.NewMockExecutor() executor.On("RunCommand", rExecutablePath, []string{"-s", "-e", "cat(.libPaths(), sep=\"\\n\")"}, s.base, mock.Anything).Return([]byte(windowsLibPathsOutput), []byte{}, nil) lister.rExecutor = executor From 5ee4fa01efb7afda61654997b0aeb63cfd94b5d4 Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Fri, 20 Dec 2024 09:29:51 -0800 Subject: [PATCH 23/28] fix unit test missing mock call --- internal/inspect/dependencies/renv/available_packages_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/inspect/dependencies/renv/available_packages_test.go b/internal/inspect/dependencies/renv/available_packages_test.go index a39644a9a..6162e834b 100644 --- a/internal/inspect/dependencies/renv/available_packages_test.go +++ b/internal/inspect/dependencies/renv/available_packages_test.go @@ -177,7 +177,7 @@ func (s *AvailablePackagesSuite) TestGetLibPathsWindows() { s.NoError(err) executor := executortest.NewMockExecutor() - executor.On("RunCommand", rExecutablePath, []string{"-s", "-e", "cat(.libPaths(), sep=\"\\n\")"}, s.base, mock.Anything).Return([]byte(windowsLibPathsOutput), []byte{}, nil) + executor.On("RunCommand", rExecutablePath.String(), []string{"-s", "-e", "cat(.libPaths(), sep=\"\\n\")"}, s.base, mock.Anything).Return([]byte(windowsLibPathsOutput), []byte{}, nil) lister.rExecutor = executor repos, err := lister.GetLibPaths(logging.New()) From 33a48868430a32f68683ae4544cc2ac89709cf8f Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Fri, 20 Dec 2024 13:58:34 -0800 Subject: [PATCH 24/28] debugging occasional vscode-ui test failure in CI --- test/vscode-ui/test/specs/nested-fastapi-deployment.spec.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/vscode-ui/test/specs/nested-fastapi-deployment.spec.ts b/test/vscode-ui/test/specs/nested-fastapi-deployment.spec.ts index c39da2002..624718789 100644 --- a/test/vscode-ui/test/specs/nested-fastapi-deployment.spec.ts +++ b/test/vscode-ui/test/specs/nested-fastapi-deployment.spec.ts @@ -57,6 +57,9 @@ describe("Nested Fast API Deployment", () => { ); await expect(myConfig).toExist(); myConfig.click(); + + // wait for the click to be done. + await expect(myConfig).not.toExist(); }); it("can edit config", async () => { From 0feddffec1c5880d7a9d1693765140f3f8df3654 Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Fri, 20 Dec 2024 14:21:54 -0800 Subject: [PATCH 25/28] debugging occasional vscode-ui test failure in CI --- test/vscode-ui/test/specs/nested-fastapi-deployment.spec.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/vscode-ui/test/specs/nested-fastapi-deployment.spec.ts b/test/vscode-ui/test/specs/nested-fastapi-deployment.spec.ts index 624718789..9ee096b46 100644 --- a/test/vscode-ui/test/specs/nested-fastapi-deployment.spec.ts +++ b/test/vscode-ui/test/specs/nested-fastapi-deployment.spec.ts @@ -58,8 +58,9 @@ describe("Nested Fast API Deployment", () => { await expect(myConfig).toExist(); myConfig.click(); - // wait for the click to be done. - await expect(myConfig).not.toExist(); + // wait for selection to be complete + const deployment = await $(".quick-pick-label"); + await expect(deployment).toHaveText(title); }); it("can edit config", async () => { From a28a3d15cf156602056db21a8f03ffbe717d2bb1 Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Fri, 20 Dec 2024 14:41:41 -0800 Subject: [PATCH 26/28] debugging occasional vscode-ui test failure in CI --- test/vscode-ui/test/specs/nested-fastapi-deployment.spec.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/test/vscode-ui/test/specs/nested-fastapi-deployment.spec.ts b/test/vscode-ui/test/specs/nested-fastapi-deployment.spec.ts index 9ee096b46..5475fc278 100644 --- a/test/vscode-ui/test/specs/nested-fastapi-deployment.spec.ts +++ b/test/vscode-ui/test/specs/nested-fastapi-deployment.spec.ts @@ -59,6 +59,7 @@ describe("Nested Fast API Deployment", () => { myConfig.click(); // wait for selection to be complete + await helper.switchToSubframe(); const deployment = await $(".quick-pick-label"); await expect(deployment).toHaveText(title); }); From ed931fbc35f104b1151b336e6e928ecde7c13800 Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Fri, 20 Dec 2024 15:06:01 -0800 Subject: [PATCH 27/28] debugging occasional vscode-ui test failure in CI --- test/vscode-ui/test/specs/nested-fastapi-deployment.spec.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/test/vscode-ui/test/specs/nested-fastapi-deployment.spec.ts b/test/vscode-ui/test/specs/nested-fastapi-deployment.spec.ts index 5475fc278..c39da2002 100644 --- a/test/vscode-ui/test/specs/nested-fastapi-deployment.spec.ts +++ b/test/vscode-ui/test/specs/nested-fastapi-deployment.spec.ts @@ -57,11 +57,6 @@ describe("Nested Fast API Deployment", () => { ); await expect(myConfig).toExist(); myConfig.click(); - - // wait for selection to be complete - await helper.switchToSubframe(); - const deployment = await $(".quick-pick-label"); - await expect(deployment).toHaveText(title); }); it("can edit config", async () => { From 818652c2ba86cd7805b352fd7c221f664a5ba0f1 Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Fri, 20 Dec 2024 16:57:37 -0800 Subject: [PATCH 28/28] debugging occasional vscode-ui test failure in CI --- test/vscode-ui/test/specs/nested-fastapi-deployment.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/vscode-ui/test/specs/nested-fastapi-deployment.spec.ts b/test/vscode-ui/test/specs/nested-fastapi-deployment.spec.ts index c39da2002..62e88084f 100644 --- a/test/vscode-ui/test/specs/nested-fastapi-deployment.spec.ts +++ b/test/vscode-ui/test/specs/nested-fastapi-deployment.spec.ts @@ -56,7 +56,7 @@ describe("Nested Fast API Deployment", () => { `aria/my connect server • fastapi-simple${sep}simple.py`, ); await expect(myConfig).toExist(); - myConfig.click(); + await myConfig.click(); }); it("can edit config", async () => {