seafloor-geodesy / gnatss

Community Seafloor Global Navigation Satellite Systems - Acoustic (GNSS-A) Transponder Surveying Software
https://gnatss.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
10 stars 14 forks source link

test(loaders): Add test for load_deletions #127 #179

Closed madhavmk closed 11 months ago

madhavmk commented 11 months ago
codecov[bot] commented 11 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (d0f2bd0) 43.84% compared to head (21e43b3) 46.83%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #179 +/- ## ========================================== + Coverage 43.84% 46.83% +2.98% ========================================== Files 16 16 Lines 837 837 ========================================== + Hits 367 392 +25 + Misses 470 445 -25 ``` [see 1 file with indirect coverage changes](https://app.codecov.io/gh/uw-ssec/offshore-geodesy/pull/179/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uw-ssec)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

madhavmk commented 11 months ago

@lsetiawan This PR is ready for review.

lsetiawan commented 11 months ago

Thanks @madhavmk for this! This is great. However, you're missing the file_path case: https://app.codecov.io/gh/uw-ssec/offshore-geodesy/pull/179/blob/src/gnatss/loaders.py#L289. This is when the program is given the old deletns.dat file:

deletions:
    path: ./tests/data/2022/NCL1/deletns.dat
madhavmk commented 11 months ago

Thanks @madhavmk for this! This is great. However, you're missing the file_path case: https://app.codecov.io/gh/uw-ssec/offshore-geodesy/pull/179/blob/src/gnatss/loaders.py#L289. This is when the program is given the old deletns.dat file:

deletions:
    path: ./tests/data/2022/NCL1/deletns.dat

Right. I purposely skipped this testcase, as you mentioned we do not support it moving forward. Do you still recommend me to create a testcase for this?

lsetiawan commented 11 months ago

Right. I purposely skipped this testcase, as you mentioned we do not support it moving forward. Do you still recommend me to create a testcase for this?

That makes sense, we won't support it, but for now, let's test for this. It shouldn't take too much lift and this ensures that the program can be compared with an old delete file.

madhavmk commented 11 months ago

Right. I purposely skipped this testcase, as you mentioned we do not support it moving forward. Do you still recommend me to create a testcase for this?

That makes sense, we won't support it, but for now, let's test for this. It shouldn't take too much lift and this ensures that the program can be compared with an old delete file.

Completed!

lsetiawan commented 11 months ago

This is now ready to merge once the small change I made is passing. Thanks for this!