From dcbf8aac91b6dc03ce91646be41e773fe1595ba7 Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Fri, 21 Aug 2026 15:55:48 +0200 Subject: [PATCH 1/3] gconnman_tech_test.cpp: Run also if manager has technologies It could be that the manager fill the technologies before the onTechnologiesChanged is emitted. Check if there are already technologies in the manager and run the tests on them. The latter make the test more robust. Signed-off-by: Eduardo Gonzalez --- tests/gconnman_tech_test.cpp | 302 +++++++++++++++++++++-------------- 1 file changed, 180 insertions(+), 122 deletions(-) diff --git a/tests/gconnman_tech_test.cpp b/tests/gconnman_tech_test.cpp index d09989b..7164e76 100644 --- a/tests/gconnman_tech_test.cpp +++ b/tests/gconnman_tech_test.cpp @@ -17,24 +17,33 @@ TEST(Connman, getTechs) { Connman connman; const auto manager = connman.manager(); - manager->onTechnologiesChanged( - [&called, main_tid = thread_bundle.main_tid, - loop_tid = thread_bundle.loop_tid](const auto& technologies) { - called = true; + auto do_on_techs = [&called, main_tid = thread_bundle.main_tid, + loop_tid = thread_bundle.loop_tid]( + const auto& technologies, + const bool check_thread_id = true) { + called = true; + if (check_thread_id) { const auto callback_tid = std::this_thread::get_id(); EXPECT_NE(callback_tid, main_tid); EXPECT_NE(callback_tid, loop_tid); - ASSERT_FALSE(technologies.empty()); - for (const auto& tech : technologies) { - const auto props = tech->properties(); - EXPECT_FALSE(props.getName().empty()); - if (props.isConnected()) { - EXPECT_TRUE(props.isPowered()) - << "Technology is connected but not powered"; - } - std::cout << props; + } + ASSERT_FALSE(technologies.empty()) << "No technologies returned"; + for (const auto& tech : technologies) { + const auto props = tech->properties(); + EXPECT_FALSE(props.getName().empty()); + if (props.isConnected()) { + EXPECT_TRUE(props.isPowered()) + << "Technology is connected but not powered"; } - }); + std::cout << props; + } + }; + + if (manager->technologies().empty()) { + manager->onTechnologiesChanged(do_on_techs); + } else { + do_on_techs(manager->technologies(), false); + } } ASSERT_TRUE(called) << "TechnologiesChanged callback was never called"; } @@ -47,14 +56,16 @@ TEST(Connman, PowerOnAllTechnologies) { Connman connman; const auto manager = connman.manager(); - manager->onTechnologiesChanged([&called, - main_tid = thread_bundle.main_tid, - loop_tid = thread_bundle.loop_tid]( - const auto& technologies) { - const auto callback_tid = std::this_thread::get_id(); - EXPECT_NE(callback_tid, main_tid); - EXPECT_NE(callback_tid, loop_tid); - ASSERT_FALSE(technologies.empty()); + auto do_on_techs = [&called, main_tid = thread_bundle.main_tid, + loop_tid = thread_bundle.loop_tid]( + const auto& technologies, + const bool check_thread_id = true) { + if (check_thread_id) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + } + ASSERT_FALSE(technologies.empty()) << "No technologies returned"; // Power on all technologies for (const auto& tech : technologies) { tech->onPropertyChanged([main_tid, loop_tid](const auto& prop) { @@ -85,7 +96,12 @@ TEST(Connman, PowerOnAllTechnologies) { }); } } - }); + }; + if (manager->technologies().empty()) { + manager->onTechnologiesChanged(do_on_techs); + } else { + do_on_techs(manager->technologies(), false); + } } ASSERT_TRUE(called) << "setPowered callback was never called"; } @@ -97,32 +113,42 @@ TEST(Connman, ScanWifiTechnology) { Connman connman; const auto manager = connman.manager(); - manager->onTechnologiesChanged( - [&called, main_tid = thread_bundle.main_tid, - loop_tid = thread_bundle.loop_tid](const auto& technologies) { - ASSERT_FALSE(technologies.empty()) - << "No technologies returned"; + auto do_on_techs = [&called, main_tid = thread_bundle.main_tid, + loop_tid = thread_bundle.loop_tid]( + const auto& technologies, + const bool check_thread_id = true) { + if (check_thread_id) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + } + ASSERT_FALSE(technologies.empty()) << "No technologies returned"; - for (const auto& tech : technologies) { - const auto props = tech->properties(); - const auto name = props.getName(); - if (props.getType() == Type::Wifi) { - std::cout << "Scanning technology with name: " << name - << "\n"; - tech->scan( - [&called, name, main_tid, loop_tid](bool success) { - const auto callback_tid = - std::this_thread::get_id(); - EXPECT_NE(callback_tid, main_tid); - EXPECT_NE(callback_tid, loop_tid); - called = true; - EXPECT_TRUE(success); - std::cout << "Technology " << name - << " scanned successfully.\n"; - }); - } + for (const auto& tech : technologies) { + const auto props = tech->properties(); + const auto name = props.getName(); + if (props.getType() == Type::Wifi) { + std::cout << "Scanning technology with name: " << name + << "\n"; + tech->scan([&called, name, main_tid, + loop_tid](bool success) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + called = true; + EXPECT_TRUE(success); + std::cout << "Technology " << name + << " scanned successfully.\n"; + }); } - }); + } + }; + + if (manager->technologies().empty()) { + manager->onTechnologiesChanged(do_on_techs); + } else { + do_on_techs(manager->technologies(), false); + } } ASSERT_TRUE(called) << "TechnologiesChanged callback was never called"; } @@ -134,66 +160,75 @@ TEST(Connman, SetTetheringOn) { Connman connman; const auto manager = connman.manager(); - manager->onTechnologiesChanged( - [&called, main_tid = thread_bundle.main_tid, - loop_tid = thread_bundle.loop_tid](const auto& technologies) { - ASSERT_FALSE(technologies.empty()) - << "No technologies returned"; + auto do_on_techs = [&called, main_tid = thread_bundle.main_tid, + loop_tid = thread_bundle.loop_tid]( + const auto& technologies, + const bool check_thread_id = true) { + if (check_thread_id) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + } + ASSERT_FALSE(technologies.empty()) << "No technologies returned"; - for (const auto& tech : technologies) { - const auto props = tech->properties(); - const auto name = props.getName(); + for (const auto& tech : technologies) { + const auto props = tech->properties(); + const auto name = props.getName(); - if (props.getType() == Type::Wifi) { // test only wifi - std::cout << "Setting tethering properties for " << name - << "\n"; - tech->setTetheringIdentifier( - "AmarulaTestSSID", - [name, main_tid, loop_tid](bool success) { - const auto callback_tid = - std::this_thread::get_id(); - EXPECT_NE(callback_tid, main_tid); - EXPECT_NE(callback_tid, loop_tid); - EXPECT_TRUE(success) - << "Failed to set tethering identifier for " - << name; - }); - tech->setTetheringPassphrase( - "AmarulaTestPassphrase", - [name, main_tid, loop_tid](bool success) { - const auto callback_tid = - std::this_thread::get_id(); - EXPECT_NE(callback_tid, main_tid); - EXPECT_NE(callback_tid, loop_tid); - EXPECT_TRUE(success) - << "Failed to set tethering passphrase for " - << name; - }); + if (props.getType() == Type::Wifi) { // test only wifi + std::cout << "Setting tethering properties for " << name + << "\n"; + tech->setTetheringIdentifier( + "AmarulaTestSSID", + [name, main_tid, loop_tid](bool success) { + const auto callback_tid = + std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + EXPECT_TRUE(success) + << "Failed to set tethering identifier for " + << name; + }); + tech->setTetheringPassphrase( + "AmarulaTestPassphrase", + [name, main_tid, loop_tid](bool success) { + const auto callback_tid = + std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + EXPECT_TRUE(success) + << "Failed to set tethering passphrase for " + << name; + }); - tech->setTetheringFreq( - WIFI_FREQ_2412_MHZ, - [name, main_tid, loop_tid](bool success) { - const auto callback_tid = - std::this_thread::get_id(); - EXPECT_NE(callback_tid, main_tid); - EXPECT_NE(callback_tid, loop_tid); - EXPECT_TRUE(success) - << "Failed to set tethering frequency for " - << name; - }); - tech->setTethering(true, [&called, name, main_tid, - loop_tid](bool success) { + tech->setTetheringFreq( + WIFI_FREQ_2412_MHZ, + [name, main_tid, loop_tid](bool success) { const auto callback_tid = std::this_thread::get_id(); EXPECT_NE(callback_tid, main_tid); EXPECT_NE(callback_tid, loop_tid); EXPECT_TRUE(success) - << "Failed to set tethering for " << name; - called = true; + << "Failed to set tethering frequency for " + << name; }); - } + tech->setTethering(true, [&called, name, main_tid, + loop_tid](bool success) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + EXPECT_TRUE(success) + << "Failed to set tethering for " << name; + called = true; + }); } - }); + } + }; + if (manager->technologies().empty()) { + manager->onTechnologiesChanged(do_on_techs); + } else { + do_on_techs(manager->technologies(), false); + } } ASSERT_TRUE(called) << "setTethering callback was never called"; } @@ -205,31 +240,40 @@ TEST(Connman, SetTetheringOff) { Connman connman; const auto manager = connman.manager(); - manager->onTechnologiesChanged( - [&called, main_tid = thread_bundle.main_tid, - loop_tid = thread_bundle.loop_tid](const auto& technologies) { - ASSERT_FALSE(technologies.empty()) - << "No technologies returned"; + auto do_on_techs = [&called, main_tid = thread_bundle.main_tid, + loop_tid = thread_bundle.loop_tid]( + const auto& technologies, + const bool check_thread_id = true) { + if (check_thread_id) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + } + ASSERT_FALSE(technologies.empty()) << "No technologies returned"; - for (const auto& tech : technologies) { - const auto props = tech->properties(); - const auto name = props.getName(); + for (const auto& tech : technologies) { + const auto props = tech->properties(); + const auto name = props.getName(); - if (props.getType() == Type::Wifi) { // test only wifi - std::cout << "Disable tethering for " << name << "\n"; - tech->setTethering(false, [&called, name, main_tid, - loop_tid](bool success) { - const auto callback_tid = - std::this_thread::get_id(); - EXPECT_NE(callback_tid, main_tid); - EXPECT_NE(callback_tid, loop_tid); - EXPECT_TRUE(success) - << "Failed to unset tethering for " << name; - called = true; - }); - } + if (props.getType() == Type::Wifi) { // test only wifi + std::cout << "Disable tethering for " << name << "\n"; + tech->setTethering(false, [&called, name, main_tid, + loop_tid](bool success) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + EXPECT_TRUE(success) + << "Failed to unset tethering for " << name; + called = true; + }); } - }); + } + }; + if (manager->technologies().empty()) { + manager->onTechnologiesChanged(do_on_techs); + } else { + do_on_techs(manager->technologies(), false); + } } ASSERT_TRUE(called) << "setTethering callback was never called"; } @@ -237,12 +281,20 @@ TEST(Connman, SetTetheringOff) { TEST(Connman, PowerOffAllTechnologies) { bool called = false; { + const ThreadBundle thread_bundle; Connman connman; const auto manager = connman.manager(); - manager->onTechnologiesChanged([&](const auto& technologies) { + auto do_on_techs = [&called, main_tid = thread_bundle.main_tid, + loop_tid = thread_bundle.loop_tid]( + const auto& technologies, + const bool check_thread_id = true) { + if (check_thread_id) { + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + } ASSERT_FALSE(technologies.empty()) << "No technologies returned"; - // Power off all technologies for (const auto& tech : technologies) { tech->onPropertyChanged([&](const auto& prop) { @@ -266,7 +318,13 @@ TEST(Connman, PowerOffAllTechnologies) { }); } } - }); + }; + + if (manager->technologies().empty()) { + manager->onTechnologiesChanged(do_on_techs); + } else { + do_on_techs(manager->technologies(), false); + } } ASSERT_TRUE(called) << "setPowered callback was never called"; } From 0836256ac7049519dafa615394e4670597d2e282 Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Mon, 24 Aug 2026 10:59:28 +0200 Subject: [PATCH 2/3] gconnman_tech_test.cpp: Check the changed property Could be that other property change triggers the callback and the EXPECT macro is evaluated while the powered property value have not change it. Check if the powered property is the one that changed to evaluate the macro. The latter make the tests more robust. Signed-off-by: Eduardo Gonzalez --- tests/gconnman_tech_test.cpp | 52 +++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/tests/gconnman_tech_test.cpp b/tests/gconnman_tech_test.cpp index 7164e76..d30fa03 100644 --- a/tests/gconnman_tech_test.cpp +++ b/tests/gconnman_tech_test.cpp @@ -68,19 +68,24 @@ TEST(Connman, PowerOnAllTechnologies) { ASSERT_FALSE(technologies.empty()) << "No technologies returned"; // Power on all technologies for (const auto& tech : technologies) { - tech->onPropertyChanged([main_tid, loop_tid](const auto& prop) { - EXPECT_TRUE(prop.isPowered()) - << "Technology " << prop.getName() - << " was not powered ON"; - const auto callback_tid = std::this_thread::get_id(); - EXPECT_NE(callback_tid, main_tid); - EXPECT_NE(callback_tid, loop_tid); - std::cout << "onPropertyChanged:\n"; - std::cout << prop; - }); - const auto prop = tech->properties(); - const auto name = prop.getName(); - if (!prop.isPowered()) { + const auto is_powered = tech->properties().isPowered(); + tech->onPropertyChanged( + [main_tid, loop_tid, is_powered](const auto& prop) { + // If the powered state changed, check that it is now + // powered on + if (is_powered != prop.isPowered()) { + EXPECT_TRUE(prop.isPowered()) + << "Technology " << prop.getName() + << " was not powered ON"; + } + const auto callback_tid = std::this_thread::get_id(); + EXPECT_NE(callback_tid, main_tid); + EXPECT_NE(callback_tid, loop_tid); + std::cout << "onPropertyChanged:\n"; + std::cout << prop; + }); + const auto name = tech->properties().getName(); + if (!is_powered) { std::cout << "Powering on technology: " << name << '\n'; tech->setPowered(true, [&called, name, main_tid, loop_tid](auto success) { @@ -297,18 +302,21 @@ TEST(Connman, PowerOffAllTechnologies) { ASSERT_FALSE(technologies.empty()) << "No technologies returned"; // Power off all technologies for (const auto& tech : technologies) { - tech->onPropertyChanged([&](const auto& prop) { - EXPECT_FALSE(prop.isPowered()) - << "Technology " << prop.getName() - << " was not powered OFF"; + const auto is_powered = tech->properties().isPowered(); + tech->onPropertyChanged([is_powered](const auto& prop) { + // If the powered state changed, check that it is now + // powered off + if (is_powered != prop.isPowered()) { + EXPECT_FALSE(prop.isPowered()) + << "Technology " << prop.getName() + << " was not powered OFF"; + } std::cout << "onPropertyChanged:\n"; std::cout << prop; }); - const auto prop = tech->properties(); - const auto name = prop.getName(); - if (prop.isPowered()) { - std::cout << "Powering off technology: " << prop.getName() - << '\n'; + const auto name = tech->properties().getName(); + if (is_powered) { + std::cout << "Powering off technology: " << name << '\n'; tech->setPowered(false, [&, name](auto success) { std::cout << "setPowered callback for " << name << ": " << (success ? "Success" : "Failure") << '\n'; From 2302f87b28d11b16a242d792dca8258f49f8046e Mon Sep 17 00:00:00 2001 From: Eduardo Gonzalez Date: Fri, 21 Aug 2026 12:36:30 +0200 Subject: [PATCH 3/3] tests: Add valgrind executable tests Add tests that run valgrind on the actual tests. Install the latter tests in the development package. The latter helps the tester to run the memory leak checks and make it simpler to reproduce. Fixes #23 Signed-off-by: Eduardo Gonzalez --- tests/CMakeLists.txt | 26 ++++++++++++++++++++++++++ tests/valgrind_test.in | 9 +++++++++ 2 files changed, 35 insertions(+) create mode 100644 tests/valgrind_test.in diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 2b226cb..94265e4 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -9,8 +9,33 @@ set(BUILD_SHARED_LIBS OFF) FetchContent_MakeAvailable(googletest) include(GoogleTest) +function(add_valgrind_wrapper test_name) + set(TEST_NAME "${test_name}") + + configure_file(${CMAKE_CURRENT_SOURCE_DIR}/valgrind_test.in + ${CMAKE_CURRENT_BINARY_DIR}/${test_name}_valgrind @ONLY) + + file( + CHMOD + ${CMAKE_CURRENT_BINARY_DIR}/${test_name}_valgrind + PERMISSIONS + OWNER_READ + OWNER_WRITE + OWNER_EXECUTE + GROUP_READ + GROUP_EXECUTE + WORLD_READ + WORLD_EXECUTE) + + install( + PROGRAMS ${CMAKE_CURRENT_BINARY_DIR}/${test_name}_valgrind + DESTINATION ${CMAKE_INSTALL_BINDIR} + COMPONENT ${PROJECT_NAME}-dev) +endfunction() + add_executable(gdbusproxypp_test gdbusproxypp_test.cpp) target_link_libraries(gdbusproxypp_test PRIVATE GDbusProxy gtest_main) +add_valgrind_wrapper(gdbusproxypp_test) install( TARGETS gdbusproxypp_test @@ -31,6 +56,7 @@ if(BUILD_CONNMAN) TARGETS ${connman_test} EXPORT ${PROJECT_NAME}-config COMPONENT ${PROJECT_NAME}-dev) + add_valgrind_wrapper(${connman_test}) endforeach() endif(BUILD_CONNMAN) diff --git a/tests/valgrind_test.in b/tests/valgrind_test.in new file mode 100644 index 0000000..456e166 --- /dev/null +++ b/tests/valgrind_test.in @@ -0,0 +1,9 @@ +#!/bin/sh + +exec env \ + G_SLICE=always-malloc \ + G_DEBUG=gc-friendly \ + valgrind \ + --leak-check=full \ + --show-leak-kinds=none \ + "$(dirname "$0")/@TEST_NAME@" "$@"