Skip to content

Commit

Permalink
don't restrict shinyapps.io from previous deployment check in deploym…
Browse files Browse the repository at this point in the history
…entTarget (#933)

* don't restrict shinyapps.io from previous deployment check in deploymentTarget

* add tests for shinyapps.io deploymentTarget and NEWS.md entry for 932 bugfix

* add tests for shinyapps.io deploymentTarget and NEWS.md entry for 932 bugfix

* Revert "add tests for shinyapps.io deploymentTarget and NEWS.md entry for 932 bugfix"

This reverts commit 78ddd1d.

* clearer separation between shinyapps/connect tests

* shorten test helper function names
  • Loading branch information
mslynch authored Jul 27, 2023
1 parent 02cada1 commit 74a2cf7
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 7 deletions.
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# rsconnect (development version)

* Fixed redeployments to shinyapps.io where `appName` is provided, but no local
record of the deployment exists. (#932)

* `deployApp()` and `writeManifest()` now error if your library and `renv.lock`
are out-of-sync. Previously it always used what was defined in the `renv.lock`
but that was (a) slow and (b) could lead to different results than what you
Expand Down
2 changes: 1 addition & 1 deletion R/deploymentTarget.R
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ deploymentTarget <- function(recordPath = ".",
}

appId <- NULL
if (!isCloudServer(fullAccount$server)) {
if (!isPositCloudServer(fullAccount$server)) {
# Have we previously deployed elsewhere? We can't do this on cloud
# because it assigns random app names (see #808 for details).
existing <- applications(fullAccount$name, fullAccount$server)
Expand Down
28 changes: 22 additions & 6 deletions tests/testthat/test-deploymentTarget.R
Original file line number Diff line number Diff line change
Expand Up @@ -186,10 +186,10 @@ test_that("default title is the empty string", {
expect_equal(target$appTitle, "")
})

test_that("can find existing application on server & use it", {
confirm_existing_app_used <- function(server) {
local_temp_config()
addTestServer()
addTestAccount("ron")
addTestAccount("ron", server = server)
local_mocked_bindings(
applications = function(...) data.frame(
name = "my_app",
Expand All @@ -201,14 +201,22 @@ test_that("can find existing application on server & use it", {
)

app_dir <- withr::local_tempdir()
target <- deploymentTarget(app_dir, appName = "my_app")
target <- deploymentTarget(app_dir, appName = "my_app", server = server)
expect_equal(target$appId, 123)
}

test_that("can find existing application on server & use it", {
confirm_existing_app_used("example.com")
})

test_that("can find existing application on server & not use it", {
test_that("can find existing application on shinyapps.io & use it", {
confirm_existing_app_used("shinyapps.io")
})

confirm_existing_app_not_used <- function(server) {
local_temp_config()
addTestServer()
addTestAccount("ron")
addTestAccount("ron", server = server)
local_mocked_bindings(
applications = function(...) data.frame(
name = "my_app",
Expand All @@ -220,9 +228,17 @@ test_that("can find existing application on server & not use it", {
)

app_dir <- withr::local_tempdir()
target <- deploymentTarget(app_dir, appName = "my_app")
target <- deploymentTarget(app_dir, appName = "my_app", server = server)
expect_equal(target$appName, "my_app-1")
expect_equal(target$appId, NULL)
}

test_that("can find existing application on server & not use it", {
confirm_existing_app_not_used("example.com")
})

test_that("can find existing application on shinyapps.io & not use it", {
confirm_existing_app_not_used("shinyapps.io")
})

# defaultAppName ----------------------------------------------------------
Expand Down

0 comments on commit 74a2cf7

Please sign in to comment.