KentonWhite / ProjectTemplate

A template utility for R projects that provides a skeletal project.
http://projecttemplate.net
GNU General Public License v3.0
623 stars 159 forks source link

Test throws Errors if Context(file-name.R) not added #265

Closed jflanaga closed 6 years ago

jflanaga commented 6 years ago

Report an Issue / Request a Feature

I'm submitting a (Check one with "x") :


Issue Severity Classification -

(Check one with "x") :

Expected Behavior

test.project() returns same information as expect_that.

Current Behavior

test.project() returns Error in x[[method]](...) : attempt to apply non-function

✔ | OK F W S | Context
⠙ |  2       | 0Error in x[[method]](...) : attempt to apply non-function

══ Results ════════════════════════════════════════════════════════════════════════════════════════════
OK:       2
Failed:   3
Warnings: 0
Skipped:  0
Steps to Reproduce Behavior

Just run a test that fails

expect_that(1+1, equals(3))

Screenshots
Version Information

Latest version

Possible Solution

If you add context(name_of_file) before the tests, it works as expected.

This has been reported in testthat

KentonWhite commented 6 years ago

It looks like a proper testthat test should always start with context(). What I can do here is modify the example test/1.R to include the context file. Would that help? I could add a global context("ProjectTemplate"), but this goes against the idea of context(), which is grouping tests together. Do you have an opinion for why a global context would make sense?

jflanaga commented 6 years ago

I'm still trying to get my head around what a proper testthat test should look like, so I don't have an opinion about it. I think your initial proposal makes sense.

Hugovdberg commented 6 years ago

Perhaps a context("Example tests") would be better. This makes extra clear the file is initially just used for examples. People can always change it afterwards. The context ProjectTemplate suggests it contains tests for the actual ProjectTemplate functionality.

On a side note, expect_that is deprecated, you should use expect_equal(1+1, 3) in your case, or one of the other more explicit expect_* functions in other cases. The fallback should be expect_true or expect_false if none of the other functions suffice.

KentonWhite commented 6 years ago

Updated example test to use testthat best practices.