From 39ba3164467f59d3f61a7482253a277e44ab4090 Mon Sep 17 00:00:00 2001 From: David Gardner Date: Thu, 28 Sep 2023 13:50:36 -0700 Subject: [PATCH 1/4] Test edgeholder connections --- cpp/mrc/tests/test_edges.cpp | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/cpp/mrc/tests/test_edges.cpp b/cpp/mrc/tests/test_edges.cpp index 86e42dfb5..f8de4eef2 100644 --- a/cpp/mrc/tests/test_edges.cpp +++ b/cpp/mrc/tests/test_edges.cpp @@ -23,6 +23,7 @@ #include "mrc/edge/edge_channel.hpp" #include "mrc/edge/edge_readable.hpp" #include "mrc/edge/edge_writable.hpp" +#include "mrc/edge/forward.hpp" #include "mrc/node/generic_source.hpp" #include "mrc/node/operators/broadcast.hpp" #include "mrc/node/operators/combine_latest.hpp" @@ -57,7 +58,7 @@ using namespace std::chrono_literals; TEST_CLASS(Edges); -using TestEdgesDeathTest = TestEdges; // NOLINT(readability-identifier-naming) +using TestEdgesDeathTest = TestEdges; // NOLINT(readability-identifier-naming)p namespace mrc::node { @@ -996,4 +997,28 @@ TEST_F(TestEdges, EdgeTapWithSpliceRxComponent) EXPECT_TRUE(node->stream_fn_called); } + +template +class TestEdgeHolder : public edge::EdgeHolder +{ + public: + bool has_active_connection() const + { + return this->check_active_connection(false); + } + + void call_release_edge_connection() + { + this->release_edge_connection(); + } +}; + +TEST_F(TestEdges, EdgeHolderIsConnected) +{ + TestEdgeHolder edge_holder; + + EXPECT_FALSE(edge_holder.has_active_connection()); + edge_holder.call_release_edge_connection(); + EXPECT_TRUE(edge_holder.has_active_connection()); +} } // namespace mrc From 790a50cd6f0d67518916cb1d9378aa8f3777c080 Mon Sep 17 00:00:00 2001 From: David Gardner Date: Thu, 28 Sep 2023 14:13:14 -0700 Subject: [PATCH 2/4] release_edge_connection should only reset the m_connected_edge, prevents check_active_connection from mistakenly returning true for a holder where init_owned_edge has been called but neither the init_connected_edge method or the add_connector method have not been called --- cpp/mrc/include/mrc/edge/edge_holder.hpp | 1 - cpp/mrc/tests/test_edges.cpp | 11 ++++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/cpp/mrc/include/mrc/edge/edge_holder.hpp b/cpp/mrc/include/mrc/edge/edge_holder.hpp index b3d801484..0262a7e71 100644 --- a/cpp/mrc/include/mrc/edge/edge_holder.hpp +++ b/cpp/mrc/include/mrc/edge/edge_holder.hpp @@ -152,7 +152,6 @@ class EdgeHolder void release_edge_connection() { - m_owned_edge_lifetime.reset(); m_connected_edge.reset(); } diff --git a/cpp/mrc/tests/test_edges.cpp b/cpp/mrc/tests/test_edges.cpp index f8de4eef2..874ae2ad5 100644 --- a/cpp/mrc/tests/test_edges.cpp +++ b/cpp/mrc/tests/test_edges.cpp @@ -1011,14 +1011,23 @@ class TestEdgeHolder : public edge::EdgeHolder { this->release_edge_connection(); } + + void call_init_owned_edge(std::shared_ptr> edge) + { + this->init_owned_edge(std::move(edge)); + } }; TEST_F(TestEdges, EdgeHolderIsConnected) { TestEdgeHolder edge_holder; + auto edge = std::make_shared>(); + EXPECT_FALSE(edge_holder.has_active_connection()); + edge_holder.call_init_owned_edge(edge); EXPECT_FALSE(edge_holder.has_active_connection()); + edge_holder.call_release_edge_connection(); - EXPECT_TRUE(edge_holder.has_active_connection()); + EXPECT_FALSE(edge_holder.has_active_connection()); } } // namespace mrc From 50b50e7b60df46bec125b5b3586be7c379217f4b Mon Sep 17 00:00:00 2001 From: David Gardner Date: Thu, 28 Sep 2023 14:15:33 -0700 Subject: [PATCH 3/4] IWYU fixes --- cpp/mrc/tests/test_edges.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/mrc/tests/test_edges.cpp b/cpp/mrc/tests/test_edges.cpp index 874ae2ad5..4e0629af0 100644 --- a/cpp/mrc/tests/test_edges.cpp +++ b/cpp/mrc/tests/test_edges.cpp @@ -19,11 +19,12 @@ #include "mrc/channel/buffered_channel.hpp" // IWYU pragma: keep #include "mrc/channel/forward.hpp" +#include "mrc/edge/edge.hpp" // for Edge #include "mrc/edge/edge_builder.hpp" #include "mrc/edge/edge_channel.hpp" +#include "mrc/edge/edge_holder.hpp" // for EdgeHolder #include "mrc/edge/edge_readable.hpp" #include "mrc/edge/edge_writable.hpp" -#include "mrc/edge/forward.hpp" #include "mrc/node/generic_source.hpp" #include "mrc/node/operators/broadcast.hpp" #include "mrc/node/operators/combine_latest.hpp" @@ -41,7 +42,6 @@ #include // for observable_member #include -#include #include #include #include From 8e9089247f3a1caed5fff928001bc2db361a0b24 Mon Sep 17 00:00:00 2001 From: David Gardner Date: Thu, 28 Sep 2023 15:29:39 -0700 Subject: [PATCH 4/4] Fix type-o --- cpp/mrc/tests/test_edges.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/mrc/tests/test_edges.cpp b/cpp/mrc/tests/test_edges.cpp index 4e0629af0..91c6d4e09 100644 --- a/cpp/mrc/tests/test_edges.cpp +++ b/cpp/mrc/tests/test_edges.cpp @@ -58,7 +58,7 @@ using namespace std::chrono_literals; TEST_CLASS(Edges); -using TestEdgesDeathTest = TestEdges; // NOLINT(readability-identifier-naming)p +using TestEdgesDeathTest = TestEdges; // NOLINT(readability-identifier-naming) namespace mrc::node {