Write tests for fxns that don't have any yet

ropensci / rnoaa

R interface to many NOAA data APIs

https://docs.ropensci.org/rnoaa

Other

330 stars 84 forks source link

Write tests for fxns that don't have any yet #208

Open sckott opened 7 years ago

sckott commented 7 years ago

coverage in %

R/arc2.R: 0.00%
R/argo.R: 0.00%
R/caching.R: 0.00%
R/cmorph.R: 0.00%
R/defunct.R: 0.00%
R/ersst.R: 0.00%
R/homr_definitions.R: 0.00%
R/meteo_cache.r: 0.00%
R/meteo-autoplot.R: 0.00%
R/ncdc_combine.r: 0.00%
R/onload.R: 0.00%
R/tornadoes.R: 0.00%
R/utils.r: 0.00%
R/vis_miss.R: 0.00%
R/swdi.r: 1.64%
R/isd_stations.R: 33.33%

jonmcalder commented 7 years ago

Given that I'm not a beginner, but am still fairly new to API packages, would it be considered poor form if I got started on a few of these?

Also just to clarify, should all tests that involve API requests be skipped on CRAN, Travis and Appveyor and just tested locally? Or should certain tests be allowed to run on Travis and/or Appveyor initially and then 'skipped' later?

sckott commented 7 years ago

thanks @jonmcalder for dropping by.

I say go for it.

all that make web requests should be skipped on cran - but for the most part all can be run on the CI's

note that: on pull requests, if there are any environment variables (e.g., for ncdc() fxn) they aren't avail. to pull requests, so any that depend on env vars may fail, but i think ncdc_* fxns are the only ones affected

jonmcalder commented 7 years ago

Cool - thanks for the quick response!

kev-ho commented 7 years ago

Is there any resources you'd recommend reading or guidelines that should be followed in developing these tests?

On Tue, 10 Oct 2017 at 04:51 Jon Calder notifications@github.com wrote:

Cool - thanks for the quick response!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ropensci/rnoaa/issues/208#issuecomment-335300455, or mute the thread https://github.com/notifications/unsubscribe-auth/AH0fWnxfiui8fTOJS_lWVIcz1Zsl-nVeks5sqpVmgaJpZM4MjUTT .

jonmcalder commented 7 years ago

Hi @grandpotato! Hopefully @sckott will chime in with some more concrete suggestions here, but I'd say you could start by looking at the existing tests in tests/testthat and trying to emulate them.

Also read through Hadley's chapter on tests in R packages if you haven't already.

And if you're gonna work on this, could you let me know which functions you're tackling so we don't double up on anything? I've already started working on some of the earlier functions in the above list, so my suggestion would be to split the list in two and then you can work on the second half if you're happy with that?

sckott commented 7 years ago

What @jonmcalder covers it I think.

Some requests from my side:

try to write tests that take as little time as possible so that test suite runs as quickly as possible - i know I can do better on this front
good to know if tests run on all platforms, which will happen on Travis when doing a PR

let me know if you have any questions

kev-ho commented 7 years ago

@jonmcalder Good idea on not doubling up. I'm thinking I'll just start working from the bottom of the list and see how I go. Splitting it in half makes me feel accountable. And that's too much for me right now haha

jonmcalder commented 7 years ago

Cool - no pressure.

brooklynbagel commented 6 years ago

Are there still any functions missing tests? I've done some package development and am familiar with testthat.

jsta commented 5 years ago

@brooklynbagel I think running goodpractice::gp() will tell you where tests are missing.

sckott commented 5 years ago

yep, or devtools::test() or similar