nasa / CF

The Core Flight System (cFS) CFDP application.
Apache License 2.0
77 stars 45 forks source link

Remove use of random numbers in coverage test #86

Open jphickey opened 2 years ago

jphickey commented 2 years ago

The objective of a coverage test is to exercise the various paths in the implementation to ensure they do not trigger any undefined behavior. It is not intended to be a "fuzz test" - although fuzz testing can be valuable, that is a separate test, typically done using a standard (non-stub) build via the public interface, where arbitrary/random input values are expected to be fully scrubbed and handled correctly - not for internal APIs that are expected to have more controlled inputs.

Currently the CF unit tests use lots of random values, and in particular may pass values that are not even within acceptable range, or do not make sense when paired with other inputs (e.g. a pointer and size where the size is a random value, exceeding the actual size of the object being pointed to). This randomness does not really add value to the coverage test objective, if anything it detracts from it by (possibly) creating opportunities for test cases to follow different paths through the code under test with each invocation.

The unit tests for CF should be scrubbed to remove use of random values, replace with a single/specific value(s) that are guaranteed to follow the intended path through the code under test.

jphickey commented 2 years ago

Additional info - this isn't just a style thing, I came across a couple cases of tests (e.g. for CF_Chunks_FindSmallestSize) not actually running consistent paths through the FSW depending on which random values were chosen. This means unit tests may or may not actually get 100% line coverage as intended, and some cases do not follow consistent logic paths because they are run with different input values.