From 1f54d0cda6f151616edfd0651b785d5f69138e69 Mon Sep 17 00:00:00 2001 From: Wim Date: Mon, 10 Feb 2020 15:13:07 -0500 Subject: [PATCH 1/5] Always display message about how to view diff --- R/snapshot.R | 4 ---- 1 file changed, 4 deletions(-) diff --git a/R/snapshot.R b/R/snapshot.R index 67597ef2..6c03e4fc 100644 --- a/R/snapshot.R +++ b/R/snapshot.R @@ -247,7 +247,6 @@ snapshotCompareSingle <- function( if (interactive) { response <- readline("Would you like to view the differences between expected and current results [y/n]? ") if (tolower(response) == "y") { - quiet <- TRUE result <- viewTestDiff(appDir, testname, interactive, suffix = suffix)[[1]] if (result == "accept") { @@ -255,9 +254,6 @@ snapshotCompareSingle <- function( snapshot_status <- "updated" } } - } - - if (!quiet && interactive) { if (is.null(suffix) || suffix == "") { suffix_param <- "" From a9b227d438de0b6a3cfa601c1620e7f8acfdb73e Mon Sep 17 00:00:00 2001 From: Wim Date: Mon, 10 Feb 2020 15:52:04 -0500 Subject: [PATCH 2/5] Show message when diffs are rejected or not opened --- R/snapshot.R | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/R/snapshot.R b/R/snapshot.R index 6c03e4fc..e626fdd6 100644 --- a/R/snapshot.R +++ b/R/snapshot.R @@ -255,15 +255,18 @@ snapshotCompareSingle <- function( } } - if (is.null(suffix) || suffix == "") { - suffix_param <- "" - } else { - suffix_param <- paste0(', suffix="', suffix, '"') + if (result == "reject" && !quiet) { + if (is.null(suffix) || suffix == "") { + suffix_param <- "" + } else { + suffix_param <- paste0(', suffix="', suffix, '"') + } + message('\n To view differences between expected and current results, run:\n', + ' viewTestDiff("', relativeAppDir, '", "', testname, '"', suffix_param, ')\n', + ' To save current results as expected results, run:\n', + ' snapshotUpdate("', relativeAppDir, '", "', testname, '"', suffix_param, ')\n') + } - message('\n To view differences between expected and current results, run:\n', - ' viewTestDiff("', relativeAppDir, '", "', testname, '"', suffix_param, ')\n', - ' To save current results as expected results, run:\n', - ' snapshotUpdate("', relativeAppDir, '", "', testname, '"', suffix_param, ')\n') } } else { From c8e550eb9ef822dd515cd9a74c6a63aa57ad17e8 Mon Sep 17 00:00:00 2001 From: Wim Date: Mon, 10 Feb 2020 15:57:28 -0500 Subject: [PATCH 3/5] Only set quiet to TRUE when results are accepted --- R/snapshot.R | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/R/snapshot.R b/R/snapshot.R index e626fdd6..3d191e26 100644 --- a/R/snapshot.R +++ b/R/snapshot.R @@ -26,8 +26,8 @@ sd_snapshot <- function(self, private, items, filename, screenshot) extra_names <- setdiff(names(items), c("input", "output", "export")) if (length(extra_names) > 0) { stop("'items' must be a list containing one or more items named", - "'input', 'output' and 'export'. Each of these can be TRUE, FALSE, ", - " or a character vector.") + "'input', 'output' and 'export'. Each of these can be TRUE, FALSE, ", + " or a character vector.") } if (is.null(items$input)) items$input <- FALSE @@ -252,21 +252,22 @@ snapshotCompareSingle <- function( if (result == "accept") { snapshot_pass <- TRUE snapshot_status <- "updated" + quiet <- TRUE } } + } - if (result == "reject" && !quiet) { - if (is.null(suffix) || suffix == "") { - suffix_param <- "" - } else { - suffix_param <- paste0(', suffix="', suffix, '"') - } - message('\n To view differences between expected and current results, run:\n', - ' viewTestDiff("', relativeAppDir, '", "', testname, '"', suffix_param, ')\n', - ' To save current results as expected results, run:\n', - ' snapshotUpdate("', relativeAppDir, '", "', testname, '"', suffix_param, ')\n') + if (!quiet && interactive) { + if (is.null(suffix) || suffix == "") { + suffix_param <- "" + } else { + suffix_param <- paste0(', suffix="', suffix, '"') } + message('\n To view differences between expected and current results, run:\n', + ' viewTestDiff("', relativeAppDir, '", "', testname, '"', suffix_param, ')\n', + ' To save current results as expected results, run:\n', + ' snapshotUpdate("', relativeAppDir, '", "', testname, '"', suffix_param, ')\n') } } else { From d89a3d94df594b149c7ca5c2b54a95ae2832e4fe Mon Sep 17 00:00:00 2001 From: Wim Date: Mon, 10 Feb 2020 16:01:01 -0500 Subject: [PATCH 4/5] Fix whitespaces --- R/snapshot.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/R/snapshot.R b/R/snapshot.R index 3d191e26..0de650bd 100644 --- a/R/snapshot.R +++ b/R/snapshot.R @@ -26,8 +26,8 @@ sd_snapshot <- function(self, private, items, filename, screenshot) extra_names <- setdiff(names(items), c("input", "output", "export")) if (length(extra_names) > 0) { stop("'items' must be a list containing one or more items named", - "'input', 'output' and 'export'. Each of these can be TRUE, FALSE, ", - " or a character vector.") + "'input', 'output' and 'export'. Each of these can be TRUE, FALSE, ", + " or a character vector.") } if (is.null(items$input)) items$input <- FALSE From 86e575c75eef8c23dc2cfc984fd9d2942f75e66e Mon Sep 17 00:00:00 2001 From: Wim Date: Fri, 14 Feb 2020 15:58:15 -0500 Subject: [PATCH 5/5] Add news item --- NEWS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.md b/NEWS.md index 7a64d657..026a7312 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,8 @@ shinytest (development version) ========== +* Also display the massage about where to find the diff when de diff viewer was opened but the diffs were not accepted. ([#131](https://github.com/rstudio/shinytest/issues/131)) + * Recommend that tests be placed in `tests/shinytests/` instead of directly in the tests directory. Users with their tests in the `tests/` directory will now see a message about this change. Storing shinytests directly in `tests/` will be deprecated in the future. * Added new `suffix` option, which allows adding a suffix to an expected results directory. This makes it possible to store multiple sets of results, which can be useful, for example, if you run tests on multiple platforms. ([#295](https://github.com/rstudio/shinytest/pull/295))