Skip to content

Commit

Permalink
pw_unit_test: Fix googletest backend
Browse files Browse the repository at this point in the history
With this change, you can use the googletest backend with
simple_printing_main.

The most important changes here are,

1. Making //pw_unit_test directly expose the pw_unit_test/framework.h
   header.
2. Adding the dep on googletest_handler_adapter to the googletest
   target. This was a missing dependency before.

But doing (2) requires removing a bunch of target_compatible_with
attributes that produced a circular dependency. When the backend is set
to googletest, pw_unit_test depends on googletest, which depends on
googletest_handler_adapter. But the googletest_handler_adapter used to
have a dependency on pw_unit_test itself, through the config_setting
that depends on pw_unit_test.

I also remove the target_compatible_with on the "framework_test". This
test should be compatible with all backends, and indeed passes for the
googletest backend.

Internal-only consequences: this change makes is a layering check
violation to #include "gtest/gtest.h" from a pw_cc_test that doesn't
directly depend on googletest. This is good: tests that use pw_unit_test
as a framework should #include "pw_unit_test/framework.h" instead,
which provides only that subset of gtest.h symbols that are actually
supported when building an on-device test.

Test: bazel test --//targets:pw_unit_test_backend=//pw_unit_test:googletest //...
Fixed: b/324116813
Change-Id: I1bd77d936adcfcbcf5122fabd6a9b9f158926188
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/190593
Presubmit-Verified: CQ Bot Account <[email protected]>
Reviewed-by: Armando Montanez <[email protected]>
Commit-Queue: Ted Pudlik <[email protected]>
  • Loading branch information
tpudlik authored and CQ Bot Account committed Feb 23, 2024
1 parent 277297b commit ff074e7
Showing 1 changed file with 27 additions and 41 deletions.
68 changes: 27 additions & 41 deletions pw_unit_test/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -39,16 +39,12 @@ label_flag(
build_setting_default = "//pw_build:default_module_config",
)

alias(
name = "pw_unit_test",
actual = "@pigweed//targets:pw_unit_test_backend",
)

cc_library(
name = "framework",
name = "pw_unit_test",
testonly = True,
hdrs = ["public/pw_unit_test/framework.h"],
includes = ["public"],
deps = ["@pigweed//targets:pw_unit_test_backend"],
)

cc_library(
Expand All @@ -71,7 +67,6 @@ cc_library(
deps = [
":config",
":event_handler",
":framework",
"//pw_assert",
"//pw_polyfill",
"//pw_preprocessor",
Expand All @@ -88,7 +83,7 @@ cc_library(
],
includes = ["googletest_public_overrides"],
deps = [
":framework",
":googletest_handler_adapter",
"@com_google_googletest//:gtest",
],
)
Expand All @@ -100,6 +95,10 @@ config_setting(
flag_values = {
"@pigweed//targets:pw_unit_test_backend": "@pigweed//pw_unit_test:light",
},
# Do not depend on this config_setting outside upstream Pigweed. Config
# settings based on label flags are unreliable. See
# https://github.com/bazelbuild/bazel/issues/21189.
visibility = ["//:__subpackages__"],
)

config_setting(
Expand All @@ -108,6 +107,10 @@ config_setting(
flag_values = {
"@pigweed//targets:pw_unit_test_backend": "@pigweed//pw_unit_test:googletest",
},
# Do not depend on this config_setting outside upstream Pigweed. Config
# settings based on label flags are unreliable. See
# https://github.com/bazelbuild/bazel/issues/21189.
visibility = ["//:__subpackages__"],
)

cc_library(
Expand All @@ -132,10 +135,6 @@ cc_library(
srcs = ["googletest_handler_adapter.cc"],
hdrs = ["public/pw_unit_test/googletest_handler_adapter.h"],
includes = ["public"],
target_compatible_with = select({
":gtest_setting": [],
"//conditions:default": ["@platforms//:incompatible"],
}),
deps = [
":event_handler",
"//pw_preprocessor",
Expand All @@ -148,10 +147,6 @@ cc_library(
testonly = True,
hdrs = ["public/pw_unit_test/googletest_test_matchers.h"],
includes = ["public"],
target_compatible_with = select({
":gtest_setting": [],
"//conditions:default": ["@platforms//:incompatible"],
}),
deps = [
"//pw_result",
"//pw_status",
Expand All @@ -162,10 +157,6 @@ cc_library(
pw_cc_test(
name = "googletest_test_matchers_test",
srcs = ["googletest_test_matchers_test.cc"],
target_compatible_with = select({
":gtest_setting": [],
"//conditions:default": ["@platforms//:incompatible"],
}),
deps = [
":googletest_test_matchers",
],
Expand Down Expand Up @@ -221,8 +212,15 @@ cc_library(
],
)

cc_library(
# TODO: b/324116813 - Remove this alias once no downstream project depends on
# it.
alias(
name = "logging_event_handler",
actual = "logging",
)

cc_library(
name = "logging",
testonly = True,
srcs = [
"logging_event_handler.cc",
Expand All @@ -239,16 +237,6 @@ cc_library(
],
)

# Provides logging to either the light framework or an external GoogleTest.
cc_library(
name = "logging",
testonly = True,
deps = [":logging_event_handler"] + select({
":gtest_setting": [":googletest_handler_adapter"],
"//conditions:default": [],
}),
)

pw_cc_binary(
name = "logging_main",
testonly = True,
Expand Down Expand Up @@ -353,7 +341,12 @@ cc_library(
srcs = ["static_library_support.cc"],
hdrs = ["public/pw_unit_test/static_library_support.h"],
includes = ["public"],
deps = [":light"], # This library only works with the light backend
# This library only works with the light backend
target_compatible_with = select({
":light_setting": [],
"//conditions:default": ["@platforms//:incompatible"],
}),
deps = [":pw_unit_test"],
)

cc_library(
Expand All @@ -371,11 +364,8 @@ cc_library(
pw_cc_test(
name = "static_library_support_test",
srcs = ["static_library_support_test.cc"],
target_compatible_with = select({
"//pw_unit_test:light_setting": [],
"//conditions:default": ["@platforms//:incompatible"],
}),
deps = [
":pw_unit_test",
":static_library_support",
":tests_in_archive",
"//pw_assert",
Expand All @@ -385,10 +375,6 @@ pw_cc_test(
pw_cc_test(
name = "framework_test",
srcs = ["framework_test.cc"],
target_compatible_with = select({
"//pw_unit_test:light_setting": [],
"//conditions:default": ["@platforms//:incompatible"],
}),
deps = [
":pw_unit_test",
"//pw_assert",
Expand All @@ -400,7 +386,7 @@ pw_cc_test(
name = "framework_light_test",
srcs = ["framework_light_test.cc"],
target_compatible_with = select({
"//pw_unit_test:light_setting": [],
":light_setting": [],
"//conditions:default": ["@platforms//:incompatible"],
}),
deps = [
Expand Down

0 comments on commit ff074e7

Please sign in to comment.