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