Test #40

cpsievert · 2015-01-27T18:19:55Z

This is currently broken, but a step closer to an ideal testing framework.

Most importantly, I want an argument to specify which browser to use for testing.

It might also be nice to eventually export a function like animint::run_test() one day, so to run a test on brand new code, we'd just have to devtools::load_all(); run_test().

tdhock · 2015-01-29T18:08:30Z

I tried run_tests("firefox") from both /animint and /animint/tests, but neither worked... (same for phantomjs)

> run_tests()
Unknown option '--webdriver=4444'
Couldnt connect to host on http://localhost:4444/wd/hub.
Please ensure a Selenium server is running.Error in queryRD(paste0(serverURL, "/session"), "POST", qdata = toJSON(serverOpts)) (from #40) : 

Error in read.table(text = pid.tab) (from #7) : no lines available in input
In addition: Warning message:
running command 'lsof -i tcp:4848' had status 1 
> run_tests("firefox")
aesthetics : List of 1
 $ showSelected: symbol foo
.
animation : ...
1 character value : ....12
chunks : ......
coord : ....34
facet-coord : 56
facet lines : .7
facet_grid(space="free") : 89abcdefghijk.l...
facet-strips : mn
aes(href) : .opError:    Summary: NoSuchElement
     Detail: An element could not be located on the page using the given search parameters.
     class: org.openqa.selenium.NoSuchElementException
Error in read.table(text = pid.tab) (from #7) : no lines available in input
In addition: Warning message:
running command 'lsof -i tcp:4848' had status 1 
> getwd()
[1] "/home/thocking/R/animint/tests"
> setwd("testthat")
> run_tests("firefox")
Error in file(filename, "r", encoding = encoding) (from #13) : 
  cannot open the connection
In addition: Warning messages:
1: In run_tests("firefox") :
  Make sure your working directory is set to animint's source path
2: In file(filename, "r", encoding = encoding) :
  cannot open file 'testthat/functions.R': No such file or directory
> setwd("~/R/animint")
> run_tests("firefox")
aesthetics : List of 1
 $ showSelected: symbol foo
.
animation : ...
1 character value : ....12
chunks : ......
coord : ....34
facet-coord : 56
facet lines : .7
facet_grid(space="free") : 89abcdefghijk.l...
facet-strips : mn
aes(href) : .opError:    Summary: NoSuchElement
     Detail: An element could not be located on the page using the given search parameters.
     class: org.openqa.selenium.NoSuchElementException
Error in read.table(text = pid.tab) (from #7) : no lines available in input
In addition: Warning message:
running command 'lsof -i tcp:4848' had status 1 
>

cpsievert · 2015-01-30T05:13:08Z

@tdhock you'll want to devtools::install_github("yihui/servr")

The newest version seems to break testing on the master branch (different arguments), but the exciting thing for us is that he added native support for spawning a daemonized file server. This should improve our current hack, but it seems to be the root of the remDr$navigate() issues that I elude to above.

Anyway, I started #41, which simply tries to implement the new daemon support. Let's try to get that working first...

cpsievert · 2015-01-30T23:48:40Z

@tdhock The way we currently run tests (e.g. devtools::load_all(); testthat::test_dir("tests")) won't allow us to take advantage of the new arguments in run_tests(). So, instead, what I'm thinking is that we should move functions in "tests/testthat/functions.R" and "tests/testthat.R" to R/and export run_tests(). That way, we could do devtools::load_all(); run_tests() for phantomjs and devtools::load_all(); run_tests("firefox") for firefox. If you like that idea, I'll do that next.

tdhock · 2015-01-31T23:15:02Z

sure, that sounds fine to include those functions in the package and export
them.

On Fri, Jan 30, 2015 at 6:48 PM, Carson notifications@github.com wrote:

@tdhock https://github.com/tdhock The way we currently run tests (e.g. devtools::load_all();
testthat::test_dir("tests")) won't allow us to take advantage of the new
arguments in run_tests(). So, instead, what I'm thinking is that we
should move functions in "tests/testthat/functions.R" and
"tests/testthat.R" to R/and export run_tests(). That way, we could do devtools::load_all();
run_tests() for phantomjs and devtools::load_all(); run_tests("firefox")
for firefox. If you like that idea, I'll do that next.

—
Reply to this email directly or view it on GitHub
#40 (comment).

cpsievert · 2015-03-25T22:33:40Z

Ah, yes, I usually inject a browser() instead of manually executing code in this case. So the workflow for adding a new test could be:

library(animint)
init_tests("firefox")
run_tests(filter = "new-test")

If tests fail, inject a browser() at the relevant spot in test-new-test.R, then

devtools::load_all()
run_tests(filter = "new-test")

cpsievert · 2015-03-25T22:42:00Z

And, actually for sake of autocomplete, these probably make more sense:

tests_init()
tests_run()
tests_exit()

cpsievert · 2015-03-27T03:53:27Z

@tdhock any idea what I can add/edit here so that Sys.which("phantomjs") works?

tdhock · 2015-03-27T12:42:34Z

which and Sys.which depends on your PATH environment variable which you can get from the shell via

thocking@silene:~$ echo $PATH
/home/thocking/R/datatable-foverlaps/bedtools2/bin:/home/thocking/R/phantomjs-2.0.0/bin:/home/thocking/bin:/home/thocking/.cabal/bin:/home/thocking/R/datatable-foverlaps/bedtools2/bin:/home/thocking/R/phantomjs-2.0.0/bin:/home/thocking/bin:/home/thocking/.cabal/bin:/home/thocking/bin:/usr/lib/lightdm/lightdm:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games
thocking@silene:~$

or in R via

> Sys.getenv("PATH")
[1] "/home/thocking/R/datatable-foverlaps/bedtools2/bin:/home/thocking/R/phantomjs-2.0.0/bin:/home/thocking/bin:/home/thocking/.cabal/bin:/home/thocking/bin:/usr/lib/lightdm/lightdm:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games"

so maybe you have to add something to the PATH ? it is weird if /usr/local/bin is not on the PATH by default, but you could add it via (before starting R)

thocking@silene:~$ export PATH=/usr/local/bin:$PATH

tdhock · 2015-03-27T12:44:00Z

.travis.yml

+  - sudo rm -rf /usr/local/phantomjs
+  - curl -L -O https://s3.amazonaws.com/travis-phantomjs/phantomjs-2.0.0-ubuntu-12.04.tar.bz2
+  - tar xjf phantomjs-2.0.0-ubuntu-12.04.tar.bz2
+  - sudo mv phantomjs /usr/local/phantomjs


you should probably put it under /usr/local/bin (/usr/local is probably not on the PATH)

cpsievert · 2015-03-27T15:37:48Z

Grr. This is frustrating. If I interactively source the testhat.R script, it seems to work, but it doesn't when I do R CMD check.

My current approach is to start an environment that stores testing info (as well as the remote driver) in tests_init(). To make this environment available to functions.R and the actual testing environment, I do the following. I have a feeling that R CMD check doesn't like that last bit...

tdhock · 2015-03-27T15:40:18Z

why not just store the remDr object in the global environment, as on the
master branch?

On Fri, Mar 27, 2015 at 11:37 AM, Carson notifications@github.com wrote:

Grr. This is frustrating. If I interactively source the testhat.R script,
it seems to work, but it doesn't when I do R CMD check.

My current approach is to start an environment that stores testing info

animint/R/testHelpers.R

Lines 23 to 24 in 35f279b

# initiate environment that will store important testing info

env <- new.env(parent = globalenv())

(as well as the remote driver) in tests_init(). To make this environment
available to functions.R and the actual testing environment, I do the
following

animint/R/testHelpers.R

Lines 80 to 81 in 35f279b

sys.source(file.path(testDir, "functions.R"), envir = envir)

testthat::test_dir(testDir, filter = filter, env = envir)

.
I have a feeling that R CMD check doesn't like that last bit...

—
Reply to this email directly or view it on GitHub
#40 (comment).

cpsievert · 2015-03-27T15:46:09Z

That's probably what I'll do next. I suppose it isn't a huge deal in this case, but I was trying to avoid that since (from my understanding) introducing objects into the global workspace is considered poor practice?

srvanderplas · 2015-03-27T15:47:02Z

Is it still poor practice if you remove them afterwards?

On Fri, Mar 27, 2015 at 10:46 AM, Carson notifications@github.com wrote:

That's probably what I'll do next. I suppose it isn't a huge deal in this
case, but I was trying to avoid that since (from my understanding)
introducing objects into the global workspace is considered poor practice?

—
Reply to this email directly or view it on GitHub
#40 (comment).

tdhock · 2015-03-27T15:51:24Z

In this case I consider it to be good practice to put remDr in the global
environment, since it simplifies the testing framework (we can assume there
is working remDr in any tests that need to control the browser).

On Fri, Mar 27, 2015 at 11:47 AM, Susan VanderPlas <notifications@github.com

wrote:

Is it still poor practice if you remove them afterwards?

On Fri, Mar 27, 2015 at 10:46 AM, Carson notifications@github.com wrote:

That's probably what I'll do next. I suppose it isn't a huge deal in this
case, but I was trying to avoid that since (from my understanding)
introducing objects into the global workspace is considered poor
practice?

—
Reply to this email directly or view it on GitHub
#40 (comment).

—
Reply to this email directly or view it on GitHub
#40 (comment).

cpsievert · 2015-03-30T01:13:38Z

OK, I think this is finally functional. Go ahead and see if you can find any weaknesses @tdhock.

This should make interactive testing a lot less painful. You can even switch back and forth between different browsers easily:

tests_init()
tests_init("firefox") # shuts down phantomjs before starting selenium server
tests_init() # shuts down selenium before starting phantomjs
tests_run()
tests_exit() # shuts down everything (even local file servers)

cpsievert · 2015-03-30T01:16:56Z

I'll update the wiki (hopefully tonight) and mention some things to be aware of when writing tests.

tdhock · 2015-03-30T13:37:45Z

R/testHelpers.R

+  if (!inherits(e, "try-error")) {
+    pids <- as.integer(e)
+    res <- c(res, tools::pskill(pids))
+  }


e could be an try-error message or a character vector of pids, right? I suggest to clarify using tryCatch:

tryCatch({ pid.lines <- readLines(pid_file(), warn=FALSE) pids <- as.integer(pid.lines) res <- c(res, tools::pskill(pids)) })

Yes.

That approach will display an error message if there is an error, right? I think it makes sense to fail silently in this case.

tdhock · 2015-03-30T13:41:52Z

This is great work, thanks a lot Carson.

Besides the minor comments above, I think everything is perfect. Merge it when you like +1

cpsievert · 2015-03-30T16:05:15Z

For posterity, here are a couple things that are still sub-optimal:

(1) Sys.sleep() non-sense that tries to stall certain calls, but may result in error if background processes are not up and running yet.

(2) My hack for running a "daemonized" file servr. servr has a better solution, but it clashes with RSelenium's navigate method.

Test

cpsievert added 6 commits January 26, 2015 10:18

Merge rotate test

8ccfb40

Merge conflicts

e1e93d9

Improve testthat.R implementation

ee97bbd

New approach to testing

1cebb86

Actually run tests

9555d17

Allow run_tests to be run under tests directory

29ca341

worthless

aea5c66

cpsievert mentioned this pull request Jan 30, 2015

Improve testing infrastructure #41

Closed

cpsievert added 2 commits January 30, 2015 16:38

Merge branch 'master' of github.com:tdhock/animint into test

2d5472e

Attempt to get tests working again

44ac7c4

servr bleeding edge

b3f6ee4

cpsievert added 15 commits February 2, 2015 14:55

Export run_tests()

596a74b

export(knit_print.animint)

b4ba943

Travis test

3ab2c68

Add dir argument

0093cab

Directory handling

10932b9

Better directory handling

0be41e5

debug

71df185

derp

9dc0cd0

Double derp

22e20ac

source functions

32262ed

export animintEnv; attempt to kill selenium on exit

0191db1

Use selenium as a backend regardless of browser

b1c6d37

Remove need to kill by port number

880a95f

Wait a bit longer

baf364b

Wait longer

2ca5b83

cpsievert added 6 commits March 26, 2015 19:43

Modularize testing functions

b556aba

Move R_TESTS to tests_init()

8caf08e

testthat.R gets sourced in tests/

d9eb2e7

Try installing phantomjs 2.0

50a82fa

sudo make me a sandwich

19dec59

someday I'll learn linux bash

b43b93c

tdhock reviewed Mar 27, 2015
View reviewed changes

Try /usr/local/bin

35f279b

cpsievert added 4 commits March 27, 2015 12:49

Go back to a global remote driver

5d01893

A more robust approach

530a1a2

inst/ gets collapsed when package is installed

58642f5

R_TESTS non-sense

d27438d

tdhock reviewed Mar 30, 2015
View reviewed changes

cpsievert added a commit that referenced this pull request Mar 30, 2015

Merge pull request #40 from tdhock/test

c0d87b4

Test

cpsievert merged commit c0d87b4 into master Mar 30, 2015

cpsievert deleted the test branch March 30, 2015 17:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Test #40

Test #40

cpsievert commented Jan 27, 2015

tdhock commented Jan 29, 2015

cpsievert commented Jan 30, 2015

cpsievert commented Jan 30, 2015

tdhock commented Jan 31, 2015

cpsievert commented Mar 25, 2015

cpsievert commented Mar 25, 2015

cpsievert commented Mar 27, 2015

tdhock commented Mar 27, 2015

tdhock Mar 27, 2015

cpsievert commented Mar 27, 2015

tdhock commented Mar 27, 2015

cpsievert commented Mar 27, 2015

srvanderplas commented Mar 27, 2015

tdhock commented Mar 27, 2015

cpsievert commented Mar 30, 2015

cpsievert commented Mar 30, 2015

tdhock Mar 30, 2015

cpsievert Mar 30, 2015

tdhock commented Mar 30, 2015

cpsievert commented Mar 30, 2015

Test #40

Test #40

Conversation

cpsievert commented Jan 27, 2015

tdhock commented Jan 29, 2015

cpsievert commented Jan 30, 2015

cpsievert commented Jan 30, 2015

tdhock commented Jan 31, 2015

cpsievert commented Mar 25, 2015

cpsievert commented Mar 25, 2015

cpsievert commented Mar 27, 2015

tdhock commented Mar 27, 2015

tdhock Mar 27, 2015

Choose a reason for hiding this comment

cpsievert commented Mar 27, 2015

tdhock commented Mar 27, 2015

cpsievert commented Mar 27, 2015

srvanderplas commented Mar 27, 2015

tdhock commented Mar 27, 2015

cpsievert commented Mar 30, 2015

cpsievert commented Mar 30, 2015

tdhock Mar 30, 2015

Choose a reason for hiding this comment

cpsievert Mar 30, 2015

Choose a reason for hiding this comment

tdhock commented Mar 30, 2015

cpsievert commented Mar 30, 2015