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 to testthat #49

Merged
merged 4 commits into from
Jul 10, 2015
Merged

Move to testthat #49

merged 4 commits into from
Jul 10, 2015

Conversation

dmpe
Copy link
Contributor

@dmpe dmpe commented Jul 8, 2015

6 tests will still continue to fail (bigger changes to the code required)
update description
#48

Given that users do not care about tests, I wouldn't have a problem in releasing such an update. Indeed, in order to pass all the tests, there are bigger changes required e.g. this message is a problem to testthat (not 100% sure how to fix yet)

read.socrata("https://soda.demo.socrata.com/api/views/433-bgaj") does not match '3 characters before dash instead of 4'. Actual value: "Error in validateUrl(url, app_token) : \n  433-bgaj is not a valid Socrata dataset unique identifier.\n"

Similar to the message below:

2015-07-08 19:55:02.200 getResponse: Error in httr GET: 401  http://data.cityofchicago.org/resource/j8vp-2qpg.json

@tomschenkjr
Copy link
Contributor

It's so great to finally see code coverage results. I can work on the 6 tests that failed. I like to keep the integrity of the unit tests, but think it can be sorted before the push this weekend.

Below is a quick list (mostly for myself) to follow-up on after my very cursory overview:

  • Investigate error on Travis CI build. It failed at an unexpected spot.
  • Fix the unit tests that are unexpectedly failing. See output in Appveyor.
  • Quick update to the coveralls badge to flat/square and inline with other badges.

@dmpe
Copy link
Contributor Author

dmpe commented Jul 9, 2015

Good. You may also try to restart the build (upper right corner https://travis-ci.org/Chicago/RSocrata/builds/70104458)

@tomschenkjr
Copy link
Contributor

@dmpe good point, rerun fixed the Travis hiccup. Thanks.



test_that("is 4x4 URL", {
expect_error(read.socrata("https://soda.demo.socrata.com/api/views/4334c-bgajc"), "11 characters instead of 9")
Copy link
Contributor

Choose a reason for hiding this comment

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

I see the issue, just some differences in the RUnit approach vesus testthat. For instance, this test passes with the syntax:

test_that("is 4x4 URL", {
  expect_error(read.socrata("https://soda.demo.socrata.com/api/views/4334c-bgajc"), "4334c-bgajc is not a valid Socrata dataset unique identifier", label="11 characters instead of 9")
  expect_error(read.socrata("https://soda.demo.socrata.com/api/views/433-bga"), "433-bga is not a valid Socrata dataset unique identifier", label="7 characters instead of 9")
  expect_error(read.socrata("https://soda.demo.socrata.com/api/views/433-bgaj"), "433-bgaj is not a valid Socrata dataset unique identifier", label="3 characters before dash instead of 4")
  expect_error(read.socrata("https://soda.demo.socrata.com/api/views/4334-!gaj"), "4334-!gaj is not a valid Socrata dataset unique identifier", label="non-alphanumeric character")
})

I will make changes to the tests that fail.

@tomschenkjr tomschenkjr merged commit 4b124e2 into Chicago:dev Jul 10, 2015
@dmpe dmpe deleted the dev branch July 10, 2015 08:59
@dmpe dmpe restored the dev branch July 11, 2015 08:09
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