search

phoible / dev

PHOIBLE data and development.
https://phoible.org/
GNU General Public License v3.0
115 stars 30 forks source link

better testing #293

Closed drammock closed 4 years ago

drammock commented 4 years ago

Turns out that each test file needs a context() in order to return informative results, and it works better to wrap individual tests into test_that() quasi-functions. So I've done that here. I also updated the language code validity test to more accurately test what we want (i.e., we've decided we don't want NA iso codes or glottocodes anymore), combined the metadata-related tests into one file, and changed filenames to better reflect what is going on.

BTW, I've reported to SIL the problem with the ISO 639 tab-delimited file that is causing a parse warning in the test for ISO code validity.

drammock commented 4 years ago

OK @bambooforest this is ready for review. Test output is now way more helpful; for example:

test-inventory-metadata.R:47: failure: language codes are valid
INVALID ISO CODES:
daf duj wit kxl krm kxu drh NA dit mwd qgu bjd ggr yiy

test-inventory-metadata.R:52: failure: language codes are valid
INVALID GLOTTOCODES:
NA

is now printed in the test output.

bambooforest commented 4 years ago

@drammock -- many thanks! this is super helpful and i've learned some tricks. thanks. merging.

only one thing that pops out at me wrt style. sometimes you use the :: library::function calls, e.g.:

readr::read_csv

other times not, e.g. pull. only noticed this because i was reading about readr this morning.

drammock commented 4 years ago

I tend to use package::func when I only need one or two things from a package. It helps me remember in which package those infrequently used functions reside, and keeps the global namespace cleaner. If I end up using something several times I'll usually go back and import the library. Do you have a strong preference for one way or the other?

bambooforest commented 4 years ago

No strong preference. Just struck me that some functions are prefixed and others aren't. I'm ok with a mix.

drammock commented 4 years ago

Yeah, I think a mix is good. I am definitely not going to write dplyr::%>% every time I want to pipe! But I often find myself wanting just one thing that I thought was in dplyr but turns out to be in readr or purrr or tidyr... Using the :: seems helpful in that case.