[Tests] Cover DeckFilterString parse-error reporting (#7402)

* [Tests] Cover DeckFilterString parse-error reporting

DeckFilterString had no tests, so nothing pinned its error contract. Add
coverage for the parse-error message, per-instance isolation, and the fact
that the permanent parser's logger keeps no state between instances.

The logger is reinstalled by each constructor and invoked only by the parse
that immediately follows, so the logger's capture is live whenever it runs.
These tests guard that arrangement rather than a fixed defect.

* [Tests] Link gtest_main on the fetched-gtest path

The downloaded-gtest branch set GTEST_BOTH_LIBRARIES to `gtest` alone, while
FindGTest sets it to both halves. Every test target links that list rather than
picking an entry point, so on this path nothing provides main() and any test
source that does not define its own fails at link with `undefined reference to
main` pointing at crt1.o, naming neither gtest nor the line responsible.

That stayed latent while every test carried its own four-line main(). A test that
omits it now fails, and the error points at the C runtime rather than at gtest,
which is a long way from the cause. Restoring both halves makes the two paths
agree and drops the boilerplate requirement on new test sources.

The 18 sources that still define main() keep linking: gtest_main.cc defines
nothing but main, so the linker leaves its object in the archive unused rather
than reporting a duplicate.

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
This commit is contained in:
BruebachL 2026-10-08 14:18:41 +02:00 • committed by Lukas Brübach
parent 86ae4a0c6b
commit 90bb3b8807
2 changed files with 120 additions and 0 deletions

View file

@ -260,6 +260,20 @@ cockatrice_add_test(
LABELS
server
)
cockatrice_add_test(
NAME
deck_filter_string_test
SOURCES
${VERSION_STRING_CPP}
${CMAKE_SOURCE_DIR}/cockatrice/src/filters/deck_filter_string.cpp
deck_filter_string_test.cpp
LIBS
libcockatrice_deck_list
libcockatrice_filters
libcockatrice_card
INCLUDE_DIRS
${CMAKE_SOURCE_DIR}/cockatrice/src
)
cockatrice_add_test(
NAME
lag_monitor_test

View file

@ -0,0 +1,106 @@
/** @file deck_filter_string_test.cpp
* @brief Tests for DeckFilterString's parse-error reporting.
* @ingroup Tests
*/
#include <deck_filter_string.h>
#include <gtest/gtest.h>
namespace
{
/// The parser is a file-static permanent object whose logger is reinstalled by every
/// constructor, so these tests pin the observable contract that no state leaks from one
/// instance into the next, and that a destroyed instance cannot disturb a later parse.
QString parseErrorFor(const QString &expression)
{
DeckFilterString filter(expression);
return filter.valid() ? QString() : filter.error();
}
} // namespace
TEST(DeckFilterStringError, ValidExpressionHasNoError)
{
EXPECT_TRUE(parseErrorFor("name:foo").isEmpty());
EXPECT_TRUE(parseErrorFor("[[Lightning Bolt]]").isEmpty());
}
TEST(DeckFilterStringError, InvalidExpressionReportsAPosition)
{
const QString error = parseErrorFor("name:foo AND (");
ASSERT_FALSE(error.isEmpty());
EXPECT_TRUE(error.contains("Error at position")) << qPrintable(error);
}
TEST(DeckFilterStringError, DefaultConstructedIsNotValid)
{
DeckFilterString filter;
EXPECT_FALSE(filter.valid());
EXPECT_EQ(filter.error(), QString("Not initialized"));
}
TEST(DeckFilterStringError, EmptyExpressionIsValid)
{
EXPECT_TRUE(parseErrorFor("").isEmpty());
EXPECT_TRUE(parseErrorFor(" ").isEmpty());
}
// An error must not survive into an expression that parses. The parser is permanent and
// keeps its logger between calls, so a stale message here means state leaked.
TEST(DeckFilterStringError, ErrorDoesNotLeakIntoTheNextValidParse)
{
ASSERT_FALSE(parseErrorFor("name:foo AND (").isEmpty());
EXPECT_TRUE(parseErrorFor("name:foo").isEmpty());
}
TEST(DeckFilterStringError, ErrorDoesNotLeakIntoTheNextInvalidParse)
{
const QString first = parseErrorFor("name:foo AND (");
const QString second = parseErrorFor("format:");
ASSERT_FALSE(first.isEmpty());
ASSERT_FALSE(second.isEmpty());
// Each parse reports its own position. If the second reused the first's message, the
// message would still be non-empty but would name the wrong offset.
EXPECT_TRUE(second.contains("position 8")) << qPrintable(second);
EXPECT_TRUE(first.contains("position 15")) << qPrintable(first);
}
// Two filters can be live at once, so each must report only its own error.
TEST(DeckFilterStringError, SecondInstanceDoesNotDisturbTheFirst)
{
DeckFilterString first("name:foo AND (");
const QString firstError = first.error();
ASSERT_FALSE(firstError.isEmpty());
{
DeckFilterString second("format:");
ASSERT_FALSE(second.valid());
}
EXPECT_FALSE(first.valid());
EXPECT_EQ(first.error(), firstError);
}
TEST(DeckFilterStringError, DestroyedInstanceDoesNotDisturbALaterParse)
{
{
DeckFilterString doomed("name:foo AND (");
ASSERT_FALSE(doomed.valid());
}
// A parse after an instance has gone must still report its own error.
const QString error = parseErrorFor("name:foo AND name:bar AND (");
ASSERT_FALSE(error.isEmpty());
EXPECT_TRUE(error.contains("position 28")) << qPrintable(error);
}
TEST(DeckFilterStringError, ManyInstancesInSequenceEachReportThemselves)
{
// Long enough that an instance and its successor land on the same reused heap block,
// so a stale read would show up rather than passing by luck.
for (int i = 0; i < 200; ++i) {
const QString invalid = parseErrorFor("name:foo AND (");
ASSERT_FALSE(invalid.isEmpty()) << "iteration " << i;
ASSERT_TRUE(parseErrorFor("name:foo").isEmpty()) << "iteration " << i;
}
}