From cdea10eae5dbdb430da9cf4c8a9ed2f89e6f68cb Mon Sep 17 00:00:00 2001 From: John Harrison Date: Sat, 21 Jan 2017 22:00:28 +0700 Subject: [PATCH] Unix pipe tofile (#8) * Move infun_read and generic_start_process to read_pipes for all OS. Add utility functions to spawn to file. * Switch chrome to pipe to file * Switch gecko to pipe to file * Switch iedriver to pipe to file * Switch phantomjs to pipe to file * Switch selenium to pipe to file * Fix typo * tidy up spawn_to_file functions * shell quote file paths * shell quote driver paths * Update test * Fix test for windows --- R/chrome.R | 26 +++++++----------- R/gecko.R | 29 +++++++------------- R/iedriver.R | 31 +++++++--------------- R/phantom.R | 30 +++++++-------------- R/selenium.R | 42 +++++++++++------------------ R/utils.R | 48 ++++++++++++++++++++++++++-------- tests/testthat/test-selenium.R | 15 +++++++---- 7 files changed, 100 insertions(+), 121 deletions(-) diff --git a/R/chrome.R b/R/chrome.R index 3ae930b..fab9a11 100644 --- a/R/chrome.R +++ b/R/chrome.R @@ -45,27 +45,20 @@ chrome <- function(port = 4567L, version = "latest", path = "wd/hub", if(retcommand){ return(paste(c(chromeversion[["path"]], args), collapse = " ")) } + pfile <- pipe_files() errTfile <- tempfile(fileext = ".txt") write(character(), errTfile) outTfile <- tempfile(fileext = ".txt") write(character(), outTfile) - chromedrv <- if(identical(.Platform[["OS.type"]], "windows")){ - tfile <- tempfile(fileext = ".bat") - write(paste(c(chromeversion[["path"]], args), collapse = " "), tfile) - subprocess::spawn_process(tfile, arguments = c(">", outTfile, - "2>", errTfile)) - }else{ - subprocess::spawn_process( - chromeversion[["path"]], arguments = args - ) - } + chromedrv <- spawn_tofile(chromeversion[["path"]], + args, pfile[["out"]], pfile[["err"]]) if(!is.na(subprocess::process_return_code(chromedrv))){ stop("Chromedriver couldn't be started", subprocess::process_read(chromedrv, "stderr")) } startlog <- generic_start_log(chromedrv, - outfile = outTfile, - errfile = errTfile) + outfile = pfile[["out"]], + errfile = pfile[["err"]]) if(length(startlog[["stderr"]]) >0){ if(any(grepl("Address already in use", startlog[["stderr"]]))){ subprocess::process_kill(chromedrv) @@ -77,15 +70,16 @@ chrome <- function(port = 4567L, version = "latest", path = "wd/hub", process = chromedrv, output = function(timeout = 0L){ infun_read(chromedrv, log, "stdout", timeout = timeout, - outfile = outTfile, errfile = errTfile) + outfile = pfile[["out"]], errfile = pfile[["err"]]) }, error = function(timeout = 0L){ infun_read(chromedrv, log, "stderr", timeout = timeout, - outfile = outTfile, errfile = errTfile) + outfile = pfile[["out"]], errfile = pfile[["err"]]) }, stop = function(){subprocess::process_kill(chromedrv)}, log = function(){ - infun_read(chromedrv, log, outfile = outTfile, errfile = errTfile) + infun_read(chromedrv, log, + outfile = pfile[["out"]], errfile = pfile[["err"]]) as.list(log) } ) @@ -120,7 +114,7 @@ chrome_ver <- function(platform, version){ mtch <- match(version, chromever) if(is.na(mtch) || is.null(mtch)){ stop("version requested doesnt match versions available = ", - paste(chromever, collpase = ",")) + paste(chromever, collapse = ",")) } chromever[mtch] } diff --git a/R/gecko.R b/R/gecko.R index 491f8f5..9347aab 100644 --- a/R/gecko.R +++ b/R/gecko.R @@ -46,27 +46,16 @@ gecko <- function(port = 4567L, version = "latest", check = TRUE, if(retcommand){ return(paste(c(geckoversion[["path"]], args), collapse = " ")) } - errTfile <- tempfile(fileext = ".txt") - write(character(), errTfile) - outTfile <- tempfile(fileext = ".txt") - write(character(), outTfile) - geckodrv <- if(identical(.Platform[["OS.type"]], "windows")){ - tfile <- tempfile(fileext = ".bat") - write(paste(c(geckoversion[["path"]], args), collapse = " "), tfile) - subprocess::spawn_process(tfile, arguments = c(">", outTfile, - "2>", errTfile)) - }else{ - subprocess::spawn_process( - geckoversion[["path"]], arguments = args - ) - } + pfile <- pipe_files() + geckodrv <- spawn_tofile(geckoversion[["path"]], + args, pfile[["out"]], pfile[["err"]]) if(!is.na(subprocess::process_return_code(geckodrv))){ stop("Geckodriver couldn't be started", subprocess::process_read(geckodrv, "stderr")) } startlog <- generic_start_log(geckodrv, - outfile = outTfile, - errfile = errTfile) + outfile = pfile[["out"]], + errfile = pfile[["err"]]) if(length(startlog[["stderr"]]) >0){ if(any(grepl("Address in use", startlog[["stderr"]]))){ subprocess::process_kill(geckodrv) @@ -78,15 +67,15 @@ gecko <- function(port = 4567L, version = "latest", check = TRUE, process = geckodrv, output = function(timeout = 0L){ infun_read(geckodrv, log, "stdout", timeout = timeout, - outfile = outTfile, errfile = errTfile) + outfile = pfile[["out"]], errfile = pfile[["err"]]) }, error = function(timeout = 0L){ infun_read(geckodrv, log, "stderr", timeout = timeout, - outfile = outTfile, errfile = errTfile) + outfile = pfile[["out"]], errfile = pfile[["err"]]) }, stop = function(){subprocess::process_kill(geckodrv)}, log = function(){ - infun_read(geckodrv, log, outfile = outTfile, errfile = errTfile) + infun_read(geckodrv, log, outfile = pfile[["out"]], errfile = pfile[["err"]]) as.list(log) } ) @@ -121,7 +110,7 @@ gecko_ver <- function(platform, version){ mtch <- match(version, geckover) if(is.na(mtch) || is.null(mtch)){ stop("version requested doesnt match versions available = ", - paste(geckover, collpase = ",")) + paste(geckover, collapse = ",")) } geckover[mtch] } diff --git a/R/iedriver.R b/R/iedriver.R index 26b0f70..1895232 100644 --- a/R/iedriver.R +++ b/R/iedriver.R @@ -43,34 +43,21 @@ iedriver <- function(port = 4567L, version = "latest", check = TRUE, ieversion <- ie_ver(ieplat, version) eopts <- list(...) args <- c(Reduce(c, eopts[names(eopts) == "args"])) - tFile <- tempfile(fileext = ".txt") args[["port"]] <- sprintf("/port=%s", port) args[["log-level"]] <- sprintf("/log-level=%s", loglevel) - args[["log-path"]] <- sprintf("/log-file=%s", tFile) if(retcommand){ return(paste(c(ieversion[["path"]], args), collapse = " ")) } - errTfile <- tempfile(fileext = ".txt") - write(character(), errTfile) - outTfile <- tempfile(fileext = ".txt") - write(character(), outTfile) - iedrv <- if(identical(.Platform[["OS.type"]], "windows")){ - tfile <- tempfile(fileext = ".bat") - write(paste(c(ieversion[["path"]], args), collapse = " "), tfile) - subprocess::spawn_process(tfile, arguments = c(">", outTfile, - "2>", errTfile)) - }else{ - subprocess::spawn_process( - ieversion[["path"]], arguments = args - ) - } + pfile <- pipe_files() + iedrv <- spawn_tofile(ieversion[["path"]], + args, pfile[["out"]], pfile[["err"]]) if(!is.na(subprocess::process_return_code(iedrv))){ stop("iedriver couldn't be started", subprocess::process_read(iedrv, "stderr")) } startlog <- generic_start_log(iedrv, - outfile = outTfile, - errfile = errTfile) + outfile = pfile[["out"]], + errfile = pfile[["err"]]) if(length(startlog[["stderr"]]) >0){ if(any(grepl("Address in use", startlog[["stderr"]]))){ subprocess::process_kill(iedrv) @@ -82,15 +69,15 @@ iedriver <- function(port = 4567L, version = "latest", check = TRUE, process = iedrv, output = function(timeout = 0L){ infun_read(iedrv, log, "stdout", timeout = timeout, - outfile = outTfile, errfile = errTfile) + outfile = pfile[["out"]], errfile = pfile[["err"]]) }, error = function(timeout = 0L){ infun_read(iedrv, log, "stderr", timeout = timeout, - outfile = outTfile, errfile = errTfile) + outfile = pfile[["out"]], errfile = pfile[["err"]]) }, stop = function(){subprocess::process_kill(iedrv)}, log = function(){ - infun_read(iedrv, log, outfile = outTfile, errfile = errTfile) + infun_read(iedrv, log, outfile = pfile[["out"]], errfile = pfile[["err"]]) as.list(log) } ) @@ -126,7 +113,7 @@ ie_ver <- function(platform, version){ mtch <- match(version, iever) if(is.na(mtch) || is.null(mtch)){ stop("version requested doesnt match versions available = ", - paste(iever, collpase = ",")) + paste(iever, collapse = ",")) } iever[mtch] } diff --git a/R/phantom.R b/R/phantom.R index 379a1e0..419a123 100644 --- a/R/phantom.R +++ b/R/phantom.R @@ -46,27 +46,16 @@ phantomjs <- function(port = 4567L, version = "2.1.1", check = TRUE, if(retcommand){ return(paste(c(phantomversion[["path"]], args), collapse = " ")) } - errTfile <- tempfile(fileext = ".txt") - write(character(), errTfile) - outTfile <- tempfile(fileext = ".txt") - write(character(), outTfile) - phantomdrv <- if(identical(.Platform[["OS.type"]], "windows")){ - tfile <- tempfile(fileext = ".bat") - write(paste(c(phantomversion[["path"]], args), collapse = " "), tfile) - subprocess::spawn_process(tfile, arguments = c(">", outTfile, - "2>", errTfile)) - }else{ - subprocess::spawn_process( - phantomversion[["path"]], arguments = args - ) - } + pfile <- pipe_files() + phantomdrv <- spawn_tofile(phantomversion[["path"]], + args, pfile[["out"]], pfile[["err"]]) if(!is.na(subprocess::process_return_code(phantomdrv))){ stop("PhantomJS couldn't be started", subprocess::process_read(phantomdrv, "stderr")) } startlog <- generic_start_log(phantomdrv, - outfile = outTfile, - errfile = errTfile) + outfile = pfile[["out"]], + errfile = pfile[["err"]]) if(length(startlog[["stdout"]]) >0){ if(any( grepl("GhostDriver - main.fail.*sourceURL", startlog[["stdout"]]) @@ -80,15 +69,16 @@ phantomjs <- function(port = 4567L, version = "2.1.1", check = TRUE, process = phantomdrv, output = function(timeout = 0L){ infun_read(phantomdrv, log, "stdout", timeout = timeout, - outfile = outTfile, errfile = errTfile) + outfile = pfile[["out"]], errfile = pfile[["err"]]) }, error = function(timeout = 0L){ infun_read(phantomdrv, log, "stderr", timeout = timeout, - outfile = outTfile, errfile = errTfile) + outfile = pfile[["out"]], errfile = pfile[["err"]]) }, stop = function(){subprocess::process_kill(phantomdrv)}, log = function(){ - infun_read(phantomdrv, log, outfile = outTfile, errfile = errTfile) + infun_read(phantomdrv, log, + outfile = pfile[["out"]], errfile = pfile[["err"]]) as.list(log) } ) @@ -126,7 +116,7 @@ phantom_ver <- function(platform, version){ mtch <- match(version, phantomver) if(is.na(mtch) || is.null(mtch)){ stop("version requested doesnt match versions available = ", - paste(phantomver, collpase = ",")) + paste(phantomver, collapse = ",")) } phantomver[mtch] } diff --git a/R/selenium.R b/R/selenium.R index 28b55dc..1fd7a4e 100644 --- a/R/selenium.R +++ b/R/selenium.R @@ -73,36 +73,23 @@ selenium <- function(port = 4567L, verbose = verbose, jvmargs) # should be the last JVM argument jvmargs[["jar"]] <- "-jar" - jvmargs[["selpath"]] <- seleniumversion[["path"]] + jvmargs[["selpath"]] <- shQuote(seleniumversion[["path"]]) # Selenium JAR arguments selargs[["portswitch"]] <- "-port" selargs[["port"]] <- port if(retcommand){ return(paste(c(javapath, jvmargs, selargs), collapse = " ")) } - errTfile <- tempfile(fileext = ".txt") - write(character(), errTfile) - Sys.chmod(errTfile) - outTfile <- tempfile(fileext = ".txt") - write(character(), outTfile) - Sys.chmod(errTfile) - seleniumdrv <- if(identical(.Platform[["OS.type"]], "windows")){ - tfile <- tempfile(fileext = ".bat") - write(paste(c(javapath, jvmargs, selargs), collapse = " "), tfile) - subprocess::spawn_process(tfile, arguments = c(">", outTfile, - "2>", errTfile)) - }else{ - subprocess::spawn_process( - javapath, arguments = c(jvmargs, selargs) - ) - } + pfile <- pipe_files() + seleniumdrv <- spawn_tofile(javapath, args = c(jvmargs, selargs), + pfile[["out"]], pfile[["err"]]) if(!is.na(subprocess::process_return_code(seleniumdrv))){ stop("Selenium server couldn't be started", subprocess::process_read(seleniumdrv, "stderr")) } startlog <- generic_start_log(seleniumdrv, poll = 10000L, - outfile = outTfile, - errfile = errTfile) + outfile = pfile[["out"]], + errfile = pfile[["err"]]) if(length(startlog[["stderr"]]) >0){ if(any(grepl("Address already in use", startlog[["stderr"]]))){ subprocess::process_kill(seleniumdrv) @@ -119,15 +106,16 @@ selenium <- function(port = 4567L, process = seleniumdrv, output = function(timeout = 0L){ infun_read(seleniumdrv, log, "stdout", timeout = timeout, - outfile = outTfile, errfile = errTfile) + outfile = pfile[["out"]], errfile = pfile[["err"]]) }, error = function(timeout = 0L){ infun_read(seleniumdrv, log, "stderr", timeout = timeout, - outfile = outTfile, errfile = errTfile) + outfile = pfile[["out"]], errfile = pfile[["err"]]) }, stop = function(){subprocess::process_kill(seleniumdrv)}, log = function(){ - infun_read(seleniumdrv, log, outfile = outTfile, errfile = errTfile) + infun_read(seleniumdrv, log, + outfile = pfile[["out"]], errfile = pfile[["err"]]) as.list(log) } ) @@ -159,7 +147,7 @@ selenium_ver <- function(platform, version){ mtch <- match(version, selver) if(is.na(mtch) || is.null(mtch)){ stop("version requested doesnt match versions available = ", - paste(selver, collpase = ",")) + paste(selver, collapse = ",")) } selver[mtch] } @@ -182,7 +170,7 @@ selenium_check_drivers <- function(chromever, geckover, phantomver, cver <- chrome_ver(chromecheck[["platform"]], chromever) jvmargs[["chrome"]] <- sprintf( "-Dwebdriver.chrome.driver=%s", - cver[["path"]] + shQuote(cver[["path"]]) ) } if(!is.null(geckover)){ @@ -190,7 +178,7 @@ selenium_check_drivers <- function(chromever, geckover, phantomver, gver <- gecko_ver(geckocheck[["platform"]], geckover) jvmargs[["gecko"]] <- sprintf( "-Dwebdriver.gecko.driver=%s", - gver[["path"]] + shQuote(gver[["path"]]) ) } if(!is.null(phantomver)){ @@ -198,7 +186,7 @@ selenium_check_drivers <- function(chromever, geckover, phantomver, pver <- phantom_ver(phantomcheck[["platform"]], phantomver) jvmargs[["phantom"]] <- sprintf( "-Dphantomjs.binary.path=%s", - pver[["path"]] + shQuote(pver[["path"]]) ) } if(!is.null(iedrver)){ @@ -206,7 +194,7 @@ selenium_check_drivers <- function(chromever, geckover, phantomver, iever <- ie_ver(iecheck[["platform"]], iedrver) jvmargs[["internetexplorer"]] <- sprintf( "-Dwebdriver.ie.driver=%s", - iever[["path"]] + shQuote(iever[["path"]]) ) } jvmargs diff --git a/R/utils.R b/R/utils.R index 44d141a..61e59e6 100644 --- a/R/utils.R +++ b/R/utils.R @@ -6,11 +6,7 @@ os_arch <- function(string = ""){ infun_read <- function(handle, env, pipe = subprocess::PIPE_BOTH, timeout = 0L, outfile, errfile){ - msg <- if(identical(.Platform[["OS.type"]], "windows")){ - read_pipes(env, outfile, errfile, pipe = pipe, timeout = timeout) - }else{ - subprocess::process_read(handle, pipe = pipe, timeout = timeout) - } + msg <- read_pipes(env, outfile, errfile, pipe = pipe, timeout = timeout) if(identical(pipe, subprocess::PIPE_BOTH)){ env[["stdout"]] <- c(env[["stdout"]], msg[["stdout"]]) @@ -33,12 +29,8 @@ generic_start_log <- function(handle, poll = 3000L, increment = 500L, begin <- Sys.time() errchk <- tryCatch( { - if(identical(.Platform[["OS.type"]], "windows")){ - read_pipes(startlog, outfile, errfile, - timeout = min(increment, poll)) - }else{ - subprocess::process_read(handle, timeout = min(increment, poll)) - } + read_pipes(startlog, outfile, errfile, + timeout = min(increment, poll)) }, error = function(e){ e @@ -76,3 +68,37 @@ read_pipes <- function(env, outfile, errfile, pipe = subprocess::PIPE_BOTH, return(errres) } } + +unix_spawn_tofile <- function(command, args, outfile, errfile, ...){ + tfile <- tempfile(fileext = ".sh") + write("#!/bin/sh", tfile) + write(paste(c(shQuote(command), args, ">", + shQuote(outfile), "2>", shQuote(errfile)), collapse = " "), + tfile, append = TRUE) + Sys.chmod(tfile) + subprocess::spawn_process(tfile, ...) +} + +windows_spawn_tofile <- function(command, args, outfile, errfile, ...){ + tfile <- tempfile(fileext = ".bat") + write(paste(c(shQuote(command), args, ">", + shQuote(outfile), "2>", shQuote(errfile)), collapse = " "), + tfile) + subprocess::spawn_process(tfile, ...) +} + +spawn_tofile <- function(command, args, outfile, errfile, ...){ + if(identical(.Platform[["OS.type"]], "windows")){ + windows_spawn_tofile(command, args, outfile, errfile, ...) + }else{ + unix_spawn_tofile(command, args, outfile, errfile, ...) + } +} + +pipe_files <- function(){ + errTfile <- tempfile(fileext = ".txt") + write(character(), errTfile) + outTfile <- tempfile(fileext = ".txt") + write(character(), outTfile) + list(out = outTfile, err = errTfile) +} diff --git a/tests/testthat/test-selenium.R b/tests/testthat/test-selenium.R index e4cf51e..3a6fdbb 100644 --- a/tests/testthat/test-selenium.R +++ b/tests/testthat/test-selenium.R @@ -59,11 +59,16 @@ test_that("canCallSelenium", { } ) expect_identical(selServ$process, "hello") - expect_true(grepl("-Dwebdriver.chrome.driver=some.path " %+% - "-Dwebdriver.gecko.driver=some.path " %+% - "-Dphantomjs.binary.path=some.path " %+% - "-Dwebdriver.ie.driver=some.path " %+% - "-jar some.path -port 4567", retCommand)) + exRet <- "-Dwebdriver.chrome.driver='some.path' " %+% + "-Dwebdriver.gecko.driver='some.path' " %+% + "-Dphantomjs.binary.path='some.path' " %+% + "-Dwebdriver.ie.driver='some.path' " %+% + "-jar 'some.path' -port 4567" + if(identical(.Platform[["OS.type"]], "unix")){ + expect_true(grepl(exRet, retCommand)) + }else{ + expect_true(grepl(gsub("'", "\"", exRet), retCommand)) + } }) test_that("errorIfJavaNotFound", {