search

Teal-Insights / r-wbids

R package to access and analyze World Bank International Debt Statistics (IDS)
https://teal-insights.github.io/r-wbids/
Other
3 stars 0 forks source link

Add test-coverage.yaml workflow #33

Closed christophscheuch closed 2 days ago

christophscheuch commented 3 days ago

Includes updating the tests to get 100% coverage. Current status:

wbids Coverage: 100.00% R/ids_bulk.R: 100.00% R/ids_bulk_files.R: 100.00% R/ids_bulk_series.R: 100.00% R/ids_get.R: 100.00% R/ids_list_counterparts.R: 100.00% R/ids_list_geographies.R: 100.00% R/ids_list_series_topics.R: 100.00% R/ids_list_series.R: 100.00% R/perform_request.R: 100.00% R/read_bulk_info.R: 100.00%

With this PR, we should also get a 100% codecov badge as here: https://github.com/tidy-intelligence/r-wbwdi

christophscheuch commented 3 days ago

This PR got bigger than expected because I decided to do some ad hoc refactorings to make code test coverage work somehow and because I discovered redundancies and potential for improvements. There is only a small part in the ids_bulk() function where I just gave up (see the #nocov tags).

My biggest learning is that from now on, every PR should also include a check for test coverage. It really helps to remove redundant code and question the implementation. I created a dev workflow write-up here that includes the process: https://github.com/Teal-Insights/r-wbids/wiki/Development-Workflow.

I guess the PR is fine since all tests and examples pass. Maybe you can just glance over it @chriscarrollsmith if you find something fishy.

chriscarrollsmith commented 3 days ago

Wow, this is a pretty kick-ass PR! Great work! And thank you for the documentation, too.

christophscheuch commented 3 days ago

Is there a reason you prefer the cli library's stop, message, and warning functions over the standard base R ones?

cli has better formatting and more options than base and it basically comes as a "free depedency" anyway if you use dplyr or tidyr.