From e50fd111f38a9640921b9f71c397f7cd77e6f939 Mon Sep 17 00:00:00 2001 From: Spandan Das Date: Wed, 26 Jul 2023 17:35:20 +0000 Subject: [PATCH] Emit gtest in BUILD files only if Android.bp sets it cc_test bazel macro will default `gtest` to True (similar to Soong). So we can skip explicitly setting this in the generated BUILD files. The benefit will be that it will make the generated files less verbose, without loss of information. This will be implemented by changing its datatype to *bool from bool. Test: go test ./bp2build Test: TH Change-Id: I284e10f1d58c8e7893b170209827f7d5084ca95e --- bp2build/cc_test_conversion_test.go | 42 ++++++++++++++++++++++++----- cc/test.go | 22 +++++++-------- 2 files changed, 46 insertions(+), 18 deletions(-) diff --git a/bp2build/cc_test_conversion_test.go b/bp2build/cc_test_conversion_test.go index 20eb092d1..3c037b494 100644 --- a/bp2build/cc_test_conversion_test.go +++ b/bp2build/cc_test_conversion_test.go @@ -120,7 +120,6 @@ cc_test_library { "//build/bazel/platforms/os:windows": [":hostlib"], "//conditions:default": [], })`, - "gtest": "True", "local_includes": `["."]`, "dynamic_deps": `[":cc_test_lib2"] + select({ "//build/bazel/platforms/os:android": [":foolib"], @@ -182,7 +181,6 @@ cc_test { "tags": `["no-remote"]`, "local_includes": `["."]`, "srcs": `["test.cpp"]`, - "gtest": "True", "deps": `[ ":libgtest_main", ":libgtest", @@ -209,7 +207,6 @@ cc_test { simpleModuleDoNotConvertBp2build("cc_library_static", "libgtest"), targets: []testBazelTarget{ {"cc_test", "mytest", AttrNameToString{ - "gtest": "True", "local_includes": `["."]`, "srcs": `["test.cpp"]`, "target_compatible_with": `["//build/bazel/platforms/os:android"]`, @@ -239,7 +236,6 @@ cc_test { simpleModuleDoNotConvertBp2build("cc_library_static", "libgtest"), targets: []testBazelTarget{ {"cc_test", "mytest", AttrNameToString{ - "gtest": "True", "local_includes": `["."]`, "srcs": `["test.cpp"]`, "target_compatible_with": `["//build/bazel/platforms/os:android"]`, @@ -273,7 +269,6 @@ cc_test { targets: []testBazelTarget{ {"cc_test", "mytest", AttrNameToString{ "auto_generate_test_config": "True", - "gtest": "True", "local_includes": `["."]`, "srcs": `["test.cpp"]`, "target_compatible_with": `["//build/bazel/platforms/os:android"]`, @@ -304,7 +299,6 @@ cc_test { simpleModuleDoNotConvertBp2build("cc_library_static", "libgtest"), targets: []testBazelTarget{ {"cc_test", "mytest", AttrNameToString{ - "gtest": "True", "local_includes": `["."]`, "srcs": `["test.cpp"]`, "target_compatible_with": `["//build/bazel/platforms/os:android"]`, @@ -332,7 +326,6 @@ cc_test { 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"]`, @@ -344,3 +337,38 @@ cc_test { }) } + +func TestCcTest_GtestExplicitlySpecifiedInAndroidBp(t *testing.T) { + runCcTestTestCase(t, ccTestBp2buildTestCase{ + description: "If `gtest` is explicit in Android.bp, it should be explicit in BUILD files as well", + blueprint: ` +cc_test { + name: "mytest_with_gtest", + gtest: true, +} +cc_test { + name: "mytest_with_no_gtest", + gtest: false, +} +` + simpleModuleDoNotConvertBp2build("cc_library_static", "libgtest_main") + + simpleModuleDoNotConvertBp2build("cc_library_static", "libgtest"), + targets: []testBazelTarget{ + {"cc_test", "mytest_with_gtest", AttrNameToString{ + "local_includes": `["."]`, + "deps": `[ + ":libgtest_main", + ":libgtest", + ]`, + "gtest": "True", + "target_compatible_with": `["//build/bazel/platforms/os:android"]`, + }, + }, + {"cc_test", "mytest_with_no_gtest", AttrNameToString{ + "local_includes": `["."]`, + "gtest": "False", + "target_compatible_with": `["//build/bazel/platforms/os:android"]`, + }, + }, + }, + }) +} diff --git a/cc/test.go b/cc/test.go index 53a097aba..0be230151 100644 --- a/cc/test.go +++ b/cc/test.go @@ -682,7 +682,7 @@ func (handler *ccTestBazelHandler) ProcessBazelQueryResponse(ctx android.ModuleC type testBinaryAttributes struct { binaryAttributes - Gtest bool + Gtest *bool tidyAttributes tradefed.TestConfigAttributes @@ -720,15 +720,15 @@ func testBinaryBp2build(ctx android.TopDownMutatorContext, m *Module) { m.convertTidyAttributes(ctx, &testBinaryAttrs.tidyAttributes) - gtestIsolated := m.linker.(*testBinary).isolated(ctx) - for _, propIntf := range m.GetProperties() { - if testLinkerProps, ok := propIntf.(*TestLinkerProperties); ok { - testBinaryAttrs.Gtest = proptools.BoolDefault(testLinkerProps.Gtest, true) - break - } - } + testBinary := m.linker.(*testBinary) + gtest := testBinary.gtest() + gtestIsolated := testBinary.isolated(ctx) + // Use the underling bool pointer for Gtest in attrs + // This ensures that if this property is not set in Android.bp file, it will not be set in BUILD file either + // cc_test macro will default gtest to True + testBinaryAttrs.Gtest = testBinary.LinkerProperties.Gtest - addImplicitGtestDeps(ctx, &testBinaryAttrs, gtestIsolated) + addImplicitGtestDeps(ctx, &testBinaryAttrs, gtest, gtestIsolated) for _, testProps := range m.GetProperties() { if p, ok := testProps.(*TestBinaryProperties); ok { @@ -764,7 +764,7 @@ 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, gtestIsolated bool) { +func addImplicitGtestDeps(ctx android.BazelConversionPathContext, attrs *testBinaryAttributes, gtest, gtestIsolated bool) { addDepsAndDedupe := func(lla *bazel.LabelListAttribute, modules []string) { moduleLabels := android.BazelLabelForModuleDeps(ctx, modules) lla.Value.Append(moduleLabels) @@ -773,7 +773,7 @@ func addImplicitGtestDeps(ctx android.BazelConversionPathContext, attrs *testBin } // 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 { + if gtest { // TODO - b/244433197: Handle canUseSdk if gtestIsolated { addDepsAndDedupe(&attrs.Deps, []string{"libgtest_isolated_main"})