From 682e78686b66afb316962c9a662ea44f5c4aac93 Mon Sep 17 00:00:00 2001 From: Spandan Das Date: Thu, 22 Jun 2023 22:22:11 +0000 Subject: [PATCH] Create additional test target for go modules in bp2build For go modules with non-empty testSrcs, we will create an additional go_test target. To build a standalone test executable, we need to include the source files in the compilation unit. This will be done using the `embed` attribute 1. For tests of go_library, this is straightforward. It will embed the go_library and "inherit" its .go files and deps 2. For tetss of go_binary, we need to create an additional go_source target since go_binary cannot be embedded inside a go_test Using `b test` for these tests revealed that certain tests are not hermitic and rely on `testdata` files checked into the tree. This CL introduces an allowlist to skip generating go_test targets for them. Test: bp2build unit test Test: TH Bug: 284483729 Bug: 288491147 Change-Id: Ic736d655babc2f6067e4da75384900b7b8bdc2ed --- bp2build/build_conversion.go | 188 +++++++++++++++++++++++++++++++-- bp2build/go_conversion_test.go | 60 ++++++++++- 2 files changed, 236 insertions(+), 12 deletions(-) diff --git a/bp2build/build_conversion.go b/bp2build/build_conversion.go index 0e6596bf7..0ac01ea30 100644 --- a/bp2build/build_conversion.go +++ b/bp2build/build_conversion.go @@ -353,9 +353,91 @@ type goAttributes struct { Importpath bazel.StringAttribute Srcs bazel.LabelListAttribute Deps bazel.LabelListAttribute + Data bazel.LabelListAttribute Target_compatible_with bazel.LabelListAttribute + + // attributes for the dynamically generated go_test target + Embed bazel.LabelListAttribute } +type goTestProperties struct { + name string + dir string + testSrcs []string + linuxTestSrcs []string + darwinTestSrcs []string + testData []string + // Name of the target that should be compiled together with the test + embedName string +} + +// Creates a go_test target for bootstrap_go_package / blueprint_go_binary +func generateBazelTargetsGoTest(ctx *android.Context, goModulesMap nameToGoLibraryModule, gp goTestProperties) (BazelTarget, error) { + ca := android.CommonAttributes{ + Name: gp.name, + } + ga := goAttributes{ + Srcs: goSrcLabels(ctx.Config(), gp.dir, gp.testSrcs, gp.linuxTestSrcs, gp.darwinTestSrcs), + Data: goSrcLabels(ctx.Config(), gp.dir, gp.testData, []string{}, []string{}), + Embed: bazel.MakeLabelListAttribute( + bazel.MakeLabelList( + []bazel.Label{bazel.Label{Label: ":" + gp.embedName}}, + ), + ), + Target_compatible_with: targetNotCompatibleWithAndroid(), + } + + libTest := goBazelTarget{ + targetName: gp.name, + targetPackage: gp.dir, + bazelRuleClass: "go_test", + bazelRuleLoadLocation: "@io_bazel_rules_go//go:def.bzl", + bazelAttributes: []interface{}{&ca, &ga}, + } + return generateBazelTarget(ctx, libTest) +} + +// TODO - b/288491147: testSrcs of certain bootstrap_go_package/blueprint_go_binary are not hermetic and depend on +// testdata checked into the filesystem. +// Denylist the generation of go_test targets for these Soong modules. +// The go_library/go_binary will still be generated, since those are hermitic. +var ( + goTestsDenylist = []string{ + "android-archive-zip", + "bazel_notice_gen", + "blueprint-bootstrap-bpdoc", + "blueprint-microfactory", + "blueprint-pathtools", + "bssl_ar", + "compliance_checkmetadata", + "compliance_checkshare", + "compliance_dumpgraph", + "compliance_dumpresolutions", + "compliance_listshare", + "compliance-module", + "compliancenotice_bom", + "compliancenotice_shippedlibs", + "compliance_rtrace", + "compliance_sbom", + "golang-protobuf-internal-fuzz-jsonfuzz", + "golang-protobuf-internal-fuzz-textfuzz", + "golang-protobuf-internal-fuzz-wirefuzz", + "htmlnotice", + "protoc-gen-go", + "rbcrun-module", + "spdx-tools-builder", + "spdx-tools-builder2v1", + "spdx-tools-builder2v2", + "spdx-tools-builder2v3", + "spdx-tools-idsearcher", + "spdx-tools-spdx-json", + "spdx-tools-utils", + "soong-ui-build", + "textnotice", + "xmlnotice", + } +) + func generateBazelTargetsGoPackage(ctx *android.Context, g *bootstrap.GoPackage, goModulesMap nameToGoLibraryModule) ([]BazelTarget, []error) { ca := android.CommonAttributes{ Name: g.Name(), @@ -390,12 +472,33 @@ func generateBazelTargetsGoPackage(ctx *android.Context, g *bootstrap.GoPackage, bazelRuleLoadLocation: "@io_bazel_rules_go//go:def.bzl", bazelAttributes: []interface{}{&ca, &ga}, } - // TODO - b/284483729: Create go_test target from testSrcs - libTarget, err := generateBazelTarget(ctx, lib) - if err != nil { - return []BazelTarget{}, []error{err} + retTargets := []BazelTarget{} + var retErrs []error + if libTarget, err := generateBazelTarget(ctx, lib); err == nil { + retTargets = append(retTargets, libTarget) + } else { + retErrs = []error{err} } - return []BazelTarget{libTarget}, nil + + // If the library contains test srcs, create an additional go_test target + if !android.InList(g.Name(), goTestsDenylist) && (len(g.TestSrcs()) > 0 || len(g.LinuxTestSrcs()) > 0 || len(g.DarwinTestSrcs()) > 0) { + gp := goTestProperties{ + name: g.Name() + "-test", + dir: ctx.ModuleDir(g), + testSrcs: g.TestSrcs(), + linuxTestSrcs: g.LinuxTestSrcs(), + darwinTestSrcs: g.DarwinTestSrcs(), + testData: g.TestData(), + embedName: g.Name(), // embed the source go_library in the test so that its .go files are included in the compilation unit + } + if libTestTarget, err := generateBazelTargetsGoTest(ctx, goModulesMap, gp); err == nil { + retTargets = append(retTargets, libTestTarget) + } else { + retErrs = append(retErrs, err) + } + } + + return retTargets, retErrs } type goLibraryModule struct { @@ -440,6 +543,9 @@ func generateBazelTargetsGoBinary(ctx *android.Context, g *bootstrap.GoBinary, g Name: g.Name(), } + retTargets := []BazelTarget{} + var retErrs []error + // For this bootstrap_go_package dep chain, // A --> B --> C ( ---> depends on) // Soong provides the convenience of only listing B as deps of A even if a src file of A imports C @@ -450,12 +556,70 @@ func generateBazelTargetsGoBinary(ctx *android.Context, g *bootstrap.GoBinary, g // bp2build does not have sufficient info on whether C is a direct dep of A or not, so for now collect all transitive deps and add them to deps transitiveDeps := transitiveGoDeps(g.Deps(), goModulesMap) + goSource := "" + // If the library contains test srcs, create an additional go_test target + // The go_test target will embed a go_source containining the source .go files it tests + if !android.InList(g.Name(), goTestsDenylist) && (len(g.TestSrcs()) > 0 || len(g.LinuxTestSrcs()) > 0 || len(g.DarwinTestSrcs()) > 0) { + // Create a go_source containing the source .go files of go_library + // This target will be an `embed` of the go_binary and go_test + goSource = g.Name() + "-source" + ca := android.CommonAttributes{ + Name: goSource, + } + ga := goAttributes{ + Srcs: goSrcLabels(ctx.Config(), ctx.ModuleDir(g), g.Srcs(), g.LinuxSrcs(), g.DarwinSrcs()), + Deps: goDepLabels(transitiveDeps, goModulesMap), + Target_compatible_with: targetNotCompatibleWithAndroid(), + } + libTestSource := goBazelTarget{ + targetName: goSource, + targetPackage: ctx.ModuleDir(g), + bazelRuleClass: "go_source", + bazelRuleLoadLocation: "@io_bazel_rules_go//go:def.bzl", + bazelAttributes: []interface{}{&ca, &ga}, + } + if libSourceTarget, err := generateBazelTarget(ctx, libTestSource); err == nil { + retTargets = append(retTargets, libSourceTarget) + } else { + retErrs = append(retErrs, err) + } + + // Create a go_test target + gp := goTestProperties{ + name: g.Name() + "-test", + dir: ctx.ModuleDir(g), + testSrcs: g.TestSrcs(), + linuxTestSrcs: g.LinuxTestSrcs(), + darwinTestSrcs: g.DarwinTestSrcs(), + testData: g.TestData(), + // embed the go_source in the test + embedName: g.Name() + "-source", + } + if libTestTarget, err := generateBazelTargetsGoTest(ctx, goModulesMap, gp); err == nil { + retTargets = append(retTargets, libTestTarget) + } else { + retErrs = append(retErrs, err) + } + + } + + // Create a go_binary target ga := goAttributes{ - Srcs: goSrcLabels(ctx.Config(), ctx.ModuleDir(g), g.Srcs(), g.LinuxSrcs(), g.DarwinSrcs()), Deps: goDepLabels(transitiveDeps, goModulesMap), Target_compatible_with: targetNotCompatibleWithAndroid(), } + // If the binary has testSrcs, embed the common `go_source` + if goSource != "" { + ga.Embed = bazel.MakeLabelListAttribute( + bazel.MakeLabelList( + []bazel.Label{bazel.Label{Label: ":" + goSource}}, + ), + ) + } else { + ga.Srcs = goSrcLabels(ctx.Config(), ctx.ModuleDir(g), g.Srcs(), g.LinuxSrcs(), g.DarwinSrcs()) + } + bin := goBazelTarget{ targetName: g.Name(), targetPackage: ctx.ModuleDir(g), @@ -463,12 +627,14 @@ func generateBazelTargetsGoBinary(ctx *android.Context, g *bootstrap.GoBinary, g bazelRuleLoadLocation: "@io_bazel_rules_go//go:def.bzl", bazelAttributes: []interface{}{&ca, &ga}, } - // TODO - b/284483729: Create go_test target from testSrcs - binTarget, err := generateBazelTarget(ctx, bin) - if err != nil { - return []BazelTarget{}, []error{err} + + if binTarget, err := generateBazelTarget(ctx, bin); err == nil { + retTargets = append(retTargets, binTarget) + } else { + retErrs = []error{err} } - return []BazelTarget{binTarget}, nil + + return retTargets, retErrs } func GenerateBazelTargets(ctx *CodegenContext, generateFilegroups bool) (conversionResults, []error) { diff --git a/bp2build/go_conversion_test.go b/bp2build/go_conversion_test.go index 507fbf0ae..2387641f2 100644 --- a/bp2build/go_conversion_test.go +++ b/bp2build/go_conversion_test.go @@ -45,11 +45,17 @@ bootstrap_go_package { srcs: [ "foo_linux.go", ], + testSrcs: [ + "foo_linux_test.go", + ], }, darwin: { srcs: [ "foo_darwin.go", ], + testSrcs: [ + "foo_darwin_test.go", + ], }, testSrcs: [ "foo1_test.go", @@ -84,7 +90,21 @@ bootstrap_go_package { })`, }, android.HostSupported, - )}, + ), + makeBazelTargetHostOrDevice("go_test", "foo-test", + AttrNameToString{ + "embed": `[":foo"]`, + "srcs": `[ + "foo1_test.go", + "foo2_test.go", + ] + select({ + "//build/bazel/platforms/os:darwin": ["foo_darwin_test.go"], + "//build/bazel/platforms/os:linux_glibc": ["foo_linux_test.go"], + "//conditions:default": [], + })`, + }, + android.HostSupported, + )}, }) } @@ -125,6 +145,44 @@ bootstrap_go_package { }) } +func TestConvertGoBinaryWithTestSrcs(t *testing.T) { + bp := ` +blueprint_go_binary { + name: "foo", + srcs: ["main.go"], + testSrcs: ["main_test.go"], +} +` + t.Parallel() + runGoTests(t, Bp2buildTestCase{ + Description: "Convert blueprint_go_binary with testSrcs", + Blueprint: bp, + ExpectedBazelTargets: []string{ + makeBazelTargetHostOrDevice("go_binary", "foo", + AttrNameToString{ + "deps": `[]`, + "embed": `[":foo-source"]`, + }, + android.HostSupported, + ), + makeBazelTargetHostOrDevice("go_source", "foo-source", + AttrNameToString{ + "deps": `[]`, + "srcs": `["main.go"]`, + }, + android.HostSupported, + ), + makeBazelTargetHostOrDevice("go_test", "foo-test", + AttrNameToString{ + "embed": `[":foo-source"]`, + "srcs": `["main_test.go"]`, + }, + android.HostSupported, + ), + }, + }) +} + func TestConvertGoBinaryWithSrcInDifferentPackage(t *testing.T) { bp := ` blueprint_go_binary {