gz-math icon indicating copy to clipboard operation
gz-math copied to clipboard

Experiment with fuzztest

Open scpeters opened this issue 8 months ago • 8 comments

Desired behavior

Some of our existing gz-math tests are already structured like property-based tests, so convert some of them to use FuzzTest. They should still be able to work as unit tests, but when run with a fuzzing engine, they may find more edge cases instead of the current hard-coded test cases.

Alternatives considered

Implementation suggestion

For example, the following tests are candidates:

  • https://github.com/gazebosim/gz-math/blob/gz-math8/src/Helpers_TEST.cc#L513-L528
  • https://github.com/gazebosim/gz-math/blob/gz-math8/src/Inertial_TEST.cc#L206-L272
  • https://github.com/gazebosim/gz-math/blob/gz-math8/src/Inertial_TEST.cc#L318-L371

For a first prototype, I would use a vendored copy of fuzztest (similar to our gtest_vendor folder), but we would need to decide whether we want to vendor it or not.

Additional context

scpeters avatar Apr 16 '25 19:04 scpeters

Hey, I'd like to work on this.

shashank1300 avatar Apr 18 '25 17:04 shashank1300

@shashank1300 great! If you haven't done fuzz testing before, can you try following the Quickstart with CMake tutorial?

From the Create and run a fuzz test, it looks like fuzz tests can be created for a void function that takes certain parameters and contains GTest expectations. Maybe the closest example that I see in gz-math is the void VerifyDiagonalMomentsAndAxes(const math::Vector3d &_moments) function in MassMatrix3_TEST.cc. If fuzztest wants native types, you could pass three doubles instead of a Vector3. One complication is that there are limits on which inputs are valid (each value must be positive, and the set must not violate the triangle inequality), so maybe this isn't the best example.

It might be easier to add simpler tests like IntegerAdditionCommute for our Vector types

scpeters avatar Apr 18 '25 18:04 scpeters

another starting point would be to write a function that takes two int64_t values and calls secNsecToTimePoint then timePointToSecNsec and verifies that the outputs match the inputs. Likewise with secNsecToDuration and durationToSecNsec:

https://github.com/gazebosim/gz-math/blob/gz-math8/include/gz/math/Helpers.hh#L545-L578

scpeters avatar Apr 18 '25 21:04 scpeters

Hey @scpeters, I appreciate the help. I'll start right away.

shashank1300 avatar Apr 19 '25 07:04 shashank1300

@shashank1300 how is it going?

kscottz avatar Apr 24 '25 21:04 kscottz

@kscottz I did start it but ran into some unknown error. And I haven't been able to pick it up again since, as I am busy with my Uni exams. I'll continue after a few weeks.

shashank1300 avatar Apr 30 '25 08:04 shashank1300

Let us know when you circle back. Good luck on exams!

kscottz avatar May 01 '25 19:05 kscottz

@kscottz @scpeters Hello developers ! I am interested in this too ! Today, I learn from Quickstart with CMake and complete fuzzing enging test of https://github.com/gazebosim/gz-math/blob/gz-math8/src/Helpers_TEST.cc#L513-L528.

#include <cstdint>
#include <set>
#include <tuple>
#include <iomanip>
#include <cmath>
#include <limits>
#include "gtest/gtest.h"
#include "fuzztest/fuzztest.h"
#include "gz/math/Rand.hh"
#include "gz/math/Helpers.hh"
#include <gz/utils/SuppressWarning.hh>
using namespace gz;
namespace {

void PairUnpairCorrectness(uint16_t a, uint16_t b) {
    uint16_t c, d;
    math::PairOutput key = math::Pair(a, b);
    std::tie(c, d) = math::Unpair(key);
    EXPECT_EQ(a, c) << "a: " << a << ", b: " << b;
    EXPECT_EQ(b, d) << "a: " << a << ", b: " << b;
}

void PairKeyUniqueness(const std::vector<std::pair<uint16_t, uint16_t>>& pairs) {
    std::set<math::PairOutput> keys;
    for (const auto& pair : pairs) {
        uint16_t a = pair.first;
        uint16_t b = pair.second;
        math::PairOutput key = math::Pair(a, b);
        
        EXPECT_TRUE(keys.find(key) == keys.end())
            << "Duplicate key found for pair (" << a << ", " << b << ")!";
        
        keys.insert(key);
    }
}

FUZZ_TEST(MathFuzzTests, PairUnpairCorrectness)
    .WithDomains(
        fuzztest::Arbitrary<uint16_t>(),
        fuzztest::Arbitrary<uint16_t>()
    );
FUZZ_TEST(MathFuzzTests, PairKeyUniqueness)
    .WithDomains(
        fuzztest::UniqueElementsVectorOf(
            fuzztest::PairOf(
                fuzztest::Arbitrary<uint16_t>(),
                fuzztest::Arbitrary<uint16_t>()
            )
        ).WithMaxSize(1000)
    );
} 

It can build and run successfully, and no bug found. In the process of writing , I find a point we should pay more attention(since I learn it today, please correct me if I am inaccurate):

  1. Because it is a double loop in Helper_TEST.cc there are no duplicate pairs of keys, but if the PairKeyUniqueness above will appear. If it is run directly, an error will be reported, but this does not conform to the test logic, so the UniqueElementsVectorOf constraint is performed on the input field of the second function. This means that we must ensure that the input domain of the fuzz testing engine version is consistent with the original test input domain. The first test conversion is relatively simple. The second candidate will use custom type(or we can do a test transitionz, as @scpeters said above, pass three doubles and impose constraints in the input field).

XINJIANGMO avatar Jun 24 '25 12:06 XINJIANGMO