Merge pull request #1354 from ropensci/new-test-framework

New visual testing framework based on vdiffr

Author: cpsievert

.travis.yml

@@ -2,53 +2,36 @@ language: R
 cache: packages
 sudo: true
 dist: trusty
-
-r:
-  - oldrel
-  - release
-  - devel
-
+warnings_are_errors: false
+
+matrix:
+  include:
+    - r:
+      - devel
+      - oldrel
+      - release
+        # run visual tests
+        # TODO: push the svg files somewhere on after_failure?
+        before_script: docker run -e MAPBOX_TOKEN=$MAPBOX_TOKEN -v $(pwd):/home/plotly --privileged cpsievert/plotly-orca R -e "devtools::install_deps('/home/plotly', dep = T); res <- devtools::test('/home/plotly', reporter='summary'); df <- as.data.frame(res); if (sum(df\$failed) > 0 || any(df\$error)) q(status=1)"
+      
+    
 env:
   global:
-    - _R_CHECK_FORCE_SUGGESTS_=false R_PKG="$(basename $TRAVIS_REPO_SLUG)"
+    # don't treat missing suggested packages as error
+    - _R_CHECK_FORCE_SUGGESTS_=false
     # plotly_api_key (for posting to plot.ly)
     - secure: "WsvmMHN4YVhnk0bLRE04APcLbs5s4vWKSHjEdU0bPXd0xdMTzZeP5D7pxyF1983C+P5LpSnGHv4dgwLMBkNzxJwBR7/Ta7lfO1akYILWwxib+1DVbCqUH5Z4Ba1FSCQptIrLNGR3P7+0Lem4hEhqKdPKltFnxhnXO0Y+MeG71IQ="
-    # GITHUB_PAT (for pushing to plotly-test-table)
-    - secure: "Ar5FRxsMJHxb8CQmmsoFx5Tm6ncFvF3LHtAMtfcOE2e9JAbXQ471PGzOMM7yudGa2vVuQJBpB6bWktuE30J68MQxgRmxeaJYmOp+ePqyW80y/zEa2P3R4//eb9Rif2lmsmaqz7HtVfX4xr3YX4N+ZjfyI8TFt5Phue18tXeMoJw="
     # MAPBOX_TOKEN (for testing `plot_mapbox()`)
     - secure: "QPBEqtLRdwb4ablJORzD0JdCT9ESe3nNdIehM1oxfcNKfpSdf2OFxH3TkeYY1nMpv0mLxiMNTy6xcj9Yk5MaaBCIA0P7q6OdZv9ruzQD1j3g84gP45KwBilbPGjb+/EvOS0fM25vR/pAmA8IyoUfPC2J8HwiNnW8DYdt/hJOJ9A="
 
-# installing Rserve from source requires non-standard R build (--enable-R-shlib) 
-r_binary_packages:
-  - rserve
-
-r_packages:
-  - devtools
-  - quantreg
-  
-r_github_packages:
-  - tidyverse/ggplot2
-
 before_install:
-  # mainly for installing sf (which requires units/rgeos/rgdal)
+  - echo "Sys.setenv('plotly_username' = 'cpsievert')" > ~/.Rprofile
+  # sf system dependencies
   - sudo add-apt-repository ppa:ubuntugis/ubuntugis-unstable --yes
   - sudo apt-get --yes --force-yes update -qq
   - sudo apt-get install --yes libudunits2-dev libproj-dev libgeos-dev libgdal-dev
+  # make sure R pkgs are upt-to-date
   - Rscript -e 'update.packages(ask = FALSE)'
 
-before_script:
-  - git config --global user.email "[email protected]"
-  - git config --global user.name "cpsievert"
-  - echo "Sys.setenv('plotly_username' = 'cpsievert')" > ~/.Rprofile
-  - git clone https://github.com/cpsievert/plotly-test-table.git ../plotly-test-table
-  - cd ..; rm -f *.tar.gz; R CMD build $R_PKG
-
-script:
-  # run R CMD check on the non-pull request build
-  - sh -c "if [ '$TRAVIS_PULL_REQUEST' = 'false' ]; then R CMD check ${R_PKG}_*.tar.gz --no-manual; fi"
-  # we do some fancy stuff on the pull request build that confuses R CMD check
-  - sh -c "if [ '$TRAVIS_PULL_REQUEST' != 'false' ]; then Rscript -e 'source(\"plotly/tests/testthat.R\", chdir = T)'; fi"
-
-after_success:
-  - cd plotly-test-table
-  - Rscript ../plotly/inst/build-push-comment.R
+# work around temporary travis + R 3.5 bug
+r_packages: devtools  

CONTRIBUTING.md

@@ -6,28 +6,24 @@ See the [opening issues template](https://github.com/ropensci/plotly/blob/master
 
 ## Development guidelines
 
-If you'd like to contribute changes to plotly, we use [the GitHub flow](https://guides.github.com/introduction/flow/index.html) for proposing, submitting, reviewing, and accepting changes. If you haven't done this before, Hadley Wickham provides a nice overview of git (<http://r-pkgs.had.co.nz/git.html>), as well as best practices for submitting pull requests (<http://r-pkgs.had.co.nz/git.html#pr-make>). We also recommend using his style guide when writing code (<http://adv-r.had.co.nz/Style.html>).
+If you'd like to contribute changes to plotly, we use [the GitHub flow](https://guides.github.com/introduction/flow/index.html) for proposing, submitting, reviewing, and accepting changes. If you aren't familiar with git and/or GitHub, we recommend studying these excellent free resources by [Hadley Wickham](http://r-pkgs.had.co.nz/git.html) and [Jenny Bryan](http://happygitwithr.com). We also prefer R coding style that adheres to <http://style.tidyverse.org/>.
 
-If your pull request fixes a bug, or implements a new feature, it's a good idea to write a test (<http://r-pkgs.had.co.nz/tests.html>) to demonstrate it's working. If you'd like to closely simulate the tests that run when you submit your pull request, open R under your local plotly git repo, then do the following:
+If your pull request fixes a bug and/or implements a new feature, please [write a test via testthat](http://r-pkgs.had.co.nz/tests.html) to demonstrate it's working. Tests should generally check the return value of `plotly_build()`, but can also use **vdiffr**'s `expect_doppelganger()` to add a visual test! By default, `devtools::tests()` won't run the visual tests, but you *can* run visual tests on your machine via `Sys.setenv("VDIFFR" = "true"); devtools::test()`. That being said, false positives are likely to occur simply due to superfluous differences in your system environment, so we recommend running visual tests via docker.
 
-```r
-# the pull request number is arbitrary when running locally
-Sys.setenv('TRAVIS_PULL_REQUEST' = '1')
-Sys.setenv('TRAVIS_COMMIT' = substr(system('git rev-parse HEAD', intern = T), 1, 7))
-devtools::load_all(); source('tests/testthat.R', chdir = TRUE)
-```
+### Running visual tests via docker
 
-You can also build a ggplot2/plotly comparison table. To do so, you'll first need to clone the [plotly-test-table](https://github.com/cpsievert/plotly-test-table) repo.
+To ensure a consistent and reproducible environment, visual tests should be run against the [cpsievert/plotly-orca](https://hub.docker.com/r/cpsievert/plotly-orca/) docker image. If you add a new visual test and/or expect any differences, run a container like so:
 
 ```shell
-$ git clone https://github.com/cpsievert/plotly-test-table.git ../plotly-test-table
+git clone https://github.com/ropensci/plotly.git
+cd plotly
+docker run -v $(pwd):/home/plotly --privileged -p 3838:3838 cpsievert/plotly-orca
 ```
 
-Then, from R:
+This will launch a shiny app for inspecting and validating any visual differences. To see the shiny app, open your browser to <http://0.0.0.0/3838>. If there are differences that look 'good', you should validate them via the shiny app. This will automatically copy over the new "baseline" figures over to your host machine (so that you can git add/commit/push the new baselines). If, for some reason, you want to just run the visual tests to see if they'll pass, do:
 
-```r
-Sys.setenv('PLOTLY_TABLE' = 'TRUE')
-devtools::load_all(); source('tests/testthat.R', chdir = TRUE)
+```shell
+docker run -e VMODE="ci" -v $(pwd):/home/plotly --privileged cpsievert/plotly-orca
 ```
 
 ## Code of Conduct

DESCRIPTION

@@ -56,8 +56,7 @@ Suggests:
     shiny (>= 1.1.0),
     curl,
     rmarkdown,
-    Rserve,
-    RSclient,
+    vdiffr,
     Cairo,
     broom,
     webshot,
@@ -72,6 +71,6 @@ Suggests:
     plotlyGeoAssets,
     forcats
 LazyData: true
-RoxygenNote: 6.0.1.9000
+RoxygenNote: 6.1.0
 Encoding: UTF-8
 Roxygen: list(markdown = TRUE)

NAMESPACE

@@ -153,6 +153,7 @@ export(mutate)
 export(mutate_)
 export(offline)
 export(orca)
+export(orca_serve)
 export(partial_bundle)
 export(plot_dendro)
 export(plot_geo)
@@ -244,6 +245,7 @@ importFrom(httr,add_headers)
 importFrom(httr,config)
 importFrom(httr,content)
 importFrom(httr,stop_for_status)
+importFrom(httr,warn_for_status)
 importFrom(jsonlite,fromJSON)
 importFrom(jsonlite,toJSON)
 importFrom(lazyeval,all_dots)

NEWS.md

@@ -1,5 +1,10 @@
 # 4.8.0.9000
 
+## NEW FEATURES
+
+* The `orca_serve()` function was added for efficient exporting of many plotly graphs. For examples, see `help(orca_serve)`.
+* The `orca()` function gains new arguments `more_args` and `...` for finer control over the underlying system commands.
+
 ## IMPROVEMENTS
 
 * Upgraded to plotly.js v1.40.1.

R/imports.R

@@ -6,7 +6,7 @@
 #' @importFrom tidyr unnest
 #' @importFrom viridisLite viridis
 #' @importFrom jsonlite toJSON fromJSON
-#' @importFrom httr GET POST PATCH content config add_headers stop_for_status
+#' @importFrom httr GET POST PATCH content config add_headers stop_for_status warn_for_status
 #' @importFrom htmlwidgets createWidget sizingPolicy saveWidget onRender prependContent
 #' @importFrom lazyeval f_eval is_formula all_dots is_lang f_new
 #' @importFrom tibble as_tibble

R/orca.R

@@ -1,7 +1,13 @@
-#' Static image export via orca
+#' Static image exporting 
 #' 
-#' The function makes a system call to the orca command-line utility, 
-#' see the installation instructions [here](https://github.com/plotly/orca#installation)
+#' Export plotly objects to static images (e.g., pdf, png, jpeg, svg, etc) via the
+#' [orca command-line utility](https://github.com/plotly/orca#installation).
+#' 
+#' The `orca()` function is designed for exporting one plotly graph whereas `orca_serve()`
+#' is meant for exporting many graphs at once. The former starts and stops an external (nodejs)
+#' process everytime it is called whereas the latter starts up a process when called, then
+#' returns an `export()` method for exporting graphs as well as a `close()` method for stopping 
+#' the external (background) process.
 #' 
 #' @param p a plotly object.
 #' @param file output filename.
@@ -20,28 +26,48 @@
 #' @param debug Starts app in debug mode and turn on verbose logs on stdout.
 #' @param safe Turns on safe mode: where figures likely to make browser window 
 #' hang during image generating are skipped.
+#' @param more_args additional arguments to pass along to system command. This is useful
+#' for specifying display and/or electron options, such as `--enable-webgl` or `--disable-gpu`.
+#' @param ... for `orca()`, additional arguments passed along to `processx::run`. For 
+#' `orca_serve()`, additional arguments passed along to `processx::process`.
 #' @export
 #' @author Carson Sievert
+#' @md
+#' @rdname orca
 #' @examples
 #' 
 #' \dontrun{
+#' # NOTE: in a headless environment, you may need to set `more_args="--enable-webgl"`
+#' # to export webgl correctly
 #' p <- plot_ly(z = ~volcano) %>% add_surface()
 #' orca(p, "surface-plot.svg")
+#' 
+#' #' # launch the server
+#' server <- orca_serve()
+#' 
+#' # export as many graphs as you'd like
+#' server$export(qplot(1:10), "test1.pdf")
+#' server$export(plot_ly(x = 1:10, y = 1:10), "test2.pdf")
+#' 
+#' # the underlying process is exposed as a field, so you
+#' # have full control over the external process
+#' server$process$is_alive()
+#' 
+#' # convenience method for closing down the server
+#' server$close()
+#' 
+#' # remove the exported files from disk
+#' unlink("test1.pdf")
+#' unlink("test2.pdf")
 #' }
 #' 
 
 orca <- function(p, file = "plot.png", format = tools::file_ext(file), 
                  scale = NULL, width = NULL, height = NULL, mathjax = FALSE,
                  parallel_limit = NULL, verbose = FALSE, debug = FALSE, 
-                 safe = FALSE) {
+                 safe = FALSE, more_args = NULL, ...) {
 
-  if (Sys.which("orca") == "") {
-    stop(
-      "The orca command-line utility is required to use the `orca()` function.\n\n",
-      "Follow the installation instructions here -- https://github.com/plotly/orca#installation",
-      call. = FALSE
-    )
-  }
+  orca_available()
 
   b <- plotly_build(p)
 
@@ -59,7 +85,8 @@ orca <- function(p, file = "plot.png", format = tools::file_ext(file),
     "--plotlyjs", plotlyjs_file,
     if (debug) "--debug",
     if (verbose) "--verbose",
-    if (safe) "--safe-mode"
+    if (safe) "--safe-mode",
+    more_args
   )
 
   if (!is.null(scale)) args <- c(args, "--scale", scale)
@@ -68,8 +95,125 @@ orca <- function(p, file = "plot.png", format = tools::file_ext(file),
   if (!is.null(parallel_limit)) args <- c(args, "--parallel-limit", parallel_limit)
   if (!is.null(tryNULL(mapbox_token()))) args <- c(args, "--mapbox-access-token", mapbox_token())
   if (isTRUE(mathjax)) args <- c(args, "--mathjax", file.path(mathjax_path(), "MathJax.js"))
-     
+  
   # TODO: point to local topojson? Should this only work if plot_geo(standalone = TRUE)?
   try_library("processx", "orca")
-  invisible(processx::run("orca", args, echo = TRUE, spinner = TRUE))
+  invisible(processx::run("orca", args, echo = TRUE, spinner = TRUE, ...))
+}
+
+#' Orca image export server
+#' 
+#' @inheritParams orca
+#' @param port Sets the server's port number.
+#' @param keep_alive Turn on keep alive mode where orca will (try to) relaunch server if process unexpectedly exits.
+#' @param window_max_number Sets maximum number of browser windows the server can keep open at a given time.
+#' @param request_limit Sets a request limit that makes orca exit when reached.
+#' @param quiet Suppress all logging info.
+#' 
+#' @section Methods:
+#' 
+#' The `orca_serve()` function returns an object with two methods:
+#' 
+#' \describe{
+#'   \item{\code{export(p, file = "plot.png", format = tools::file_ext(file), scale = NULL, width = NULL, height = NULL)}}{
+#'     Export a static image of a plotly graph. Arguments found here are the same as those found in `orca()`
+#'   }
+#'   \item{\code{close()}}{Close down the orca server and kill the underlying node process.}
+#' }
+#' 
+#' @section Fields:
+#' 
+#' The `orca_serve()` function returns an object with two fields:
+#' 
+#' \describe{
+#'   \item{\code{port}}{The port number that the server is listening to.}
+#'   \item{\code{process}}{An R6 class for controlling and querying the underlying node process.}
+#' }
+#' 
+#' @export
+#' @rdname orca
+
+orca_serve <- function(port = 5151, mathjax = FALSE, safe = FALSE, request_limit = NULL,
+                       keep_alive = TRUE, window_max_number = NULL, quiet = FALSE, 
+                       debug = FALSE, more_args = NULL, ...) {
+  
+  # make sure we have the required infrastructure
+  orca_available()
+  try_library("processx", "orca_serve")
+  
+  # use main bundle since any plot can be thrown at the server
+  plotlyjs <- plotlyMainBundle()
+  plotlyjs_file <- file.path(plotlyjs$src$file, plotlyjs$script)
+  
+  args <- c(
+    "serve",
+    "-p", port,
+    "--plotly", plotlyjs_file,
+    if (safe) "--safe-mode",
+    if (orca_version() >= "1.1.1") "--graph-only",
+    if (keep_alive) "--keep-alive",
+    if (debug) "--debug",
+    if (quiet) "--quiet",
+    more_args
+  )
+  
+  if (!is.null(request_limit))
+    args <- c(args, "--request-limit", request_limit)
+  
+  if (!is.null(window_max_number))
+    args <- c(args, "--window-max-number", window_max_number)
+  
+  if (!is.null(tryNULL(mapbox_token()))) 
+    args <- c(args, "--mapbox-access-token", mapbox_token())
+  
+  if (isTRUE(mathjax)) 
+    args <- c(args, "--mathjax", file.path(mathjax_path(), "MathJax.js"))
+  
+  process <- processx::process$new("orca", args, ...)
+  
+  list(
+    port = port,
+    process = process,
+    close = function() process$kill(),
+    export = function(p, file = "plot.png", format = tools::file_ext(file), scale = NULL, width = NULL, height = NULL) {
+      # request/response model works similarly to plotly_IMAGE()
+      bod <- list(
+        figure = plotly_build(p)$x[c("data", "layout")],
+        format = format,
+        width = width,
+        height = height,
+        scale = scale
+      )
+      res <- httr::POST(
+        paste0("http://127.0.0.1:", port), 
+        body = to_JSON(bod)
+      )
+      httr::stop_for_status(res)
+      httr::warn_for_status(res)
+      con <- httr::content(res, as = "raw")
+      writeBin(con, file)
+    }
+  )
+}
+
+
+orca_available <- function() {
+  if (Sys.which("orca") == "") {
+    stop(
+      "The orca command-line utility is required for this functionality.\n\n",
+      "Please follow the installation instructions here -- https://github.com/plotly/orca#installation",
+      call. = FALSE
+    )
+  }
+  
+  TRUE
+}
+
+orca_version <- function() {
+  orca_available()
+  # default to initial release if we can't correctly parse version
+  tryCatch(
+    as.package_version(system("orca --version", intern = TRUE)), 
+    error = function(e) "1.0.0"
+  )
 }