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

Additional expectations #77

Closed
wants to merge 9 commits into from
Closed

Additional expectations #77

wants to merge 9 commits into from

Conversation

jknowles
Copy link

I think it would be more convenient if three additional expectations were available for numeric values, chiefly: is_less_than is_more_than and unequal. I have coded these up and written test for them to ensure they work.

I know this functionality is available by having the user write something like:

result <- foo(param1)
test1 <- result > expectation

test_that("Values are sensible", {
expect_true(test1)
})

But to me this is unsatisfying and it would be preferable for the user to do the following:

test_that("Values are sensible", {
expect_more_than(foo(param1), 100)
expect_less_than(foo(param1), 200)
expect_unequal(foo(param1), 159)
})

This makes testing even simpler and more straightforward as it allows tests to be written with floor and ceiling values, which are often easier to think of than exact equalities in R.

But, I understand this might be considered cluttering up the code base unnecessarily. I use these functions in my own local testthat, but thought I would send them upstream and see if they are worth inclusion in the package.

@hadley
Copy link
Member

hadley commented May 29, 2013

Thanks Jared - I'll take a look when I'm next working on testthat.

@jknowles
Copy link
Author

jknowles commented Jun 4, 2013

Just added a quick additional test that allows the user to check actual value with a user specified tolerance toward the expected value.

@@ -0,0 +1,5 @@
.Rproj.user
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please remove this file from the pull request?

@hadley
Copy link
Member

hadley commented Jun 7, 2013

As well as the smaller changes described inline, can you please add success messages in line with the new expectation function? Thanks!

@jknowles
Copy link
Author

jknowles commented Jun 7, 2013

I'm not sure this did it. I couldn't quite figure out how to re-run all the package tests after making changes. Testing the testthat package is a bit meta and it's been awhile since I made the last changes and figured it out.

Hope this cleans things up a bit!

@hadley hadley closed this in 5d11e65 Jan 23, 2014
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