From 74a2cf72934b6e9dcd598cfde9e9c51598e883ea Mon Sep 17 00:00:00 2001 From: Matthew Lynch Date: Wed, 26 Jul 2023 20:21:06 -0500 Subject: [PATCH] don't restrict shinyapps.io from previous deployment check in deploymentTarget (#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 78ddd1dddcceeb666a74faba2346d030a5780895. * clearer separation between shinyapps/connect tests * shorten test helper function names --- NEWS.md | 3 +++ R/deploymentTarget.R | 2 +- tests/testthat/test-deploymentTarget.R | 28 ++++++++++++++++++++------ 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/NEWS.md b/NEWS.md index 42a12afa..c81ec11d 100644 --- a/NEWS.md +++ b/NEWS.md @@ -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 diff --git a/R/deploymentTarget.R b/R/deploymentTarget.R index 82133b7b..4e2b9690 100644 --- a/R/deploymentTarget.R +++ b/R/deploymentTarget.R @@ -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) diff --git a/tests/testthat/test-deploymentTarget.R b/tests/testthat/test-deploymentTarget.R index 9f242b3c..89093c05 100644 --- a/tests/testthat/test-deploymentTarget.R +++ b/tests/testthat/test-deploymentTarget.R @@ -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", @@ -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", @@ -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 ----------------------------------------------------------