adokter / bioRad

R package for analysis and visualisation of biological signals in weather radar data
http://adokter.github.io/bioRad
Other
29 stars 16 forks source link

Re-enable tests for read_stdout() #601

Closed peterdesmet closed 1 year ago

peterdesmet commented 1 year ago

Tests for read_vpts() were added by @PietrH. As part of #590 that function is renamed to read_stdout(), causing a merge conflict with between the tests for the new read_vpts() and those intended for read_stdout(). This resulted in the latter test being removed in https://github.com/adokter/bioRad/commit/3358d976e39fefb6e1d8eac80fdf1b93faf9665e.

This PR re-enables those tests, now calling read_stdout(). I notice however that some of the tests fail. I think this is due to the fact the tests expecting a specific error are order sensitive. E.g. a test for read_stdout(vptsfile, lat = "a") expects an error related to the latitude not being an integer, but can also fail because the radar is missing. I would update those test to provide all the necessary parameters except the one we need to test, so read_stdout(vptsfile, radar = "KGBM", lat = "a").

@PietrH since you worked on these test, would you be willing to correct them?

codecov[bot] commented 1 year ago

Codecov Report

Merging #601 (fda5556) into master (ca7e2f8) will increase coverage by 0.41%. The diff coverage is 87.43%.

:exclamation: Current head fda5556 differs from pull request most recent head 32a1679. Consider uploading reports for the commit 32a1679 to get more accurate results

@@            Coverage Diff             @@
##           master     #601      +/-   ##
==========================================
+ Coverage   81.53%   81.95%   +0.41%     
==========================================
  Files          59       60       +1     
  Lines        3537     3586      +49     
==========================================
+ Hits         2884     2939      +55     
+ Misses        653      647       -6     
Impacted Files Coverage Δ
R/read_stdout.R 83.58% <83.58%> (ø)
R/read_vpts.R 96.15% <97.67%> (+14.40%) :arrow_up:
R/utils.R 85.71% <100.00%> (ø)

... and 1 file with indirect coverage changes

PietrH commented 1 year ago

@peterdesmet , I believe I have resolved all testing woes.

Could you have a look at tests/testthat/helper.R before merging? I believe a helper should not change the testing environment.

peterdesmet commented 1 year ago

I have changed the base for this PR to #590. The helper function can be tackled there.