Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Moved wdman and binman to Suggests #177

Closed
wants to merge 2 commits into from
Closed

Conversation

JBGruber
Copy link

@JBGruber JBGruber commented Jun 24, 2018

This is a very simple PR. It removes imports of wdman and binman and moves the packages to Suggests (following the comments in #172 ). Both packages are currently unavailable from CRAN, which means RSelenium can't be installed using devtools::install_github("ropensci/RSelenium") if the user does not have them installed. wdman was only used in rsDriver() which now displays a error message if the package is unavailable, and the deprecated message for phantom() which was removed. binman is only used in the help text of rsDriver(). The details of this function now include the command how binman can be installed from GitHub.

devtools::check("RSelenium") produces the following errors, warnings and notes:

Without wdman and binman installed

R CMD check results
1 error  | 1 warning  | 1 note 
checking tests ... ERROR
  Running ‘testthat.R’
Running the tests in ‘tests/testthat.R’ failed.
Last 13 lines of output:
  ══ testthat results  ═════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════
  OK: 3 SKIPPED: 0 FAILED: 10
  1.  Error: (unknown) (@test-alerts_tests.R#2) 
  2.  Error: (unknown) (@test-api_example_tests.R#2) 
  3.  Error: (unknown) (@test-cookie_tests.R#2) 
  4.  Failure: canGetHttrError (@test-errorHandler.R#6) 
  5.  Error: (unknown) (@test-executing_javascript_tests.R#2) 
  6.  Error: (unknown) (@test-misc_remoteDriver_methods_tests.R#2) 
  7.  Error: (unknown) (@test-misc_webElement_methods_tests.R#2) 
  8.  Error: (unknown) (@test-page_loading_tests.R#2) 
  9.  Error: (unknown) (@test-takes_screenshots_tests.R#2) 
  10. Error: (unknown) (@test-util_function_tests.R#2) 
  
  Error: testthat unit tests failed
  Execution halted

checking Rd cross-references ... WARNING
Unknown package ‘wdman’ in Rd xrefs

checking package dependencies ... NOTE
Packages suggested but not available for checking: ‘wdman’ ‘binman’

With wdman and binman installed

R CMD check results
1 error  | 0 warnings | 0 notes
checking tests ... ERROR
  Running ‘testthat.R’
Running the tests in ‘tests/testthat.R’ failed.
Last 13 lines of output:
  ══ testthat results  ═════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════
  OK: 3 SKIPPED: 0 FAILED: 10
  1.  Error: (unknown) (@test-alerts_tests.R#2) 
  2.  Error: (unknown) (@test-api_example_tests.R#2) 
  3.  Error: (unknown) (@test-cookie_tests.R#2) 
  4.  Failure: canGetHttrError (@test-errorHandler.R#6) 
  5.  Error: (unknown) (@test-executing_javascript_tests.R#2) 
  6.  Error: (unknown) (@test-misc_remoteDriver_methods_tests.R#2) 
  7.  Error: (unknown) (@test-misc_webElement_methods_tests.R#2) 
  8.  Error: (unknown) (@test-page_loading_tests.R#2) 
  9.  Error: (unknown) (@test-takes_screenshots_tests.R#2) 
  10. Error: (unknown) (@test-util_function_tests.R#2) 
  
  Error: testthat unit tests failed
  Execution halted

I did my best to follow instructions in the test directory to get rid of the testthat errors. Using the debug docker image I was able to see that the sites in the local web directory were called and displayed. Yet, there were null.Pointer exceptions when I tried running the tests manually. However, since this PR does not alter any functions, it should be fine. When the testthat test are ignored, there are no errors, warnings or notes.

A manual test conducted by removing binman and wdman, installing the altered package and using RSelenium together with the selenium/standalone-firefox:2.53.0 docker image to run a few operations on websites. This worked without issues.

Session info

> sessionInfo()
R version 3.5.0 (2018-04-23)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 18.04 LTS

Matrix products: default
BLAS: /usr/lib/x86_64-linux-gnu/blas/libblas.so.3.7.1
LAPACK: /usr/lib/x86_64-linux-gnu/lapack/liblapack.so.3.7.1

locale:
 [1] LC_CTYPE=en_GB.UTF-8       LC_NUMERIC=C               LC_TIME=en_GB.UTF-8        LC_COLLATE=en_GB.UTF-8     LC_MONETARY=en_GB.UTF-8    LC_MESSAGES=en_GB.UTF-8   
 [7] LC_PAPER=en_GB.UTF-8       LC_NAME=C                  LC_ADDRESS=C               LC_TELEPHONE=C             LC_MEASUREMENT=en_GB.UTF-8 LC_IDENTIFICATION=C       

attached base packages:
[1] tools     stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] RSelenium_1.7.3 roxygen2_5.0.1  httr_1.3.1      openssl_1.0.1   caTools_1.17.1  XML_3.99-0      devtools_1.13.5

loaded via a namespace (and not attached):
 [1] Rcpp_0.12.17        rstudioapi_0.7      knitr_1.20          xml2_1.2.0          magrittr_1.5        R6_2.2.2            rlang_0.2.1         stringr_1.3.1      
 [9] git2r_0.21.0        withr_2.1.2         commonmark_1.5      yaml_2.1.19         assertthat_0.2.0    digest_0.6.15       rprojroot_1.3-2     crayon_1.3.4       
[17] bitops_1.0-6        codetools_0.2-15    testthat_2.0.0      curl_3.2            memoise_1.1.0       Rcompression_0.94-0 stringi_1.2.3       compiler_3.5.0     
[25] desc_1.2.0          backports_1.1.2    

@juyeongkim
Copy link
Contributor

Hi @JBGruber! Thank you for PR, but I am closing this as wdman and binman are back on CRAN.

@juyeongkim juyeongkim closed this Aug 1, 2018
@JBGruber
Copy link
Author

JBGruber commented Aug 2, 2018

That's fine. In #172 the broader point was made that these imports might be better moved to suggest anyway as they are used by only a few functions. But it's up to you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants