Skip to content

Implement testing across the package #42

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

Closed
ryantibs opened this issue Feb 13, 2022 · 7 comments
Closed

Implement testing across the package #42

ryantibs opened this issue Feb 13, 2022 · 7 comments
Assignees
Labels
cleanup improvements to developing&building experience&quality, not directly to built pkg&docs good first issue Good for newcomers help wanted Extra attention is needed P0 high priority

Comments

@ryantibs
Copy link
Member

Now that the package is getting farther along, I think it is about time we implement testing across the package. Especially because I believe (and hope) that more others will be contributing to it in the near future.

@capnrefsmmat You are our resident stickler-for-the-rules-kind-of-guy, did you want to take the lead on this? You know I'm kidding 😛, and I would be grateful if you took this on. I think @qpmnguyen @rafaelcatoia could be willing to help as well, so if you have some ideas/scaffolding then perhaps they would be willing to implement it with you.

@qpmnguyen
Copy link
Contributor

Hello! Happy to help write unit tests!

@ryantibs
Copy link
Member Author

@qpmnguyen @rafaelcatoia Did you guys want to start taking a look at this? You can see the way that testing is implemented in the covidcast R package (for example) and also read up on testing in R in general if you haven't had exposure to this already.

In side conversation with @capnrefsmmat, he said he is busy for a little bit with other responsibilities, but can start taking a look perhaps mid March, so in any case I think it makes sense for Quant & Rafael to just start learning/putting things together in the meantime, if you're still up for it. Thanks!

p.s. in the advanced.Rmd vignette, I have some code chunks with include = FALSE that were basically just some hand-tests I was running to make sure the results were what I expected with sliding. In building tests, you could remove these chunks from that vignette (they're not needed there anymore, they're just in there so I didn't forget them) and incorporate them into the tests you're writing.

@dajmcdon
Copy link
Contributor

@dshemetov @qpmnguyen @rafaelcatoia Is anyone actively working on this? I'm going to re-assign if no.

@dshemetov
Copy link
Contributor

I'm not.

@qpmnguyen
Copy link
Contributor

I am not currently working on this. Feel free to re-assign!

@brookslogan brookslogan added the cleanup improvements to developing&building experience&quality, not directly to built pkg&docs label May 31, 2022
@kenmawer kenmawer self-assigned this Jun 24, 2022
@dajmcdon dajmcdon closed this as completed Jul 8, 2022
@kenmawer
Copy link
Contributor

kenmawer commented Jul 12, 2022

I just realized that I misread what tests need to be done; the following do not have tests:
archive.R
growth_rate.R
methods-epi_archive.R (includes epix_slide.R, not to be confused with epi_slide.R)

@kenmawer kenmawer reopened this Jul 12, 2022
@brookslogan
Copy link
Contributor

Tests for the above have now been addressed in PRs (e.g., #159, #197); closing as completed. We may want to push on #204 to catch anything else hiding untested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup improvements to developing&building experience&quality, not directly to built pkg&docs good first issue Good for newcomers help wanted Extra attention is needed P0 high priority
Projects
None yet
Development

No branches or pull requests

8 participants