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

Improve code coverage #71

Open
tklebel opened this issue Oct 1, 2018 · 5 comments
Open

Improve code coverage #71

tklebel opened this issue Oct 1, 2018 · 5 comments

Comments

@tklebel
Copy link
Member

tklebel commented Oct 1, 2018

Test coverage is good (91% atm), but it could be better.

A good place to start would be to implement a proper test for the print-method of jst_define_import. I have drafted a test (the last one in https://github.com/ropensci/jstor/blob/master/tests/testthat/test-import-spec.R, which is currently skipped), but it depends on the console width and is thus not working. A good version of the test would also check that the colors set with the crayon package are working.

Another option would be to work on the functions for re-importing files. This involves working with the file system (checking that files are being written or being deleted), but it should also be manageable.

@starship9
Copy link
Contributor

starship9 commented Oct 5, 2018

I'd like to take this, but I'm totally new to contributing to open source projects, along with being new to concepts like testing in R.

@tklebel
Copy link
Member Author

tklebel commented Oct 5, 2018

Great, thanks for coming forward! :)

If you are not familiar with testing in R, you could read this chapter: http://r-pkgs.had.co.nz/tests.html It will serve as a good starting point.

I see from your GitHub profile, that you are in principle familiar with using git and GitHub. The general approach would be to fork the repository, clone it to your disk, add tests, commit them and create a pull request to the original repo. We would then work together on improving your pull, if necessary.

After you have read the chapter on testing mentioned above, I would recommend that you try to implement a test for lines 161 and 162 of jst_re_import.

The file where you have to add your test is this one: https://github.com/ropensci/jstor/blob/master/tests/testthat/test-re-import.R

To find out, which lines are covered by tests and which still need one, you click on the code coverage badge in the README and then look into the specific files, like the one mentioned above.

After you have implemented a test for the lines 161 and 162, you could either create a pull and ask me to review and merge your changes, or you could try to implement more tests. This is entirely up to you. I'm happy for every test you manage to implement!

Feel free to ask any further question you may need answered for your first pull request to happen! 😀

@zaynaib
Copy link

zaynaib commented Oct 9, 2018

Can I also help on this issue?

@tklebel
Copy link
Member Author

tklebel commented Oct 9, 2018

@zaynaib great! What's your knowledge of R and testing? An easy start would to choose some lines which are not covered for reimporting files, and work on those. There is a first pull request (#72), but you could pick a few cases and work on those...

@zaynaib
Copy link

zaynaib commented Oct 10, 2018

I've never done R testing but I've done testing in other coding languages like python and javascript. I'll check out the resources that you posted earlier this week.

tklebel added a commit that referenced this issue Oct 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants