Add unit tests

[benmarwick]

We should have unit tests for the core functions in this package and their variants.

This will be a bit different from usual tests because they output files and directories, and some do things on other services that we may not be able to do with Travis.

[nevrome]

I though about this and experimented with a setup to test the package manipulating functions without interfering with anything.

Here's what I came up with (not production ready yet). All the tests can happen within a temporary package that's gradually extended by and for the functions to be tested.

I already found a simple bug in add_dependencies_to_description() while writing this test. Not a good sign... Testing is quite useful.

[nevrome]

I just sat down to write some tests. Unfortunately many of the functions call yesno() at one point down the line to ask the user for her/his preferences. I'm not aware of a good way to bypass this for the test setup. Probably we have to add option parameters for some (many) functions to make them run non-interactively.

Out of curiosity: Do you know a way to simulate user input for yesno(), @joethorley? Would be convenient for tests...

[joethorley]

I’ve never looked for a way to simulate input. With my functions I generally have an argument ask = TRUE which I set to be FALSE when running tests or for non-interactive scripts - if ask = FALSE the code bypasses each question and assumes the answer is yes.

Sent from my iPhone

On Dec 27, 2017, at 09:34, Clemens Schmid notifications@github.com wrote:

I just sat down to write some tests. Unfortunately many of the functions call yesno() at one point down the line to ask the user for her/his preferences. I'm not aware of a good way to bypass this for the test setup. Probably we have to add option parameters for some (many) functions to make them run non-interactively.

Out of curiosity: Do you know a way to simulate user input for yesno(), @joethorley https://github.com/joethorley? Would be convenient for tests...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/benmarwick/rrtools/issues/43#issuecomment-354147934, or mute the thread https://github.com/notifications/unsubscribe-auth/AAldJ1RUa3F8R2KiAFZfY-I4TvmQS2Zgks5tEn-2gaJpZM4QJte2 .

[nevrome]

@joethorley Thanks for the input! I think this is the way to go. What do you say, @benmarwick?

[benmarwick]

That sounds like a great idea, thanks. I will add this ask = TRUE logic to our functions that use yesno() and update you here when it's done so we can continue discussing the tests.

[benmarwick]

Another option may be to use interactive(), with yesno(). I have added this in one place, https://github.com/benmarwick/rrtools/blob/master/R/infrastructure-git.R#L124, but got distracted by a different question before I could explore its effectiveness.

I merged in the tests you'd started on, and your add_dependencies_to_description() works great, thanks for that.

I made a start on a test for use_analysis(), thinking it would be simple, and bumped into a problem that I think originates here, https://github.com/benmarwick/rrtools/blob/master/R/hello.R#L465, where we use file.create() and file.copy(). These don't work as expected in the test, so I need to look into that a bit more.

[nevrome]

Ah - didn't know about interactive(). Good to know.

If you want I could write some tests for add_dependencies_to_description(), use_readme_rmd.R and use_travis.R. You could add some for use_analysis(), use_dockerfile() and use_circleci(). Like this the main functions are covered and we share the burden.

As discussed on twitter I still suggest to slot a basic check in on travis before the docker build. It should be possible to just add some lines like those. I will try to prepare a PR.

[nevrome]

If we add ask = TRUE we could also add quiet = FALSE to allow message suppression. That's useful for any kind of automation. Currently I solved it in the tests with suppressMessages().

[benmarwick]

Thanks very much, yes, that sounds like a good idea to add quiet as you suggest.

Can I now merge your #52 so I can get this commit that fixes the file path problem I noted earlier, and add some more tests to the ones that you've contributed?

[nevrome]

Yes - that should be possible. I'm pretty confident, that I didn't break anything. I'm trying to add another PR for the tests and changes for use_travis.R after dinner. For this I have to implement the ask = TRUE solution at some points.

[nevrome]

Oh - maybe except for the travis thing. That's highly experimental. Please check before merging.