search

ropensci / osmextract

Download and import OpenStreetMap data from Geofabrik and other providers
https://docs.ropensci.org/osmextract
GNU General Public License v3.0
170 stars 12 forks source link

Revamp tests #255

Closed agila5 closed 2 years ago

agila5 commented 2 years ago

Revamp the existing tests using the so-called test fixtures: https://testthat.r-lib.org/articles/test-fixtures.html

The main benefit of the new approach is that each test is explicitly run in a clean and independent session (so each time the PBF file is copied to the tempdir() and removed at the end of the test). That helped me a lot to detect problems in the old version of the tests. The main con is that this might be extremely difficult to modify in a few months, let's see 😅

agila5 commented 2 years ago

@Robinlovelace I know that it might be difficult to understand all the stuff, but if you have time to review, please do

Robinlovelace commented 2 years ago

@Robinlovelace I know that it might be difficult to understand all the stuff, but if you have time to review, please do

Looks like some serious changes to the testing code, and if it allows tests to run in a more isolated environment that's good. However I don't feel I have the experience to review this, could we ask someone from rOpenSci to take a look?

agila5 commented 2 years ago

could we ask someone from rOpenSci to take a look?

If you know anybody that would like to review the PR please ask, otherwise I think we can simply merge and then worry when one of the tests breaks.

The main changes are summarised in the setup_pbf() function which is used to run each test in a separated environment. Moreover, the calls to withr::local_envvar are used to temporarily modify the oe_download_directory().

agila5 commented 2 years ago

Any news here?

Robinlovelace commented 2 years ago

None from me but happy for this to be merged.