From 1de8cd5e36d133a62278e6b93bfcd903e10b7a65 Mon Sep 17 00:00:00 2001 From: timpieces Date: Fri, 23 Jan 2026 12:53:38 +0800 Subject: [PATCH] ExpressionParser: Make cosmetic test changes before adding more tests (#27098) Fix some weird quirks before adding new tests: - The whole file has clang format off, this feels unnecessary, localize it to where it's important - Put the code inside a nested test namespace within the production namespace, this is generally good practice rather than putting it in the global namespace, and it means we don't have to explicitly namespace methods that are directly under test - Rename variables to camelCase, which afaict is closer to 'normal' in freecad cpp - Remove the usage of arrays. Maintaining manual array sizes is annoying. Just use initializer lists here as they are all constants. - I have *not* renamed the snake_case methods, as I feel that it destroys too much of the git history for too little benefit --- tests/src/App/ExpressionParser.cpp | 62 +++++++++++++++--------------- 1 file changed, 32 insertions(+), 30 deletions(-) diff --git a/tests/src/App/ExpressionParser.cpp b/tests/src/App/ExpressionParser.cpp index c46098ed85..05ffa106b2 100644 --- a/tests/src/App/ExpressionParser.cpp +++ b/tests/src/App/ExpressionParser.cpp @@ -12,7 +12,8 @@ #include "src/App/InitApplication.h" -// clang-format off +namespace App::ExpressionParser::Test +{ class ExpressionParserTest: public ::testing::Test { @@ -24,39 +25,43 @@ protected: void SetUp() override { - _doc_name = App::GetApplication().getUniqueDocumentName("test"); - _this_doc = App::GetApplication().newDocument(_doc_name.c_str(), "testUser"); - _this_obj = _this_doc -> addObject("Sketcher::SketchObject"); + docName = App::GetApplication().getUniqueDocumentName("test"); + thisDoc = App::GetApplication().newDocument(docName.c_str(), "testUser"); + thisObj = thisDoc->addObject("Sketcher::SketchObject"); } void TearDown() override { - App::GetApplication().closeDocument(_doc_name.c_str()); + App::GetApplication().closeDocument(docName.c_str()); } - std::string doc_name() { return _doc_name; } - App::Document* this_doc() { return _this_doc; } - App::DocumentObject* this_obj() { return _this_obj; } + // clang-format off + std::string doc_name() { return docName; } + App::Document* this_doc() { return thisDoc; } + App::DocumentObject* this_obj() { return thisObj; } + // clang-format on - Base::Quantity parse_expression_text_as_quantity(const char* expression_text) { - const auto expression = App::ExpressionParser::parse(this_obj(), expression_text); + Base::Quantity parse_expression_text_as_quantity(const char* expression_text) + { + const auto expression = parse(thisObj, expression_text); return App::any_cast(expression->getValueAsAny()); } - Base::Quantity parse_quantity_text_as_quantity(const char* quantity_text) { + Base::Quantity parse_quantity_text_as_quantity(const char* quantity_text) + { return Base::Quantity::parse(quantity_text); } private: - std::string _doc_name; - App::Document* _this_doc {}; - App::DocumentObject* _this_obj {}; + std::string docName; + App::Document* thisDoc {}; + App::DocumentObject* thisObj {}; }; // https://github.com/FreeCAD/FreeCAD/issues/11965 TEST_F(ExpressionParserTest, functionPARSEQUANT) { - + // clang-format off EXPECT_ANY_THROW(App::ExpressionParser::parse(this_obj(), "parsequant()")) << "should not parse empty"; EXPECT_NO_THROW(App::ExpressionParser::parse(this_obj(), "parsequant(1 mm)")) << "should parse simple quantity"; @@ -67,7 +72,7 @@ TEST_F(ExpressionParserTest, functionPARSEQUANT) EXPECT_EQ(parse_quantity_text_as_quantity("1 mm"), parse_quantity_text_as_quantity("1 mm")) << "equality sanity check"; EXPECT_NE(parse_quantity_text_as_quantity("1 mm"), parse_quantity_text_as_quantity("2 mm")) << "inequality sanity check"; - std::array, 12> expression_vs_quantity_list = {{ + std::initializer_list> expression_vs_quantity_list = { // length { "1 mm", "1 mm" }, { "parsequant(1 mm)", "1 mm" }, @@ -83,13 +88,12 @@ TEST_F(ExpressionParserTest, functionPARSEQUANT) { "parsequant(10 g)", "10 g" }, { "parsequant(<<(10 + 20) kg>>)", "30000 g" }, { "parsequant(str(10 kg + 20010 g))", "30.01 kg" }, - }}; + }; - for ( const auto& expression_vs_quantity_pair : expression_vs_quantity_list ) { - auto expression_text = expression_vs_quantity_pair.first; - auto quantity_text = expression_vs_quantity_pair.second; + for (const auto& [expression_text, quantity_text] : expression_vs_quantity_list) { auto expression_result = parse_expression_text_as_quantity(expression_text); auto quantity_result = parse_quantity_text_as_quantity(quantity_text); + EXPECT_EQ(expression_result, quantity_result) << "mismatch:" " expression_text='" + std::string(expression_text) + "'" " quantity_text='" + std::string(quantity_text) + "'" @@ -97,22 +101,20 @@ TEST_F(ExpressionParserTest, functionPARSEQUANT) " quantity_representation='" + quantity_result.getUserString() + "'" ; } - + // clang-format on } TEST_F(ExpressionParserTest, isTokenAConstant) { - std::array constants {"pi", "e", "True", "False", "true", "false", "None"}; - for (const auto & constant : constants) { - EXPECT_TRUE(App::ExpressionParser::isTokenAConstant(constant)) - << "\"" << constant << "\" did not evaluate as a constant"; + for (const auto& constant : {"pi", "e", "True", "False", "true", "false", "None"}) { + EXPECT_TRUE(isTokenAConstant(constant)) + << "\"" << constant << "\" did not evaluate as a constant"; } - std::array notConstants {"PI", "E", "TRUE", "FALSE", "NONE", "none"}; - for (const auto & nonConstant : notConstants) { - EXPECT_FALSE(App::ExpressionParser::isTokenAConstant(nonConstant)) - << "\"" << nonConstant << "\" evaluated as a constant"; + for (const auto& nonConstant : {"PI", "E", "TRUE", "FALSE", "NONE", "none"}) { + EXPECT_FALSE(isTokenAConstant(nonConstant)) + << "\"" << nonConstant << "\" evaluated as a constant"; } } -// clang-format on +} // namespace App::ExpressionParser::Test