Skip to content

Commit 3d81736

Browse files
Merge pull request #3615 from IYP-Programer-Yeah:fix-per-test-filter-processing
PiperOrigin-RevId: 423326942 Change-Id: I913f31960d7917b176c9f390424630708473837a
2 parents 2ddfdf8 + 29bc520 commit 3d81736

File tree

1 file changed

+94
-51
lines changed

1 file changed

+94
-51
lines changed

googletest/src/gtest.cc

Lines changed: 94 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -725,60 +725,101 @@ static bool PatternMatchesString(const std::string& name_str,
725725
return true;
726726
}
727727

728-
bool UnitTestOptions::MatchesFilter(const std::string& name_str,
729-
const char* filter) {
730-
// The filter is a list of patterns separated by colons (:).
731-
const char* pattern = filter;
732-
while (true) {
733-
// Find the bounds of this pattern.
734-
const char* const next_sep = strchr(pattern, ':');
735-
const char* const pattern_end =
736-
next_sep != nullptr ? next_sep : pattern + strlen(pattern);
737-
738-
// Check if this pattern matches name_str.
739-
if (PatternMatchesString(name_str, pattern, pattern_end)) {
740-
return true;
741-
}
728+
namespace {
729+
730+
class UnitTestFilter {
731+
public:
732+
UnitTestFilter() = default;
733+
734+
// Constructs a filter from a string of patterns separated by `:`.
735+
explicit UnitTestFilter(const std::string& filter) {
736+
// By design "" filter matches "" string.
737+
SplitString(filter, ':', &patterns_);
738+
}
742739

743-
// Give up on this pattern. However, if we found a pattern separator (:),
744-
// advance to the next pattern (skipping over the separator) and restart.
745-
if (next_sep == nullptr) {
746-
return false;
740+
// Returns true if and only if name matches at least one of the patterns in
741+
// the filter.
742+
bool MatchesName(const std::string& name) const {
743+
return std::any_of(patterns_.begin(), patterns_.end(),
744+
[&name](const std::string& pattern) {
745+
return PatternMatchesString(
746+
name, pattern.c_str(),
747+
pattern.c_str() + pattern.size());
748+
});
749+
}
750+
751+
private:
752+
std::vector<std::string> patterns_;
753+
};
754+
755+
class PositiveAndNegativeUnitTestFilter {
756+
public:
757+
// Constructs a positive and a negative filter from a string. The string
758+
// contains a positive filter optionally followed by a '-' character and a
759+
// negative filter. In case only a negative filter is provided the positive
760+
// filter will be assumed "*".
761+
// A filter is a list of patterns separated by ':'.
762+
explicit PositiveAndNegativeUnitTestFilter(const std::string& filter) {
763+
std::vector<std::string> positive_and_negative_filters;
764+
765+
// NOTE: `SplitString` always returns a non-empty container.
766+
SplitString(filter, '-', &positive_and_negative_filters);
767+
const auto& positive_filter = positive_and_negative_filters.front();
768+
769+
if (positive_and_negative_filters.size() > 1) {
770+
positive_filter_ = UnitTestFilter(
771+
positive_filter.empty() ? kUniversalFilter : positive_filter);
772+
773+
// TODO(b/214626361): Fail on multiple '-' characters
774+
// For the moment to preserve old behavior we concatenate the rest of the
775+
// string parts with `-` as separator to generate the negative filter.
776+
auto negative_filter_string = positive_and_negative_filters[1];
777+
for (std::size_t i = 2; i < positive_and_negative_filters.size(); i++)
778+
negative_filter_string =
779+
negative_filter_string + '-' + positive_and_negative_filters[i];
780+
negative_filter_ = UnitTestFilter(negative_filter_string);
781+
} else {
782+
// In case we don't have a negative filter and positive filter is ""
783+
// we do not use kUniversalFilter by design as opposed to when we have a
784+
// negative filter.
785+
positive_filter_ = UnitTestFilter(positive_filter);
747786
}
748-
pattern = next_sep + 1;
749787
}
750-
return true;
788+
789+
// Returns true if and only if test name (this is generated by appending test
790+
// suit name and test name via a '.' character) matches the positive filter
791+
// and does not match the negative filter.
792+
bool MatchesTest(const std::string& test_suite_name,
793+
const std::string& test_name) const {
794+
return MatchesName(test_suite_name + "." + test_name);
795+
}
796+
797+
// Returns true if and only if name matches the positive filter and does not
798+
// match the negative filter.
799+
bool MatchesName(const std::string& name) const {
800+
return positive_filter_.MatchesName(name) &&
801+
!negative_filter_.MatchesName(name);
802+
}
803+
804+
private:
805+
UnitTestFilter positive_filter_;
806+
UnitTestFilter negative_filter_;
807+
};
808+
} // namespace
809+
810+
bool UnitTestOptions::MatchesFilter(const std::string& name_str,
811+
const char* filter) {
812+
return UnitTestFilter(filter).MatchesName(name_str);
751813
}
752814

753815
// Returns true if and only if the user-specified filter matches the test
754816
// suite name and the test name.
755817
bool UnitTestOptions::FilterMatchesTest(const std::string& test_suite_name,
756818
const std::string& test_name) {
757-
const std::string& full_name = test_suite_name + "." + test_name.c_str();
758-
759819
// Split --gtest_filter at '-', if there is one, to separate into
760820
// positive filter and negative filter portions
761-
std::string str = GTEST_FLAG_GET(filter);
762-
const char* const p = str.c_str();
763-
const char* const dash = strchr(p, '-');
764-
std::string positive;
765-
std::string negative;
766-
if (dash == nullptr) {
767-
positive = str.c_str(); // Whole string is a positive filter
768-
negative = "";
769-
} else {
770-
positive = std::string(p, dash); // Everything up to the dash
771-
negative = std::string(dash + 1); // Everything after the dash
772-
if (positive.empty()) {
773-
// Treat '-test1' as the same as '*-test1'
774-
positive = kUniversalFilter;
775-
}
776-
}
777-
778-
// A filter is a colon-separated list of patterns. It matches a
779-
// test if any pattern in it matches the test.
780-
return (MatchesFilter(full_name, positive.c_str()) &&
781-
!MatchesFilter(full_name, negative.c_str()));
821+
return PositiveAndNegativeUnitTestFilter(GTEST_FLAG_GET(filter))
822+
.MatchesTest(test_suite_name, test_name);
782823
}
783824

784825
#if GTEST_HAS_SEH
@@ -5719,9 +5760,9 @@ TestSuite* UnitTestImpl::GetTestSuite(
57195760
auto* const new_test_suite =
57205761
new TestSuite(test_suite_name, type_param, set_up_tc, tear_down_tc);
57215762

5763+
const UnitTestFilter death_test_suite_filter(kDeathTestSuiteFilter);
57225764
// Is this a death test suite?
5723-
if (internal::UnitTestOptions::MatchesFilter(test_suite_name,
5724-
kDeathTestSuiteFilter)) {
5765+
if (death_test_suite_filter.MatchesName(test_suite_name)) {
57255766
// Yes. Inserts the test suite after the last death test suite
57265767
// defined so far. This only works when the test suites haven't
57275768
// been shuffled. Otherwise we may end up running a death test
@@ -6054,6 +6095,9 @@ int UnitTestImpl::FilterTests(ReactionToSharding shard_tests) {
60546095
const int32_t shard_index = shard_tests == HONOR_SHARDING_PROTOCOL ?
60556096
Int32FromEnvOrDie(kTestShardIndex, -1) : -1;
60566097

6098+
const PositiveAndNegativeUnitTestFilter gtest_flag_filter(
6099+
GTEST_FLAG_GET(filter));
6100+
const UnitTestFilter disable_test_filter(kDisableTestFilter);
60576101
// num_runnable_tests are the number of tests that will
60586102
// run across all shards (i.e., match filter and are not disabled).
60596103
// num_selected_tests are the number of tests to be run on
@@ -6069,14 +6113,13 @@ int UnitTestImpl::FilterTests(ReactionToSharding shard_tests) {
60696113
const std::string test_name(test_info->name());
60706114
// A test is disabled if test suite name or test name matches
60716115
// kDisableTestFilter.
6072-
const bool is_disabled = internal::UnitTestOptions::MatchesFilter(
6073-
test_suite_name, kDisableTestFilter) ||
6074-
internal::UnitTestOptions::MatchesFilter(
6075-
test_name, kDisableTestFilter);
6116+
const bool is_disabled =
6117+
disable_test_filter.MatchesName(test_suite_name) ||
6118+
disable_test_filter.MatchesName(test_name);
60766119
test_info->is_disabled_ = is_disabled;
60776120

6078-
const bool matches_filter = internal::UnitTestOptions::FilterMatchesTest(
6079-
test_suite_name, test_name);
6121+
const bool matches_filter =
6122+
gtest_flag_filter.MatchesTest(test_suite_name, test_name);
60806123
test_info->matches_filter_ = matches_filter;
60816124

60826125
const bool is_runnable =

0 commit comments

Comments
 (0)