Try some basic unit tests #217

capnrefsmmat · 2020-10-31T03:04:47Z

WIP. Will write some more tests Soonish to see what happens (and how well our CI works).

capnrefsmmat · 2020-10-31T03:07:42Z

Apparently there's a nice package for testing plotting, so we can also do that to avoid regressing our plotting features: https://github.com/r-lib/vdiffr

capnrefsmmat · 2020-10-31T19:49:47Z

Ok, I've successfully set up mockery and vdiffr to do tests of functions that use the network (like covidcast_signal) and tests of our plotting/mapping functions. They work great on simple examples.

I also added DEVELOP.md with instructions on what to run and a PR checklist.

I think it'll be easy to make a few different plot examples (by saving a few real data frames) and save those as the key plotting tests. We could also add tests for issues like #134 when they're fixed.

cc @JedGrabman @ryantibs, so you can see how it will look to write tests of new features you're working on.

Set up vdiffr and mockery to do testing of plots and mocking of network requests.

capnrefsmmat · 2020-11-01T23:08:31Z

There are a lot more tests we should write, but I likely won't have time to write more for a while. So it's probably best if we merge this now, and then file an issue with a list of things we should add tests for. Then the testing framework will be in place for everyone to use for their new PRs, and we can fill in the testing gaps over the next few weeks.

ryantibs · 2020-11-02T06:09:35Z

This looks great, thank you @capnrefsmmat for starting us off with some tests! I won't be able to review this in any level of detail in the next few days, too bogged down with other things. Are any of @nmdefries @JedGrabman @sarah-colq @sgsmob @undefx @sangwon-hyun @nloliveira @capolitsch @brookslogan @bnaras available?

sgsmob · 2020-11-02T13:53:45Z

I can't assign myself to review this but I will

sgsmob

This will be a great asset to this repo!

Not entirely sure how to validate an initial commit of the charts but I think they are probably ok. Will approve once some more comments are added.

sgsmob · 2020-11-02T15:27:08Z

R-packages/covidcast/DESCRIPTION

    rmarkdown,
-    gridExtra
+    gridExtra,
+    testthat,


Please alphabetize these lists

sgsmob · 2020-11-02T15:41:23Z

R-packages/covidcast/tests/testthat/test-cor.R

+               expected)
+})
+
+test_that("basic lagged correlations", {


Please add a comment explaining the purpose of this test.

sgsmob · 2020-11-02T15:42:35Z

R-packages/covidcast/tests/testthat/test-cor.R

+               expected)
+})
+
+test_that("lags are in correct direction", {


Please add a comment explaining the purpose of this test.

sgsmob · 2020-11-02T15:44:19Z

R-packages/covidcast/tests/testthat/test-covidcast.R

+library(covidcast)
+library(mockery)
+
+test_that("covidcast_meta", {


Please add a comment explaining this test

sgsmob · 2020-11-02T15:45:13Z

R-packages/covidcast/tests/testthat/test-covidcast.R

+  stub(covidcast_meta, ".request",
+       list(message = "argle-bargle"))
+
+  expect_error(covidcast_meta())


Does this allow specification of which error is expected?

Not if covidcast_meta() uses stop(), which does not specify a specific error type -- unless you regexp-match against the error message.

I've just changed covidcast_meta() to use abort() with a specific error class, so we can catch and handle it, and the test can verify it was raised correctly.

sgsmob · 2020-11-02T15:46:37Z

R-packages/covidcast/tests/testthat/test-plot.R

+# https://github.com/r-lib/vdiffr/issues/71
+context("plot")
+
+test_that("simple line graph", {


Please leave a comment describing the purpose of this test

sgsmob · 2020-11-02T15:46:41Z

R-packages/covidcast/tests/testthat/test-plot.R

+  ))
+})
+
+test_that("state line graphs", {


Please leave a comment describing the purpose of this test

sgsmob · 2020-11-02T15:47:01Z

R-packages/covidcast/tests/testthat/test-plot.R

+                           range = c(0, 10)))
+})
+
+test_that("simple state choropleths", {


Please leave a comment describing the purpose of this test

sgsmob · 2020-11-02T15:48:12Z

R-packages/covidcast/tests/testthat/test-wrangle.R

+library(covidcast)
+library(dplyr)
+
+test_that("aggregate and shift", {


Please leave comments describing the purpose of the tests in this file

sgsmob · 2020-11-02T15:49:08Z

R-packages/covidcast/tests/testthat/test-wrangle.R

+                 value = c(4, 6, 5, 1, 3, 2)),
+                 class = c("covidcast_signal_long", "data.frame")))
+
+  # Now try it long in the first place. Currently fails because wide format does


If these tests are not working, they should just be removed from this file entirely.

capnrefsmmat · 2020-11-03T00:06:38Z

Pushed fixes. Rather than adding a bunch of comments to the tests, I tried to change their names so that when you read them, they're meaningful, such as

test_that("covidcast_meta raises error when API signals one", {

Also the plotting tests don't really have deep purposes besides making plots with different options, so we know if those options break or change; I added an explanation at the top of the test file.

capnrefsmmat · 2020-11-03T00:16:21Z

Also, regarding the commented-out test code in test-wrangle.R: I can use skip() to skip that instead of commenting out, but that also prints a message every time you run the tests. The issue is that I want to change the code to make that test pass, but it doesn't yet; would you prefer the code be removed entirely and returned when the issues is fixed, leave it commented out, or have it skipped and noisily complain to encourage people to fix the wrangling code?

sgsmob · 2020-11-03T14:30:28Z

Also, regarding the commented-out test code in test-wrangle.R: I can use skip() to skip that instead of commenting out, but that also prints a message every time you run the tests. The issue is that I want to change the code to make that test pass, but it doesn't yet; would you prefer the code be removed entirely and returned when the issues is fixed, leave it commented out, or have it skipped and noisily complain to encourage people to fix the wrangling code?

Adding a TODO in the comments preceding the commented out tests would be sufficient to leave them there commented out.

capnrefsmmat marked this pull request as draft October 31, 2020 03:07

capnrefsmmat added the covidcast R package label Nov 1, 2020

capnrefsmmat force-pushed the r-tests branch from e7d2472 to 2d59263 Compare November 1, 2020 18:04

capnrefsmmat mentioned this pull request Nov 1, 2020

Small R package fixes and improved documentataion #233

Merged

capnrefsmmat added 7 commits November 1, 2020 17:18

Try some basic unit tests

44ef126

Lay groundwork for further covidcast testing

265ca78

Set up vdiffr and mockery to do testing of plots and mocking of network requests.

Add a development guide

b0ae5a7

Mention NEWS.md

7897a2d

Additiona choropleth and line graph tests

2cb2542

Additional correlation/lag tests

5b5a899

Describe how to use test data files

c8cd62e

capnrefsmmat force-pushed the r-tests branch from 1a77dec to c8cd62e Compare November 1, 2020 22:19

capnrefsmmat added 2 commits November 1, 2020 17:51

New wrangling tests

23574e4

Mention categorizing functions in pkgdown docs

f8ac6b9

capnrefsmmat marked this pull request as ready for review November 1, 2020 23:07

capnrefsmmat requested a review from ryantibs November 1, 2020 23:07

JedGrabman mentioned this pull request Nov 2, 2020

Choropleth map plot does not work if all the state values are the same (R library) #134

Closed

capnrefsmmat requested review from sgsmob and removed request for ryantibs November 2, 2020 15:17

sgsmob suggested changes Nov 2, 2020

View reviewed changes

capnrefsmmat added 3 commits November 2, 2020 18:54

Alphabetize dependencies

bd61fd3

Improve test naming and comments

088cb18

Raise a specific error class when meta fetch fails

1369672

Better wrangle test names

4334ab2

capnrefsmmat requested a review from sgsmob November 3, 2020 00:16

capnrefsmmat mentioned this pull request Nov 3, 2020

Add additional correlation tests to covidcast #239

Open

6 tasks

sgsmob approved these changes Nov 3, 2020

View reviewed changes

Attach TODO to failing test

5ba6054

capnrefsmmat merged commit 9bf67da into main Nov 3, 2020

capnrefsmmat mentioned this pull request Nov 19, 2020

Write R tests for MSA and HRR plots #310

Closed

taylor-arnold deleted the r-tests branch March 19, 2021 20:23

Try some basic unit tests #217

Try some basic unit tests #217

Uh oh!

Conversation

capnrefsmmat commented Oct 31, 2020

Uh oh!

capnrefsmmat commented Oct 31, 2020

Uh oh!

capnrefsmmat commented Oct 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

capnrefsmmat commented Nov 1, 2020

Uh oh!

ryantibs commented Nov 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sgsmob commented Nov 2, 2020

Uh oh!

sgsmob left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

capnrefsmmat commented Nov 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

capnrefsmmat commented Nov 3, 2020

Uh oh!

sgsmob commented Nov 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

capnrefsmmat commented Oct 31, 2020 •

edited

Loading

ryantibs commented Nov 2, 2020 •

edited

Loading

capnrefsmmat commented Nov 3, 2020 •

edited

Loading

sgsmob commented Nov 3, 2020 •

edited

Loading