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

Move from RUnit to testthat #48

Closed
3 tasks done
dmpe opened this issue Jul 8, 2015 · 8 comments
Closed
3 tasks done

Move from RUnit to testthat #48

dmpe opened this issue Jul 8, 2015 · 8 comments

Comments

@dmpe
Copy link
Contributor

dmpe commented Jul 8, 2015

For discussion:

Instead of

test.readSocrataToken <- function(){
  df <- read.socrata('https://soda.demo.socrata.com/resource/4334-bgaj.csv', app_token="ew2rEMuESuzWPqMkyPfOSGJgE")
  checkEquals(1007, nrow(df), "rows")
  checkEquals(9, ncol(df), "columns")  
}

will become

testthat("Socrata Token is read and df is returned", {
  df <- read.socrata('https://soda.demo.socrata.com/resource/4334-bgaj.csv', app_token="ew2rEMuESuzWPqMkyPfOSGJgE")
  expect_equal(ncol(df), 9, "columns")
  expect_equal(nrow(df), 1007, "columns")
})

Advantage:

  • The package is being developed, by Hadley : https://github.com/hadley/testthat. Whereas RUnit is "ORPHANED" (I mean, for me, being "ORPHANED" means that it has no "real" future, similar to RJSONIO)
  • Nicer syntax 😏
  • Can be very easily tested in RStudio & devtools -> devtools::test()

In case of agreement, please assign me. Thanks

@tomschenkjr
Copy link
Contributor

I'm open to it. RUnit has worked well, but been more problematic when trying to get it to work with coveralls. In part, I have less examples to refer to since testthat is far more popular.

The only thing that has stopped me is my lack of familiarity with testthat and taking the time to implement it.

@dmpe
Copy link
Contributor Author

dmpe commented Jul 8, 2015

Great to hear that!
No worries, I am not the most skilful R developer (beginner; just developing RBitly and WufooR), but as I said just assign me and I contribute to that.

BTW: In RStudio this will look like this:

==> devtools::test()

Loading WufooR
Testing WufooR
Fields : .
Reports : .
Forms : ...
Users : .

DONE 

@tomschenkjr
Copy link
Contributor

No problem, it'll be great to have you contribute to this and happy to help with any questions. Sorry, I can't assign you this specific issue since you're not a member on our org account, but please feel free to proceed.

One other item to note is it's useful to move the R/RSocrata/tests/ to root.

We could put this in a yet-to-be-announced 1.6.2 release. I have to submit a bug-release of the package (1.6.1) sometime this weekend because CRAN has been hounding me. If you finish before then, I could wrap it into 1.6.1, but no rush.

@dmpe
Copy link
Contributor Author

dmpe commented Jul 8, 2015

(#47 would be then for later. OK. ) - #42

Yeeh. I will do it. Please, also, close issues which should be closed. E.g #23, #28, #40

Thanks!

@dmpe dmpe mentioned this issue Jul 8, 2015
dmpe added a commit to dmpe/RSocrata that referenced this issue Jul 8, 2015
@dmpe dmpe closed this as completed Jul 10, 2015
@dmpe dmpe reopened this Jul 10, 2015
@dmpe
Copy link
Contributor Author

dmpe commented Jul 10, 2015

(4 test locally for me still failing)

posixify function : ..............
read Socrata : .......2015-07-10 10:11:35.672 getResponse: Error in httr GET: 400 http://soda.demo.socrata.com/resource/4334-bgaj.csv?%24select=Region
.2015-07-10 10:11:36.228 getResponse: Error in httr GET: 401 http://data.cityofchicago.org/resource/j8vp-2qpg.json
....
Socrata Calendar : ..............
Checks the validity of 4x4 : ..........
Test Socrata with Token : ..........2015-07-10 10:11:43.451 getResponse: Error in httr GET: 400 https://soda.demo.socrata.com/resource/4334-bgaj.csv?%24app_token=ew2rEMuESuzWPqMkyPfOSGJgE
...2015-07-10 10:11:44.659 getResponse: Error in httr GET: 400 https://soda.demo.socrata.com/resource/4334-bgaj.csv?%24app_token=ew2rEMuESuzWPqMkyPfOSGJgE

@dmpe
Copy link
Contributor Author

dmpe commented Jul 10, 2015

@tomschenkjr In addition, I see you have put
(https://img.shields.io/coveralls/ dmpe /RSocrata/dev.svg?style=flat-square&label=Coverage status - Dev)](https://coveralls.io/r/Chicago/RSocrata?branch=dev)

which needs to be replaced with Chicago too. :)

commit already in the #47

dmpe added a commit to dmpe/RSocrata that referenced this issue Jul 10, 2015
@tomschenkjr
Copy link
Contributor

@dmpe Ok, I see that I get the error when running devtools::test(), but it's not exit 1 when running Rbuild nor being caught by either CI test.

It appears to be how httr handles errors. Per your comment in #50, I think we can improve this in a later release. Unless you have any issues, I'll close this issue in the meantime as we've effectively transferred to testthat.

@dmpe
Copy link
Contributor Author

dmpe commented Jul 11, 2015

@tomschenkjr Fine with me. I close it.
thanks

@dmpe dmpe closed this as completed Jul 11, 2015
@dmpe dmpe mentioned this issue Jul 11, 2015
4 tasks
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

No branches or pull requests

2 participants