pw_compilation_testing: Add negative compilation tests

- Convert existing negative compilation tests to the new framework. - Add a negative compilation test for pw_rpc service creation. Change-Id: I204f4602dd8f49cb7c59fda04fbb1321828640ae Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/103364 Commit-Queue: Wyatt Hepler <[email protected]> Reviewed-by: Keir Mierle <[email protected]> Pigweed-Auto-Submit: Wyatt Hepler <[email protected]>
google · Aug 5, 2022 · 1b7adca · 1b7adca
1 parent 77497ce
commit 1b7adca
Show file tree

Hide file tree

Showing 21 changed files with 116 additions and 26 deletions.
diff --git a/pw_assert/BUILD.bazel b/pw_assert/BUILD.bazel
@@ -101,6 +101,7 @@ pw_cc_test(
     deps = [
         ":facade",
         "//pw_assert",
+        "//pw_compilation_testing:negative_compilation_testing",
         "//pw_preprocessor",
         "//pw_span",
         "//pw_string",

diff --git a/pw_assert/BUILD.gn b/pw_assert/BUILD.gn
@@ -209,6 +209,7 @@ pw_test("assert_facade_test") {
     dir_pw_status,
     dir_pw_string,
   ]
+  negative_compilation_tests = true
 
   # TODO(frolv): Fix this test on the QEMU target.
   enable_if = pw_build_EXECUTABLE_TARGET_TYPE != "lm3s6965evb_executable"

diff --git a/pw_assert/CMakeLists.txt b/pw_assert/CMakeLists.txt
@@ -122,6 +122,7 @@ pw_add_test(pw_assert.assert_facade_test
     pw_assert_test/fake_backend.h
   DEPS
     pw_assert
+    pw_compilation_testing._pigweed_only_negative_compilation
     pw_status
     pw_string
   GROUPS

diff --git a/pw_assert/assert_facade_test.cc b/pw_assert/assert_facade_test.cc
@@ -26,6 +26,7 @@
 // clang-format on
 
 #include "gtest/gtest.h"
+#include "pw_compilation_testing/negative_compilation.h"
 #include "pw_status/status.h"
 
 namespace {
@@ -282,6 +283,14 @@ TEST_F(AssertFailTest, FunctionPtrNotNull) {
   PW_CHECK_NOTNULL(function);
 }
 
+[[maybe_unused]] void CompareIntWithString() {
+#if PW_NC_TEST(CompareIntWithString)
+  PW_NC_EXPECT("cannot initialize|invalid conversion");
+
+  PW_CHECK_INT_EQ(123l, "This check message is accidentally compared to 123!");
+#endif  // PW_NC_TEST
+}
+
 // Note: Due to platform inconsistencies, the below test for the NOTNULL
 // message doesn't work. Some platforms print NULL formatted as %p as "(nil)",
 // others "0x0". Leaving this here for reference.

diff --git a/pw_compilation_testing/docs.rst b/pw_compilation_testing/docs.rst
@@ -14,7 +14,9 @@ scenarios, for example:
 - For a ``constexpr`` function, testing that a ``PW_ASSERT`` is triggered as
   expected.
 
-Negative compilation tests are only supported in GN currently.
+Negative compilation tests are only supported in GN currently. Negative
+compilation tests are not currently supported in GN on Windows due to
+`b/241565082 <bugs.pigweed.dev/241565082>`_.
 
 .. warning::
 

diff --git a/pw_containers/BUILD.bazel b/pw_containers/BUILD.bazel
@@ -51,7 +51,10 @@ pw_cc_library(
         "public/pw_containers/intrusive_list.h",
     ],
     includes = ["public"],
-    deps = ["//pw_assert"],
+    deps = [
+        "//pw_assert",
+        "//pw_compilation_testing:negative_compilation_testing",
+    ],
 )
 
 pw_cc_library(

diff --git a/pw_containers/BUILD.gn b/pw_containers/BUILD.gn
@@ -138,6 +138,7 @@ pw_test("intrusive_list_test") {
     ":intrusive_list",
     "$dir_pw_preprocessor",
   ]
+  negative_compilation_tests = true
 }
 
 pw_doc_group("docs") {

diff --git a/pw_containers/CMakeLists.txt b/pw_containers/CMakeLists.txt
@@ -162,6 +162,7 @@ pw_add_test(pw_containers.intrusive_list_test
   SOURCES
     intrusive_list_test.cc
   DEPS
+    pw_compilation_testing._pigweed_only_negative_compilation
     pw_containers.intrusive_list
     pw_preprocessor
   GROUPS

diff --git a/pw_containers/intrusive_list_test.cc b/pw_containers/intrusive_list_test.cc
@@ -19,6 +19,7 @@
 #include <cstdint>
 
 #include "gtest/gtest.h"
+#include "pw_compilation_testing/negative_compilation.h"
 #include "pw_preprocessor/util.h"
 
 namespace pw {
@@ -414,7 +415,8 @@ TEST(IntrusiveList, CompareConstAndNonConstIterator) {
   EXPECT_EQ(list.end(), list.cend());
 }
 
-#if defined(PW_COMPILE_FAIL_TEST_incompatible_iterator_types)
+#if PW_NC_TEST(IncompatibleIteratorTypes)
+PW_NC_EXPECT("comparison (of|between) distinct pointer types");
 
 struct OtherItem : public IntrusiveList<OtherItem>::Item {};
 
@@ -424,11 +426,11 @@ TEST(IntrusiveList, CompareConstAndNonConstIterator_CompilationFails) {
   static_cast<void>(list.end() == list2.end());
 }
 
-#endif
+#endif  // PW_NC_TEST
+
+#if PW_NC_TEST(CannotModifyThroughConstIterator)
+PW_NC_EXPECT("function is not marked const|discards qualifiers");
 
-// TODO(b/234882063): These tests should fail to compile, enable when no-compile
-// tests are set up in Pigweed.
-#if defined(PW_COMPILE_FAIL_TEST_cannot_modify_through_const_iterator)
 TEST(IntrusiveList, ConstIteratorModify) {
   TestItem item1(1);
   TestItem item2(99);
@@ -445,7 +447,7 @@ TEST(IntrusiveList, ConstIteratorModify) {
     it++;
   }
 }
-#endif  // Compile failure test
+#endif  // PW_NC_TEST
 
 // TODO(b/235289499): These tests should trigger a CHECK failure. This requires
 // using a testing version of pw_assert.
@@ -675,28 +677,32 @@ TEST(InstrusiveList, ListOfDerivedClassItems) {
 
   EXPECT_EQ(1u, derived_from_compatible_item_type.size());
 
-// TODO(b/234882063): Make these proper automated compilation failure tests.
-#if defined(PW_COMPILE_FAIL_TEST_cannot_add_base_class_to_derived_class_list)
+#if PW_NC_TEST(CannotAddBaseClassToDerivedClassList)
+  PW_NC_EXPECT_CLANG("cannot bind to a value of unrelated type");
+  PW_NC_EXPECT_GCC("cannot convert");
+
   TestItem item2;
   derived_from_compatible_item_type.push_front(item2);
 #endif
 }
 
-#if defined(PW_COMPILE_FAIL_TEST_incompatibile_item_type)
+#if PW_NC_TEST(IncompatibileItemType)
+PW_NC_EXPECT("IntrusiveList items must be derived from IntrusiveList<T>::Item");
 
 struct Foo {};
 
 class BadItem : public IntrusiveList<Foo>::Item {};
 
 [[maybe_unused]] IntrusiveList<BadItem> derived_from_incompatible_item_type;
 
-#elif defined(PW_COMPILE_FAIL_TEST_does_not_inherit_from_item)
+#elif PW_NC_TEST(DoesNotInheritFromItem)
+PW_NC_EXPECT("IntrusiveList items must be derived from IntrusiveList<T>::Item");
 
 struct NotAnItem {};
 
 [[maybe_unused]] IntrusiveList<NotAnItem> list;
 
-#endif
+#endif  // PW_NC_TEST
 
 }  // namespace
 }  // namespace pw
diff --git a/pw_function/BUILD.gn b/pw_function/BUILD.gn
@@ -68,6 +68,7 @@ pw_test("function_test") {
     dir_pw_polyfill,
   ]
   sources = [ "function_test.cc" ]
+  negative_compilation_tests = true
 }
 
 pw_size_report("function_size") {

diff --git a/pw_function/CMakeLists.txt b/pw_function/CMakeLists.txt
@@ -44,6 +44,7 @@ pw_add_test(pw_function.function_test
   SOURCES
     function_test.cc
   DEPS
+    pw_compilation_testing._pigweed_only_negative_compilation
     pw_function
     pw_polyfill
   GROUPS

diff --git a/pw_function/function_test.cc b/pw_function/function_test.cc
@@ -15,31 +15,35 @@
 #include "pw_function/function.h"
 
 #include "gtest/gtest.h"
+#include "pw_compilation_testing/negative_compilation.h"
 #include "pw_polyfill/language_feature_macros.h"
 
 namespace pw {
 namespace {
 
-// TODO(b/234882063): Convert this to a compilation failure test.
-#if defined(PW_COMPILE_FAIL_TEST_CannotInstantiateWithNonFunction)
+#if PW_NC_TEST(CannotInstantiateWithNonFunction)
+PW_NC_EXPECT("pw::Function may only be instantiated for a function type");
 
 [[maybe_unused]] Function<int> function_pointer;
 
-#elif defined(PW_COMPILE_FAIL_TEST_CannotInstantiateWithFunctionPointer1)
+#elif PW_NC_TEST(CannotInstantiateWithFunctionPointer1)
+PW_NC_EXPECT("pw::Function may only be instantiated for a function type");
 
 [[maybe_unused]] Function<void (*)()> function_pointer;
 
-#elif defined(PW_COMPILE_FAIL_TEST_CannotInstantiateWithFunctionPointer2)
+#elif PW_NC_TEST(CannotInstantiateWithFunctionPointer2)
+PW_NC_EXPECT("pw::Function may only be instantiated for a function type");
 
 [[maybe_unused]] void SomeFunction(int);
 
 [[maybe_unused]] Function<decltype(&SomeFunction)> function_pointer;
 
-#elif defined(PW_COMPILE_FAIL_TEST_CannotInstantiateWithFunctionReference)
+#elif PW_NC_TEST(CannotInstantiateWithFunctionReference)
+PW_NC_EXPECT("pw::Function may only be instantiated for a function type");
 
 [[maybe_unused]] Function<void (&)()> function_pointer;
 
-#endif  // compile fail tests
+#endif  // PW_NC_TEST
 
 // Ensure that Function can be constant initialized.
 [[maybe_unused]] PW_CONSTINIT Function<void()> can_be_constant_initialized;

diff --git a/pw_function/public/pw_function/function.h b/pw_function/public/pw_function/function.h
@@ -46,7 +46,7 @@ namespace pw {
 template <typename Callable>
 class Function {
   static_assert(std::is_function_v<Callable>,
-                "pw::Function may only be instantianted for a function type, "
+                "pw::Function may only be instantiated for a function type, "
                 "such as pw::Function<void(int)>.");
 };
 

diff --git a/pw_rpc/BUILD.gn b/pw_rpc/BUILD.gn
@@ -19,6 +19,7 @@ import("$dir_pw_build/python.gni")
 import("$dir_pw_build/python_action.gni")
 import("$dir_pw_build/target_types.gni")
 import("$dir_pw_chrono/backend.gni")
+import("$dir_pw_compilation_testing/negative_compilation_test.gni")
 import("$dir_pw_docgen/docs.gni")
 import("$dir_pw_protobuf_compiler/proto.gni")
 import("$dir_pw_sync/backend.gni")

diff --git a/pw_rpc/raw/BUILD.bazel b/pw_rpc/raw/BUILD.bazel
@@ -179,6 +179,14 @@ pw_cc_test(
     ],
 )
 
+# Negative compilation testing is not supported by Bazel. Build this as a
+# regular unit for now test.
+pw_cc_test(
+    name = "service_nc_test",
+    srcs = ["service_nc_test.cc"],
+    deps = ["//pw_rpc:pw_rpc_test_cc.raw_rpc"],
+)
+
 pw_cc_test(
     name = "stub_generation_test",
     srcs = ["stub_generation_test.cc"],

diff --git a/pw_rpc/raw/BUILD.gn b/pw_rpc/raw/BUILD.gn
@@ -15,6 +15,7 @@
 import("//build_overrides/pigweed.gni")
 
 import("$dir_pw_build/target_types.gni")
+import("$dir_pw_compilation_testing/negative_compilation_test.gni")
 import("$dir_pw_docgen/docs.gni")
 import("$dir_pw_unit_test/test.gni")
 
@@ -119,6 +120,7 @@ pw_test("client_reader_writer_test") {
 pw_test("method_test") {
   deps = [
     ":server_api",
+    ":service_nc_test",  # Pull in the service NC test through this test
     "$dir_pw_containers",
     "..:test_protos.pwpb",
     "..:test_protos.raw_rpc",
@@ -160,3 +162,8 @@ pw_test("stub_generation_test") {
   deps = [ "..:test_protos.raw_rpc" ]
   sources = [ "stub_generation_test.cc" ]
 }
+
+pw_cc_negative_compilation_test("service_nc_test") {
+  sources = [ "service_nc_test.cc" ]
+  deps = [ "..:test_protos.raw_rpc" ]
+}
diff --git a/pw_rpc/raw/CMakeLists.txt b/pw_rpc/raw/CMakeLists.txt
@@ -21,6 +21,7 @@ pw_auto_add_simple_module(pw_rpc.raw
     pw_rpc.common
     pw_rpc.server
   TEST_DEPS
+    pw_compilation_testing._pigweed_only_negative_compilation
     pw_rpc.test_protos.pwpb
     pw_rpc.test_protos.raw_rpc
     pw_rpc.test_utils

diff --git a/pw_rpc/raw/service_nc_test.cc b/pw_rpc/raw/service_nc_test.cc
@@ -0,0 +1,38 @@
+// Copyright 2021 The Pigweed Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License"); you may not
+// use this file except in compliance with the License. You may obtain a copy of
+// the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations under
+// the License.
+
+#include "pw_compilation_testing/negative_compilation.h"
+#include "pw_rpc_test_protos/test.raw_rpc.pb.h"
+
+namespace pw::rpc {
+namespace test {
+
+#if PW_NC_TEST(NoMethods)
+PW_NC_EXPECT("TestUnaryRpc");
+
+class TestService final
+    : public pw_rpc::raw::TestService::Service<TestService> {
+ public:
+};
+
+#else
+
+class TestService {};
+
+#endif  // PW_NC_TEST
+
+TestService test_service;
+
+}  // namespace test
+}  // namespace pw::rpc
diff --git a/pw_unit_test/static_library_support_test.cc b/pw_unit_test/static_library_support_test.cc
@@ -41,15 +41,17 @@ class CheckThatTestsRanWhenDestructed {
   }
 } check_that_tests_ran;
 
-// TODO(b/234882063): Convert this to a compilation failure test.
-#if defined(PW_COMPILE_FAIL_TEST_FailsToLinkInvalidTestSuite)
+// Test that linking fails if these macros refer to tests that do not exist.
+// These cannot be a negative compilation tests because they compile
+// successfully, but fail to link.
+#if 0
 
 PW_UNIT_TEST_LINK_FILE_CONTAINING_TEST(NotARealSuite, NotARealTest);
 
-#elif defined(PW_COMPILE_FAIL_TEST_FailsToLinkInvalidTestName)
+#elif 0
 
 PW_UNIT_TEST_LINK_FILE_CONTAINING_TEST(StaticLibraryArchivedTest, NotARealTest);
 
-#endif  // compile fail tests
+#endif  // negative linking tests
 
 }  // namespace pw::unit_test
diff --git a/targets/host/target_toolchains.gni b/targets/host/target_toolchains.gni
@@ -307,7 +307,8 @@ pw_target_toolchain_host_list = [
 _pigweed_internal = {
   pw_status_CONFIG = "$dir_pw_status:check_if_used"
 
-  pw_compilation_testing_NEGATIVE_COMPILATION_ENABLED = true
+  # TODO(b/241565082): Enable NC testing in GN Windows when it is fixed.
+  pw_compilation_testing_NEGATIVE_COMPILATION_ENABLED = host_os != "win"
 }
 
 # Host toolchains exclusively for upstream Pigweed use. To give upstream Pigweed

diff --git a/targets/stm32f429i_disc1/target_toolchains.gni b/targets/stm32f429i_disc1/target_toolchains.gni
@@ -25,7 +25,8 @@ declare_args() {
 }
 
 _target_config = {
-  pw_compilation_testing_NEGATIVE_COMPILATION_ENABLED = true
+  # TODO(b/241565082): Enable NC testing in GN Windows when it is fixed.
+  pw_compilation_testing_NEGATIVE_COMPILATION_ENABLED = host_os != "win"
 
   # Use the logging main.
   pw_unit_test_MAIN = "$dir_pw_unit_test:logging_main"