search

NOAA-EMC / NCEPLIBS-bufr

The NCEPLIBS-bufr library contains routines and utilites for working with the WMO BUFR format.
Other
44 stars 19 forks source link

eliminate duplication of replication sequences within stseq function #617

Closed jbathegit closed 1 month ago

jbathegit commented 1 month ago

Fixes #612

Also eliminates dlloctbf_c binding which is no longer needed.

jbathegit commented 1 month ago

Hey @jack-woollen @edwardhartnett @AlexanderRichert-NOAA would any of you possibly have some time during the next couple of days to take a look at this?

(Thanks! :-)

jbathegit commented 1 month ago

How easy would it be to add remaining code coverage for stseq.c? I don't think you're technically adding new untested lines of code, but there are some untested lines that got shuffled around a bit, if I'm reading the diff correctly. Otherwise looks good.

Thanks Alex and Jack for your replies!

Alex, you're correct that I'm not adding any untested lines with this PR, and that all of the untested lines in stseq() are ones that previously existed. But many of those aren't easy to test, either because they're hard to simulate (e.g. if a malloc fails, or if pktdd() somehow encounters a corrupt counter value) or because I've never actually seen any cases of NWP centers generating sample data with some of these more esoteric BUFR features (e.g. substituted values, replaced/retained values) even though they are theoretically possible according to the WMO regulations.

If and when any of that changes, then I'll certainly be more than willing to add new test cases. But for now at least the testing coverage is as complete as I can manage for stseq().

edwardhartnett commented 1 month ago

Indeed in Fortan and C it is not usual to test for such error conditions. There's no reasonable way to make malloc fail, but you should certainly have code to deal with a malloc failure. So that code will be impossible to reach in testing.

In other languages, like Python, there is a concept of "mocking" which allows us to easily reach 100% test coverage. But there's no easy way to do that in Fortran/C, so I'm happy to achieve something over 80% with Fortran/C code. And I believe NCEPLIBS-bufr is well into the 90s now...