ropensci / geojsonio

Convert many data formats to & from GeoJSON & TopoJSON
https://docs.ropensci.org/geojsonio
Other
151 stars 59 forks source link

Testthat 3e: better test isolation #189

Closed czeildi closed 1 year ago

czeildi commented 1 year ago

Try to follow newer best practices, mostly related to https://testthat.r-lib.org/articles/test-fixtures.html

Description

The changes are across these general themes:

Note that to run tests interactively, devtools::load_all() is needed, which also loads helper files

I added one suggested dependency: withr which is used in tests. I believe that it is a relatively light and stable dependency, but adds safety to cleanups.

Related Issue

Closes #183

czeildi commented 1 year ago

This is a bigger PR but I hope that the individual commits have a scope that is digestable. That said, tell me if I should split this up into multiple PRs, or simply revert some of the changes.

czeildi commented 1 year ago

I converted to draft PR as there are check failures on Windows, will look into them.

mikemahoney218 commented 1 year ago

Thank you so much! This is absolutely fantastic. Having it all in one PR is fine, and adding withr to Suggests is fine by me.

I might take a little while to review this PR -- it's a particularly busy period for my day job. That said, this looks fantastic based on my quick skim. Thank you again!

github-actions[bot] commented 1 year ago

This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.