docs/style: Require unit tests to be in unnamed namespace

- Mention in the style guide that unit tests must be declared in an unnamed namespace to avoid linking issues. - Add unnamed namespaces to tests that are missing them. Change-Id: I806ffb96aca8d3271faf24510e5d51edae725e90 Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/231211 Lint: Lint 🤖 <[email protected]> Commit-Queue: Auto-Submit <[email protected]> Reviewed-by: Alexei Frolov <[email protected]> Presubmit-Verified: CQ Bot Account <[email protected]> Pigweed-Auto-Submit: Wyatt Hepler <[email protected]>
google · Aug 21, 2024 · b816ed5 · b816ed5
1 parent 972e2d4
commit b816ed5
Show file tree

Hide file tree

Showing 27 changed files with 81 additions and 5 deletions.
diff --git a/docs/style/cpp.rst b/docs/style/cpp.rst
@@ -618,7 +618,9 @@ C++ code
 * All Pigweed C++ code must be in the ``pw`` namespace. Namespaces for modules
   should be nested under ``pw``. For example, ``pw::string::Format()``.
 * Whenever possible, private code should be in a source (.cc) file and placed in
-  anonymous namespace nested under ``pw``.
+  anonymous namespace nested under ``pw``. Unit tests must be declared in an
+  anonymous namespace to avoid potential linking issues when building multiple
+  tests in one binary.
 * If private code must be exposed in a header file, it must be in a namespace
   nested under ``pw``. The namespace may be named for its subsystem or use a
   name that designates it as private, such as ``internal``.

diff --git a/pw_assert/assert_test.cc b/pw_assert/assert_test.cc
@@ -17,6 +17,8 @@
 #include "pw_status/status.h"
 #include "pw_unit_test/framework.h"
 
+namespace {
+
 // PW_ASSERT() should always be enabled, and always evaluate the expression.
 TEST(Assert, AssertTrue) {
   int evaluated = 1;
@@ -60,3 +62,5 @@ TEST(Assert, DebugAssertFalse) {
     PW_DASSERT(false);
   }
 }
+
+}  // namespace
diff --git a/pw_async_basic/dispatcher_test.cc b/pw_async_basic/dispatcher_test.cc
@@ -28,6 +28,7 @@
 using namespace std::chrono_literals;
 
 namespace pw::async {
+namespace {
 
 // Lambdas can only capture one ptr worth of memory without allocating, so we
 // group the data we want to share between tasks and their containing tests
@@ -228,4 +229,5 @@ TEST(DispatcherBasic, TasksCancelledByRunFor) {
   ASSERT_EQ(count, 3);
 }
 
+}  // namespace
 }  // namespace pw::async
diff --git a/pw_build/file_prefix_map_test.cc b/pw_build/file_prefix_map_test.cc
@@ -15,6 +15,7 @@
 #include "pw_build_private/file_prefix_map_test.h"
 
 namespace pw::build::test {
+namespace {
 
 static_assert(StringsAreEqual("", ""));
 static_assert(StringsAreEqual("test", "test"));
@@ -27,4 +28,5 @@ static_assert(StringsAreEqual(PW_BUILD_EXPECTED_SOURCE_PATH, __FILE__),
               "The __FILE__ macro should be " PW_BUILD_EXPECTED_SOURCE_PATH
               ", but it is " __FILE__);
 
+}  // namespace
 }  // namespace pw::build::test
diff --git a/pw_build/module_config_test.cc b/pw_build/module_config_test.cc
@@ -15,7 +15,9 @@
 #include "pw_thread_freertos/config.h"
 
 namespace pw::build::test {
+namespace {
 
 static_assert(PW_THREAD_FREERTOS_CONFIG_JOINING_ENABLED == 1);
 
+}
 }  // namespace pw::build::test
diff --git a/pw_i2c/address_test.cc b/pw_i2c/address_test.cc
@@ -17,6 +17,7 @@
 #include "pw_unit_test/framework.h"
 
 namespace pw::i2c {
+namespace {
 
 TEST(Address, SevenBitConstexpr) {
   constexpr Address kSevenBit =
@@ -45,4 +46,5 @@ TEST(Address, TenBitRuntimeChecked) {
 // TODO: b/234882063 - Add tests to ensure the constexpr constructors fail to
 // compile with invalid addresses once no-copmile tests are set up in Pigweed.
 
+}  // namespace
 }  // namespace pw::i2c
diff --git a/pw_log/basic_log_test.cc b/pw_log/basic_log_test.cc
@@ -28,6 +28,8 @@
 
 #include "pw_unit_test/framework.h"
 
+namespace {
+
 // TODO: b/235291136 - Test unsigned integer logging (32 and 64 bit); test
 // pointer logging.
 
@@ -245,3 +247,5 @@ TEST(CustomFormatString, ErrorLevel) {
 TEST(CustomFormatString, CriticalLevel) {
   PW_LOG_CRITICAL("Critical, emergency log. Device should not reboot");
 }
+
+}  // namespace
diff --git a/pw_log/proto_utils_test.cc b/pw_log/proto_utils_test.cc
@@ -23,6 +23,7 @@
 #include "pw_unit_test/framework.h"
 
 namespace pw::log {
+namespace {
 
 void VerifyTokenizedLogEntry(pw::protobuf::Decoder& entry_decoder,
                              pw::log_tokenized::Metadata expected_metadata,
@@ -504,4 +505,5 @@ TEST(UtilsTest, EncodeLog_InsufficientSpace) {
   EXPECT_TRUE(result.status().IsResourceExhausted());
 }
 
+}  // namespace
 }  // namespace pw::log
diff --git a/pw_metric/global_test.cc b/pw_metric/global_test.cc
@@ -20,6 +20,7 @@
 
 namespace pw {
 namespace metric {
+namespace {
 
 // Create two global metrics; make sure they show up.
 PW_METRIC_GLOBAL(stat_x, "stat_x", 123u);
@@ -53,9 +54,14 @@ TEST(Global, Groups) {
   EXPECT_EQ(power_metrics.metrics().size(), 3u);
 }
 
+}  // namespace
 }  // namespace metric
 }  // namespace pw
 
+namespace {
+
 // this is a compilation test to make sure metrics can be defined outside of
 // ::pw::metric
 PW_METRIC_GROUP_GLOBAL(global_group, "global group");
+
+}  // namespace
diff --git a/pw_metric/metric_test.cc b/pw_metric/metric_test.cc
@@ -18,6 +18,7 @@
 #include "pw_unit_test/framework.h"
 
 namespace pw::metric {
+namespace {
 
 TEST(Metric, FloatFromObject) {
   // Note leading bit is 1; it is stripped from the name to store the type.
@@ -96,6 +97,11 @@ PW_METRIC_GROUP(global_group, "a_global_group");
 PW_METRIC(global_group, global_z, "global_x", 5555u);
 PW_METRIC(global_group, global_w, "global_y", 6.0f);
 
+TEST(Metric, GlobalScope) {
+  EXPECT_EQ(global_x.value(), 5555u);
+  EXPECT_EQ(global_y.value(), 6.0f);
+}
+
 // A fake object to illustrate the API and show nesting metrics.
 // This also tests creating metrics as members inside a class.
 class I2cBus {
@@ -237,4 +243,5 @@ TEST(Metric, StaticWithinAFunction) {
   EXPECT_EQ(metric->as_int(), 2u);
 }
 
+}  // namespace
 }  // namespace pw::metric
diff --git a/pw_multisink/multisink_test.cc b/pw_multisink/multisink_test.cc
@@ -26,6 +26,8 @@
 #include "pw_unit_test/framework.h"
 
 namespace pw::multisink {
+namespace {
+
 using Drain = MultiSink::Drain;
 using Listener = MultiSink::Listener;
 
@@ -662,4 +664,5 @@ TEST(UnsafeIteration, Subset) {
   EXPECT_EQ(kExpectedEntriesMaxEntries, entry_count);
 }
 
+}  // namespace
 }  // namespace pw::multisink
diff --git a/pw_protobuf/map_utils_test.cc b/pw_protobuf/map_utils_test.cc
@@ -23,6 +23,7 @@
 #define ASSERT_OK(status) ASSERT_EQ(OkStatus(), status)
 
 namespace pw::protobuf {
+namespace {
 
 TEST(ProtoHelper, WriteProtoStringToBytesMapEntry) {
   // The following defines an instance of the message below:
@@ -153,4 +154,5 @@ TEST(ProtoHelper, WriteProtoStringToBytesMapEntryInvalidArgument) {
       Status::InvalidArgument());
 }
 
+}  // namespace
 }  // namespace pw::protobuf
diff --git a/pw_protobuf/message_test.cc b/pw_protobuf/message_test.cc
@@ -20,6 +20,7 @@
 #define ASSERT_OK(status) ASSERT_EQ(OkStatus(), status)
 
 namespace pw::protobuf {
+namespace {
 
 TEST(ProtoHelper, IterateMessage) {
   // clang-format off
@@ -769,4 +770,5 @@ TEST(ProtoHelper, AsStringToBytesMapMalformed) {
   ASSERT_EQ(count, 2ULL);
 }
 
+}  // namespace
 }  // namespace pw::protobuf
diff --git a/pw_protobuf_compiler/nanopb_test.cc b/pw_protobuf_compiler/nanopb_test.cc
@@ -15,9 +15,13 @@
 #include "pw_protobuf_compiler/nanopb_test_protos/nanopb_test.pb.h"
 #include "pw_unit_test/framework.h"
 
+namespace {
+
 TEST(Nanopb, CompilesProtobufs) {
   pw_protobuf_compiler_nanopb_test_protos_Point point = {4, 8, "point"};
   EXPECT_EQ(point.x, 4u);
   EXPECT_EQ(point.y, 8u);
   EXPECT_EQ(sizeof(point.name), 16u);
 }
+
+}  // namespace
diff --git a/pw_rpc/nanopb/server_callback_test.cc b/pw_rpc/nanopb/server_callback_test.cc
@@ -22,6 +22,7 @@
 #include "pw_unit_test/framework.h"
 
 namespace pw::rpc {
+namespace {
 
 class TestServiceImpl final
     : public test::pw_rpc::nanopb::TestService::Service<TestServiceImpl> {
@@ -110,4 +111,5 @@ TEST(NanopbTestMethodContext, ResponseWithCallbacks) {
   EXPECT_EQ(9u, decoder_context.values[2]);
 }
 
+}  // namespace
 }  // namespace pw::rpc
diff --git a/pw_rpc/nanopb/server_reader_writer_test.cc b/pw_rpc/nanopb/server_reader_writer_test.cc
@@ -23,6 +23,7 @@
 #include "pw_unit_test/framework.h"
 
 namespace pw::rpc {
+namespace {
 
 class TestServiceImpl final
     : public test::pw_rpc::nanopb::TestService::Service<TestServiceImpl> {
@@ -423,4 +424,5 @@ TEST(NanopbServerReader, CallbacksMoveCorrectly) {
   EXPECT_EQ(request.status_code, received_request.status_code);
 }
 
+}  // namespace
 }  // namespace pw::rpc
diff --git a/pw_rpc/raw/server_reader_writer_test.cc b/pw_rpc/raw/server_reader_writer_test.cc
@@ -24,6 +24,7 @@
 #include "pw_unit_test/framework.h"
 
 namespace pw::rpc {
+namespace {
 
 class TestServiceImpl final
     : public test::pw_rpc::raw::TestService::Service<TestServiceImpl> {
@@ -558,4 +559,5 @@ TEST(RawServerReaderWriter, UsableAsWriter) {
       kWriterData);
 }
 
+}  // namespace
 }  // namespace pw::rpc
diff --git a/pw_rpc/raw/service_nc_test.cc b/pw_rpc/raw/service_nc_test.cc
@@ -17,6 +17,7 @@
 
 namespace pw::rpc {
 namespace test {
+namespace {
 
 #if PW_NC_TEST(NoMethods)
 PW_NC_EXPECT("TestUnaryRpc");
@@ -32,7 +33,8 @@ class TestService {};
 
 #endif  // PW_NC_TEST
 
-TestService test_service;
+[[maybe_unused]] TestService test_service;
 
+}  // namespace
 }  // namespace test
 }  // namespace pw::rpc
diff --git a/pw_software_update/bundled_update_service_pwpb_test.cc b/pw_software_update/bundled_update_service_pwpb_test.cc
@@ -16,4 +16,8 @@
 
 #include "pw_unit_test/framework.h"
 
+namespace {
+
 TEST(BundledUpdateServicePwpb, Compiles) {}
+
+}  // namespace
diff --git a/pw_software_update/bundled_update_service_test.cc b/pw_software_update/bundled_update_service_test.cc
@@ -16,4 +16,8 @@
 
 #include "pw_unit_test/framework.h"
 
+namespace {
+
 TEST(BundledUpdateService, Compiles) {}
+
+}  // namespace
diff --git a/pw_string/string_test.cc b/pw_string/string_test.cc
@@ -22,6 +22,7 @@
 #include "pw_unit_test/framework.h"
 
 namespace pw {
+namespace {
 
 using namespace std::string_view_literals;
 
@@ -538,14 +539,14 @@ TEST(InlineString, Construct_StringView) {
   PW_NC_EXPECT_CLANG("no matching function for call to 'TakesInlineString'");
   PW_NC_EXPECT_GCC(
       "invalid initialization of reference of type .* from expression of type "
-      "'const pw::StringViewLike<char>'");
+      "'const pw::.*StringViewLike<char>'");
   TakesInlineString(kStringViewLike10);
 #elif PW_NC_TEST(Construct_StringView_NoImplicitConvFromStringViewLikeInInit1)
-  PW_NC_EXPECT_GCC("could not convert 'pw::kStringViewLike10'");
+  PW_NC_EXPECT_GCC("could not convert 'pw::.*kStringViewLike10'");
   PW_NC_EXPECT_CLANG("no viable conversion from 'const StringViewLike<char>'");
   (void)HoldsString{.value = kStringViewLike10};
 #elif PW_NC_TEST(Construct_StringView_NoImplicitConvFromStringViewLikeInInit2)
-  PW_NC_EXPECT_GCC("could not convert 'pw::kStringViewLike10'");
+  PW_NC_EXPECT_GCC("could not convert 'pw::.*kStringViewLike10'");
   PW_NC_EXPECT_CLANG("no viable conversion from 'const StringViewLike<char>'");
   (void)HoldsString{kStringViewLike10};
 #endif  // PW_NC_TEST
@@ -2128,4 +2129,5 @@ TEST(BasicStrings, ByteString) {
   EXPECT_LT(bytes, higher_bytes);
 }
 
+}  // namespace
 }  // namespace pw
diff --git a/pw_sync/lock_traits_test.cc b/pw_sync/lock_traits_test.cc
@@ -18,6 +18,7 @@
 #include "pw_unit_test/framework.h"
 
 namespace pw::sync::test {
+namespace {
 
 struct NotALock {};
 
@@ -59,4 +60,5 @@ TEST(LockTraitsTest, IsTimedLockable) {
   EXPECT_TRUE((is_timed_lockable_v<FakeTimedLockable, FakeClock>));
 }
 
+}  // namespace
 }  // namespace pw::sync::test
diff --git a/pw_tls_client_boringssl/tls_client_boringssl_test.cc b/pw_tls_client_boringssl/tls_client_boringssl_test.cc
@@ -21,11 +21,13 @@
 #include "test_certs_and_keys.h"
 
 namespace pw::tls_client {
+namespace {
 
 TEST(TLSClientBoringSSL, SessionCreationSucceeds) {
   SessionOptions options;
   auto res = Session::Create(options);
   ASSERT_EQ(res.status(), PW_STATUS_UNIMPLEMENTED);
 }
 
+}  // namespace
 }  // namespace pw::tls_client
diff --git a/pw_tls_client_mbedtls/tls_client_mbedtls_test.cc b/pw_tls_client_mbedtls/tls_client_mbedtls_test.cc
@@ -17,6 +17,7 @@
 #include "pw_unit_test/framework.h"
 
 namespace pw::tls_client {
+namespace {
 
 TEST(TLSClientMbedTLS, CreateSucceed) {
   auto options = SessionOptions().set_transport(stream::NullStream::Instance());
@@ -38,4 +39,5 @@ TEST(TLSClientMbedTLS, EntropySourceFail) {
   ASSERT_NE(res.status(), OkStatus());
 }
 
+}  // namespace
 }  // namespace pw::tls_client
diff --git a/pw_tokenizer/encode_args_test.cc b/pw_tokenizer/encode_args_test.cc
@@ -17,6 +17,7 @@
 #include "pw_unit_test/framework.h"
 
 namespace pw::tokenizer {
+namespace {
 
 static_assert(MinEncodingBufferSizeBytes<>() == 4);
 static_assert(MinEncodingBufferSizeBytes<bool>() == 4 + 2);
@@ -49,4 +50,5 @@ TEST(TokenizerCEncodingFunctions, EncodeInt64) {
   EXPECT_EQ(buffer[0], 2);  // 1 encodes to 2 with ZigZag
 }
 
+}  // namespace
 }  // namespace pw::tokenizer
diff --git a/pw_trace_tokenized/trace_buffer_test.cc b/pw_trace_tokenized/trace_buffer_test.cc
@@ -19,6 +19,8 @@
 #include "pw_trace/trace.h"
 #include "pw_unit_test/framework.h"
 
+namespace {
+
 TEST(TokenizedTrace, Simple) {
   PW_TRACE_SET_ENABLED(true);
   pw::trace::ClearBuffer();
@@ -163,3 +165,5 @@ TEST(TokenizedTrace, DeringAndViewRawBuffer) {
   buf = pw::trace::DeringAndViewRawBuffer();
   EXPECT_GT(buf.size(), size_start);
 }
+
+}  // namespace