From d1cd3518a85107d56b2e5445964e30a33d79f41b Mon Sep 17 00:00:00 2001 From: Spandan Das Date: Sat, 22 Jul 2023 02:19:42 +0000 Subject: [PATCH] Support cc_test.isolated in bp2build This property implicitly adds `libgtest_isolated_main` to the static libs of the test. bp2build will make this and `liblog` explicit in BUILD files Since the deps are made explicit in BUILD files, `isolated` becomes a no-op for cc_test. Remove this property from cc_test. Test: unit tests Bug: 244432609 Change-Id: I189a7b6b62d9064f4b2abad49ac4975468046498 --- bp2build/cc_test_conversion_test.go | 43 ++++++++++++++++++++--------- cc/test.go | 33 ++++++++++++---------- 2 files changed, 49 insertions(+), 27 deletions(-) diff --git a/bp2build/cc_test_conversion_test.go b/bp2build/cc_test_conversion_test.go index ec143ff6a..20eb092d1 100644 --- a/bp2build/cc_test_conversion_test.go +++ b/bp2build/cc_test_conversion_test.go @@ -121,7 +121,6 @@ cc_test_library { "//conditions:default": [], })`, "gtest": "True", - "isolated": "False", "local_includes": `["."]`, "dynamic_deps": `[":cc_test_lib2"] + select({ "//build/bazel/platforms/os:android": [":foolib"], @@ -158,7 +157,6 @@ cc_test { targets: []testBazelTarget{ {"cc_test", "mytest", AttrNameToString{ "gtest": "False", - "isolated": "False", "local_includes": `["."]`, "srcs": `["test.cpp"]`, }, @@ -185,7 +183,6 @@ cc_test { "local_includes": `["."]`, "srcs": `["test.cpp"]`, "gtest": "True", - "isolated": "False", "deps": `[ ":libgtest_main", ":libgtest", @@ -213,7 +210,6 @@ cc_test { targets: []testBazelTarget{ {"cc_test", "mytest", AttrNameToString{ "gtest": "True", - "isolated": "False", "local_includes": `["."]`, "srcs": `["test.cpp"]`, "target_compatible_with": `["//build/bazel/platforms/os:android"]`, @@ -244,7 +240,6 @@ cc_test { targets: []testBazelTarget{ {"cc_test", "mytest", AttrNameToString{ "gtest": "True", - "isolated": "False", "local_includes": `["."]`, "srcs": `["test.cpp"]`, "target_compatible_with": `["//build/bazel/platforms/os:android"]`, @@ -273,13 +268,12 @@ cc_test { auto_gen_config: true, isolated: true, } -` + simpleModuleDoNotConvertBp2build("cc_library_static", "libgtest_main") + - simpleModuleDoNotConvertBp2build("cc_library_static", "libgtest"), +` + simpleModuleDoNotConvertBp2build("cc_library_static", "libgtest_isolated_main") + + simpleModuleDoNotConvertBp2build("cc_library", "liblog"), targets: []testBazelTarget{ {"cc_test", "mytest", AttrNameToString{ "auto_generate_test_config": "True", "gtest": "True", - "isolated": "True", "local_includes": `["."]`, "srcs": `["test.cpp"]`, "target_compatible_with": `["//build/bazel/platforms/os:android"]`, @@ -289,10 +283,8 @@ cc_test { ]`, "template_install_base": `"/data/local/tmp"`, "template_test_config": `"test_config_template.xml"`, - "deps": `[ - ":libgtest_main", - ":libgtest", - ]`, + "deps": `[":libgtest_isolated_main"]`, + "dynamic_deps": `[":liblog"]`, }, }, }, @@ -313,7 +305,6 @@ cc_test { targets: []testBazelTarget{ {"cc_test", "mytest", AttrNameToString{ "gtest": "True", - "isolated": "False", "local_includes": `["."]`, "srcs": `["test.cpp"]`, "target_compatible_with": `["//build/bazel/platforms/os:android"]`, @@ -327,3 +318,29 @@ cc_test { }) } + +func TestCcTest_WithIsolatedTurnedOn(t *testing.T) { + runCcTestTestCase(t, ccTestBp2buildTestCase{ + description: "cc test that sets `isolated: true` should run with ligtest_isolated_main instead of libgtest_main", + blueprint: ` +cc_test { + name: "mytest", + srcs: ["test.cpp"], + isolated: true, +} +` + simpleModuleDoNotConvertBp2build("cc_library_static", "libgtest_isolated_main") + + simpleModuleDoNotConvertBp2build("cc_library", "liblog"), + targets: []testBazelTarget{ + {"cc_test", "mytest", AttrNameToString{ + "gtest": "True", + "local_includes": `["."]`, + "srcs": `["test.cpp"]`, + "target_compatible_with": `["//build/bazel/platforms/os:android"]`, + "deps": `[":libgtest_isolated_main"]`, + "dynamic_deps": `[":liblog"]`, + }, + }, + }, + }) + +} diff --git a/cc/test.go b/cc/test.go index bd8fba1f6..53a097aba 100644 --- a/cc/test.go +++ b/cc/test.go @@ -682,8 +682,7 @@ func (handler *ccTestBazelHandler) ProcessBazelQueryResponse(ctx android.ModuleC type testBinaryAttributes struct { binaryAttributes - Gtest bool - Isolated bool + Gtest bool tidyAttributes tradefed.TestConfigAttributes @@ -725,12 +724,11 @@ func testBinaryBp2build(ctx android.TopDownMutatorContext, m *Module) { for _, propIntf := range m.GetProperties() { if testLinkerProps, ok := propIntf.(*TestLinkerProperties); ok { testBinaryAttrs.Gtest = proptools.BoolDefault(testLinkerProps.Gtest, true) - testBinaryAttrs.Isolated = gtestIsolated break } } - addImplicitGtestDeps(ctx, &testBinaryAttrs) + addImplicitGtestDeps(ctx, &testBinaryAttrs, gtestIsolated) for _, testProps := range m.GetProperties() { if p, ok := testProps.(*TestBinaryProperties); ok { @@ -766,18 +764,25 @@ func testBinaryBp2build(ctx android.TopDownMutatorContext, m *Module) { // cc_test that builds using gtest needs some additional deps // addImplicitGtestDeps makes these deps explicit in the generated BUILD files -func addImplicitGtestDeps(ctx android.BazelConversionPathContext, attrs *testBinaryAttributes) { +func addImplicitGtestDeps(ctx android.BazelConversionPathContext, attrs *testBinaryAttributes, gtestIsolated bool) { + addDepsAndDedupe := func(lla *bazel.LabelListAttribute, modules []string) { + moduleLabels := android.BazelLabelForModuleDeps(ctx, modules) + lla.Value.Append(moduleLabels) + // Dedupe + lla.Value = bazel.FirstUniqueBazelLabelList(lla.Value) + } + // this must be kept in sync with Soong's implementation in: + // https://cs.android.com/android/_/android/platform/build/soong/+/460fb2d6d546b5ab493a7e5479998c4933a80f73:cc/test.go;l=300-313;drc=ec7314336a2b35ea30ce5438b83949c28e3ac429;bpv=1;bpt=0 if attrs.Gtest { - gtestDeps := android.BazelLabelForModuleDeps( - ctx, - []string{ + // TODO - b/244433197: Handle canUseSdk + if gtestIsolated { + addDepsAndDedupe(&attrs.Deps, []string{"libgtest_isolated_main"}) + addDepsAndDedupe(&attrs.Dynamic_deps, []string{"liblog"}) + } else { + addDepsAndDedupe(&attrs.Deps, []string{ "libgtest_main", "libgtest", - }, - ) - attrs.Deps.Value.Append(gtestDeps) - // Dedupe - attrs.Deps.Value = bazel.FirstUniqueBazelLabelList(attrs.Deps.Value) + }) + } } - // TODO(b/244432609): handle `isolated` property. }