Simplify access to private members from tests

libmuscle/cpp/build/libmuscle/tests/Makefile

@@ -47,7 +47,7 @@ EXTRA_LINK_DIRS := $(foreach DIR,$(DEP_DIRS),-Wl,-rpath,$(DIR)/lib)
 
 CXXFLAGS += -I$(libmuscle_testdir) -isystem $(googletest_ROOT)/include -pthread
 CXXFLAGS += $(shell export PKG_CONFIG_PATH=$(PKG_CONFIG_PATH):$(PKG_CONFIG_EXTRA_DIRS) ; pkg-config --cflags msgpack)
-CXXFLAGS += $(DEBUGFLAGS)
+CXXFLAGS += -DTESTING $(DEBUGFLAGS)
 
 LDFLAGS += $(CURDIR)/../libmuscle_d.a $(CURDIR)/../../ymmsl/libymmsl_d.a
 LDFLAGS += $(googletest_LIB) -pthread

libmuscle/cpp/src/libmuscle/communicator.hpp

@@ -15,6 +15,7 @@
 #include <libmuscle/ports_description.hpp>
 #include <libmuscle/post_office.hpp>
 #include <libmuscle/profiler.hpp>
+#include <libmuscle/test_support.hpp>
 #include <libmuscle/util.hpp>
 
 #include <ymmsl/ymmsl.hpp>
@@ -181,7 +182,7 @@ class Communicator {
          */
         void restore_message_counts(PortMessageCounts const & port_message_counts);
 
-    private:
+    PRIVATE:
         using Ports_ = std::unordered_map<std::string, Port>;
 
         ymmsl::Reference instance_id_() const;
@@ -214,8 +215,6 @@ class Communicator {
         Ports_ ports_;
         std::unique_ptr<PeerManager> peer_manager_;
         Optional<Port> muscle_settings_in_;
-
-        friend class TestCommunicator;
 };
 
 } }

libmuscle/cpp/src/libmuscle/instance.cpp

@@ -11,6 +11,7 @@
 #include <libmuscle/profiling.hpp>
 #include <libmuscle/settings_manager.hpp>
 #include <libmuscle/snapshot_manager.hpp>
+#include <libmuscle/test_support.hpp>
 #include <libmuscle/checkpoint_triggers.hpp>
 
 #include <ymmsl/ymmsl.hpp>
@@ -109,7 +110,7 @@ class Instance::Impl {
         bool should_save_final_snapshot();
         void save_final_snapshot(Message message);
 
-    private:
+    PRIVATE:
         ::ymmsl::Reference instance_name_;
         std::unique_ptr<MMPClient> manager_;
         std::unique_ptr<Logger> logger_;
@@ -173,8 +174,6 @@ class Instance::Impl {
         void close_incoming_ports_();
         void close_ports_();
         void shutdown_();
-
-        friend class TestInstance;
 };
 
 Instance::Impl::Impl(

libmuscle/cpp/src/libmuscle/instance.hpp

@@ -3,6 +3,7 @@
 #include <libmuscle/message.hpp>
 #include <libmuscle/namespace.hpp>
 #include <libmuscle/ports_description.hpp>
+#include <libmuscle/test_support.hpp>
 
 #ifdef MUSCLE_ENABLE_MPI
 #include <mpi.h>
@@ -779,14 +780,12 @@ class Instance {
          */
         void save_final_snapshot(Message message);
 
-    private:
+    PRIVATE:
         class Impl;
         std::unique_ptr<Impl> pimpl_;
 
         Impl const * impl_() const;
         Impl * impl_();
-
-        friend class TestInstance;
 };
 
 } }

libmuscle/cpp/src/libmuscle/profiler.hpp

@@ -8,6 +8,7 @@
 #include <libmuscle/mmp_client.hpp>
 #include <libmuscle/namespace.hpp>
 #include <libmuscle/profiling.hpp>
+#include <libmuscle/test_support.hpp>
 #include <libmuscle/timestamp.hpp>
 
 #include <condition_variable>
@@ -61,9 +62,7 @@ class Profiler {
          */
         void record_event(ProfileEvent && event);
 
-    private:
-        friend class TestProfiler;
-
+    PRIVATE:
         // mutex_ protects all member variables and flush_()
         std::mutex mutex_;
 

libmuscle/cpp/src/libmuscle/test_support.hpp

@@ -0,0 +1,15 @@
+#pragma once
+
+
+#ifdef TESTING
+
+#define PRIVATE public
+#define PROTECTED public
+
+#else
+
+#define PRIVATE private
+#define PROTECTED protected
+
+#endif
+

libmuscle/cpp/src/libmuscle/tests/mocks/mock_communicator.hpp

@@ -81,9 +81,6 @@ class MockCommunicator {
         static Optional<int> last_sent_slot;
         static PortMessageCounts get_message_counts_return_value;
         static PortMessageCounts last_restored_message_counts;
-
-    private:
-        friend class TestCommunicator;
 };
 
 using Communicator = MockCommunicator;

libmuscle/cpp/src/libmuscle/tests/test_communicator.cpp

@@ -71,23 +71,6 @@ int main(int argc, char *argv[]) {
 }
 
 
-// Helpers for accessing internal state
-namespace libmuscle { namespace _MUSCLE_IMPL_NS {
-
-class TestCommunicator {
-    public:
-        static std::unordered_map<std::string, libmuscle::_MUSCLE_IMPL_NS::Port> const & ports_(
-                Communicator const & comm)
-        {
-            return comm.ports_;
-        }
-};
-
-} }
-
-using libmuscle::_MUSCLE_IMPL_NS::TestCommunicator;
-
-
 /* Mocks have internal state, which needs to be reset before each test. This
  * means that the tests are not reentrant, and cannot be run in parallel.
  * It's all fast enough, so that's not a problem.
@@ -222,7 +205,7 @@ TEST(libmuscle_communicator, test_connect) {
     ASSERT_EQ(MockPeerManager::last_constructed_index, std::vector<int>({13}));
 
     // check inferred ports
-    auto const & ports = TestCommunicator::ports_(comm);
+    auto const & ports = comm.ports_;
 
     ASSERT_EQ(ports.at("in").name, "in");
     ASSERT_EQ(ports.at("in").oper, Operator::F_INIT);
@@ -277,7 +260,7 @@ TEST(libmuscle_communicator, test_connect_vector_ports) {
     ASSERT_EQ(MockPeerManager::last_constructed_peer_dims, peer_dims);
     ASSERT_EQ(MockPeerManager::last_constructed_peer_locations, peer_locations);
 
-    auto const & ports = TestCommunicator::ports_(comm);
+    auto const & ports = comm.ports_;
 
     ASSERT_EQ(ports.at("in").name, "in");
     ASSERT_EQ(ports.at("in").oper, Operator::F_INIT);
@@ -372,7 +355,7 @@ TEST(libmuscle_communicator, test_connect_inferred_ports) {
     ASSERT_EQ(MockPeerManager::last_constructed_peer_dims, peer_dims);
     ASSERT_EQ(MockPeerManager::last_constructed_peer_locations, peer_locations);
 
-    auto const & ports = TestCommunicator::ports_(comm);
+    auto const & ports = comm.ports_;
 
     ASSERT_EQ(ports.at("in").name, "in");
     ASSERT_EQ(ports.at("in").oper, Operator::F_INIT);
@@ -718,7 +701,7 @@ TEST(libmuscle_communicator, port_discard_error_on_resume) {
             {"out", {0}},
             {"in", {2}},
             {"muscle_settings_in", {0}}});
-    auto & ports = TestCommunicator::ports_(*comm);
+    auto & ports = comm->ports_;
     for (auto const & port : ports) {
         ASSERT_TRUE(port.second.is_resuming());
     }
@@ -748,7 +731,7 @@ TEST(libmuscle_communicator, port_discard_success_on_resume) {
             {"out", {0}},
             {"in", {2}},
             {"muscle_settings_in", {0}}});
-    auto & ports = TestCommunicator::ports_(*comm);
+    auto & ports = comm->ports_;
     for (auto const & port : ports) {
         ASSERT_TRUE(port.second.is_resuming());
     }

libmuscle/cpp/src/libmuscle/tests/test_instance.cpp

@@ -53,24 +53,6 @@ int main(int argc, char *argv[]) {
 }
 
 
-// Helpers for accessing internal state
-namespace libmuscle { namespace _MUSCLE_IMPL_NS {
-
-class TestInstance {
-    public:
-        static Reference & instance_name_(Instance & instance) {
-            return instance.impl_()->instance_name_;
-        }
-
-        static SettingsManager & settings_manager_(Instance & instance) {
-            return instance.impl_()->settings_manager_;
-        }
-};
-
-} }
-
-using libmuscle::_MUSCLE_IMPL_NS::TestInstance;
-
 /* Mocks have internal state, which needs to be reset before each test. This
  * means that the tests are not reentrant, and cannot be run in parallel.
  * It's all fast enough, so that's not a problem.
@@ -98,15 +80,15 @@ TEST(libmuscle_instance, create_instance) {
                 {Operator::O_F, {"out1", "out2[]"}}
                 }));
 
-    ASSERT_EQ(TestInstance::instance_name_(instance), "test_instance[13][42]");
+    ASSERT_EQ(instance.impl_()->instance_name_, "test_instance[13][42]");
     ASSERT_EQ(MockMMPClient::num_constructed, 1);
     ASSERT_EQ(MockMMPClient::last_instance_id, "test_instance[13][42]");
     ASSERT_EQ(MockMMPClient::last_location, "node042:9000");
     ASSERT_EQ(MockCommunicator::num_constructed, 1);
     ASSERT_EQ(MockMMPClient::last_registered_locations.at(0), "tcp:test1,test2");
     ASSERT_EQ(MockMMPClient::last_registered_locations.at(1), "tcp:test3");
     ASSERT_EQ(MockMMPClient::last_registered_ports.size(), 3);
-    auto & settings = TestInstance::settings_manager_(instance);
+    auto & settings = instance.impl_()->settings_manager_;
     ASSERT_EQ(settings.base["test_int"], 10);
     ASSERT_EQ(settings.base["test_string"], "testing");
     ASSERT_TRUE(settings.overlay.empty());
@@ -163,7 +145,7 @@ TEST(libmuscle_instance, get_setting) {
     settings["test4"] = 10000000000l;    // does not fit 32 bits
     settings["test5"] = 10.0;
     settings["test6"] = 1.0f / 3.0f;     // not exactly representable
-    TestInstance::settings_manager_(instance).base = settings;
+    instance.impl_()->settings_manager_.base = settings;
 
     ASSERT_TRUE(instance.get_setting("test1").is_a<std::string>());
     ASSERT_EQ(instance.get_setting("test1").as<std::string>(), "test");
@@ -402,9 +384,9 @@ TEST(libmuscle_instance, reuse_instance_receive_overlay) {
 
     instance.reuse_instance();
 
-    ASSERT_EQ(TestInstance::settings_manager_(instance).overlay.size(), 2);
-    ASSERT_EQ(TestInstance::settings_manager_(instance).overlay.at("test1"), 24);
-    ASSERT_EQ(TestInstance::settings_manager_(instance).overlay.at("test2"), "abc");
+    ASSERT_EQ(instance.impl_()->settings_manager_.overlay.size(), 2);
+    ASSERT_EQ(instance.impl_()->settings_manager_.overlay.at("test1"), 24);
+    ASSERT_EQ(instance.impl_()->settings_manager_.overlay.at("test2"), "abc");
 }
 
 TEST(libmuscle_instance, reuse_instance_closed_port) {

libmuscle/cpp/src/libmuscle/tests/test_profiler.cpp

@@ -60,23 +60,6 @@ bool operator==(Port const & lhs, Port const & rhs) {
 
 namespace libmuscle { namespace _MUSCLE_IMPL_NS {
 
-// Helper for accessing internal state
-
-class TestProfiler {
-    public:
-        static std::size_t num_queued_events(Profiler & profiler) {
-            std::lock_guard<std::mutex> lock(profiler.mutex_);
-            return profiler.events_.size();
-        }
-
-        static ProfileEvent get_queued_event(
-                Profiler & profiler, std::size_t i) {
-            std::lock_guard<std::mutex> lock(profiler.mutex_);
-            return profiler.events_.at(i);
-        }
-};
-
-
 // Helpers for comparison, not needed in the main code so put here.
 bool operator==(ProfileTimestamp const & lhs, ProfileTimestamp const & rhs) {
     return lhs.nanoseconds == rhs.nanoseconds;
@@ -116,7 +99,17 @@ bool operator==(ProfileEvent const & lhs, ProfileEvent const & rhs) {
 
 } }
 
-using libmuscle::_MUSCLE_IMPL_NS::TestProfiler;
+// Profiler has an internal mutex, which we need to lock when accessing
+std::size_t num_queued_events(Profiler & profiler) {
+    std::lock_guard<std::mutex> lock(profiler.mutex_);
+    return profiler.events_.size();
+}
+
+ProfileEvent get_queued_event(
+        Profiler & profiler, std::size_t i) {
+    std::lock_guard<std::mutex> lock(profiler.mutex_);
+    return profiler.events_.at(i);
+}
 
 
 /* Mocks have internal state, which needs to be reset before each test. This
@@ -140,7 +133,7 @@ TEST(libmuscle_profiler, test_recording_events) {
 
     ASSERT_EQ(e.start_time, t1);
     ASSERT_EQ(e.stop_time, t2);
-    ASSERT_EQ(TestProfiler::get_queued_event(profiler, 0), e);
+    ASSERT_EQ(get_queued_event(profiler, 0), e);
 }
 
 
@@ -157,7 +150,7 @@ TEST(libmuscle_profiler, test_disabling) {
 
     ASSERT_EQ(e.start_time, t1);
     ASSERT_EQ(e.stop_time, t2);
-    ASSERT_EQ(TestProfiler::num_queued_events(profiler), 0u);
+    ASSERT_EQ(num_queued_events(profiler), 0u);
 }
 
 
@@ -178,7 +171,7 @@ TEST(libmuscle_profiler, test_auto_stop_time) {
 
     profiler.record_event(std::move(e));
 
-    auto const & e2 = TestProfiler::get_queued_event(profiler, 0);
+    auto const & e2 = get_queued_event(profiler, 0);
     ASSERT_EQ(e2.start_time, t1);
     ASSERT_TRUE(e2.stop_time.is_set());
     ASSERT_LT(e2.start_time.get(), e2.stop_time.get());