Skip to content

Commit

Permalink
Turn BasicTestInfo into a proper value type.
Browse files Browse the repository at this point in the history
We do this by storing the strings as `std::string` instead of `const char*`.
This is a preparatory step to allow dynamically generated fuzz test names,
but it has value in itself as it reduces a chance of dangling pointers.

PiperOrigin-RevId: 575831577
  • Loading branch information
fniksic authored and copybara-github committed Oct 23, 2023
1 parent 89d502c commit c35abd2
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 25 deletions.
12 changes: 6 additions & 6 deletions fuzztest/internal/googletest_adaptor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ void RegisterFuzzTestsAsGoogleTests(int* argc, char*** argv) {
return new ::fuzztest::internal::GTest_TestAdaptor(test, argc, argv);
};
if (test.uses_fixture()) {
::testing::RegisterTest(test.suite_name(), test.test_name(), nullptr,
nullptr, test.file(), test.line(),
std::move(fixture_factory));
::testing::RegisterTest(
test.suite_name().c_str(), test.test_name().c_str(), nullptr, nullptr,
test.file().c_str(), test.line(), std::move(fixture_factory));
} else {
::testing::RegisterTest(test.suite_name(), test.test_name(), nullptr,
nullptr, test.file(), test.line(),
std::move(test_factory));
::testing::RegisterTest(
test.suite_name().c_str(), test.test_name().c_str(), nullptr, nullptr,
test.file().c_str(), test.line(), std::move(test_factory));
}
});

Expand Down
21 changes: 11 additions & 10 deletions fuzztest/internal/registration.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <cstdlib>
#include <functional>
#include <iostream>
#include <string>
#include <tuple>
#include <type_traits>
#include <utility>
Expand All @@ -36,9 +37,9 @@
namespace fuzztest::internal {

struct BasicTestInfo {
const char* suite_name = nullptr;
const char* test_name = nullptr;
const char* file = nullptr;
std::string suite_name;
std::string test_name;
std::string file;
int line = 0;
bool uses_fixture = false;
};
Expand Down Expand Up @@ -127,7 +128,7 @@ class Registration : private Base {

public:
explicit Registration(BasicTestInfo info, TargetFunction target_function)
: test_info_(info), target_function_(target_function) {}
: test_info_(std::move(info)), target_function_(target_function) {}

// Registers domains for a property function as a `TupleOf` individual
// domains. This is useful when the domains are specified indirectly, e.g.,
Expand Down Expand Up @@ -155,7 +156,7 @@ class Registration : private Base {
"the number of function parameters.");
using NewBase = RegistrationWithDomainsBase<value_type_t<NewDomains>...>;
return Registration<Fixture, TargetFunction, NewBase, SeedProvider>(
test_info_, target_function_, NewBase{std::move(domain)});
std::move(test_info_), target_function_, NewBase{std::move(domain)});
}

// Registers a domain for each parameter of the property function. This is the
Expand Down Expand Up @@ -194,7 +195,7 @@ class Registration : private Base {
if constexpr (!Registration::kHasSeeds) {
return Registration<Fixture, TargetFunction,
RegistrationWithSeedsBase<Base>, SeedProvider>(
test_info_, target_function_,
std::move(test_info_), target_function_,
RegistrationWithSeedsBase<Base>(std::move(*this)))
.WithSeeds(seeds);
} else {
Expand All @@ -211,7 +212,7 @@ class Registration : private Base {
if (!status.ok()) {
ReportBadSeed(seed, status);
continue;
};
}
this->seeds_.push_back(*std::move(corpus_value));
}
return std::move(*this);
Expand All @@ -231,7 +232,7 @@ class Registration : private Base {
Fixture, TargetFunction,
RegistrationWithSeedProviderBase<Base, decltype(seed_provider)>,
decltype(seed_provider)>(
test_info_, target_function_,
std::move(test_info_), target_function_,
RegistrationWithSeedProviderBase<Base, decltype(seed_provider)>(
std::move(*this), std::move(seed_provider)));
}
Expand Down Expand Up @@ -259,7 +260,7 @@ class Registration : private Base {
absl::AnyInvocable<std::vector<SeedT>(BaseFixture*) const>;
using NewBase = RegistrationWithSeedProviderBase<Base, NewSeedProvider>;
return Registration<Fixture, TargetFunction, NewBase, NewSeedProvider>(
test_info_, target_function_,
std::move(test_info_), target_function_,
NewBase(std::move(*this), std::mem_fn(seed_provider)));
}

Expand Down Expand Up @@ -287,7 +288,7 @@ class Registration : private Base {
explicit Registration(BasicTestInfo info, TargetFunction target_function,
Base base)
: Base(std::move(base)),
test_info_(info),
test_info_(std::move(info)),
target_function_(target_function) {}

BasicTestInfo test_info_;
Expand Down
2 changes: 1 addition & 1 deletion fuzztest/internal/registry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ void ForEachTest(absl::FunctionRef<void(FuzzTest&)> func) {
}

void RegisterImpl(BasicTestInfo test_info, FuzzTestFuzzerFactory factory) {
Regs().emplace_back(test_info, std::move(factory));
Regs().emplace_back(std::move(test_info), std::move(factory));
}

void RegisterSetUpTearDownTestSuiteFunctions(
Expand Down
7 changes: 4 additions & 3 deletions fuzztest/internal/registry.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,14 @@ struct RegistrationToken {
typename SeedProvider>
RegistrationToken& operator=(
Registration<Fixture, TargetFunction, RegBase, SeedProvider>&& reg) {
const BasicTestInfo test_info = reg.test_info_;
RegisterImpl(test_info, GetFuzzTestFuzzerFactory(std::move(reg)));
if constexpr (std::is_base_of_v<FixtureWithExplicitSetUp, Fixture>) {
RegisterSetUpTearDownTestSuiteFunctions(test_info.suite_name,
RegisterSetUpTearDownTestSuiteFunctions(reg.test_info_.suite_name,
&Fixture::SetUpTestSuite,
&Fixture::TearDownTestSuite);
}
BasicTestInfo test_info = reg.test_info_;
RegisterImpl(std::move(test_info),
GetFuzzTestFuzzerFactory(std::move(reg)));
return *this;
}

Expand Down
11 changes: 6 additions & 5 deletions fuzztest/internal/runtime.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "absl/functional/function_ref.h"
#include "absl/random/bit_gen_ref.h"
#include "absl/random/discrete_distribution.h"
#include "absl/strings/str_cat.h"
#include "absl/strings/str_format.h"
#include "absl/strings/string_view.h"
#include "absl/time/time.h"
Expand Down Expand Up @@ -77,14 +78,14 @@ using FuzzTestFuzzerFactory =
class FuzzTest {
public:
FuzzTest(BasicTestInfo test_info, FuzzTestFuzzerFactory factory)
: test_info_(test_info), make_(std::move(factory)) {}
: test_info_(std::move(test_info)), make_(std::move(factory)) {}

const char* suite_name() const { return test_info_.suite_name; }
const char* test_name() const { return test_info_.test_name; }
const std::string& suite_name() const { return test_info_.suite_name; }
const std::string& test_name() const { return test_info_.test_name; }
std::string full_name() const {
return suite_name() + std::string(".") + test_name();
return absl::StrCat(test_info_.suite_name, ".", test_info_.test_name);
}
const char* file() const { return test_info_.file; }
const std::string& file() const { return test_info_.file; }
int line() const { return test_info_.line; }
bool uses_fixture() const { return test_info_.uses_fixture; }
auto make() const { return make_(*this); }
Expand Down

0 comments on commit c35abd2

Please sign in to comment.