Skip to content

Commit f1215bd

Browse files
authored
fix(tool/cmd/migrate): change default samples to false add hardcoded cases in java migrate (#5351)
Changes default samples config to false when not found from BUILD.bazel and added a few hardcoded cases. `include_samples` defaults to false when not specified in java_gapic_assembly_gradle_pkg. Hardcoded a few cases to have config `samples: false`, to reflect reality in google-cloud-java. These are edge-cases (mostly hybrid libraries) that are effectively not generating samples: BUILD.bazel has `include_samples` as true, but after generate, it is not configured to move to final location via .OwlBot-hermetic.yaml. e.g. [datastore](https://github.com/googleapis/google-cloud-java/blob/main/java-datastore/.OwlBot-hermetic.yaml). For #5307
1 parent 38e370b commit f1215bd

3 files changed

Lines changed: 50 additions & 4 deletions

File tree

tool/cmd/migrate/java.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ func parseJavaBazel(googleapisDir, dir string) (*javaGAPICInfo, error) {
6060
if file == nil {
6161
return nil, nil
6262
}
63-
info := &javaGAPICInfo{Samples: true}
63+
info := &javaGAPICInfo{Samples: false}
6464
// 1. From java_gapic_library
6565
if rules := file.Rules("java_gapic_library"); len(rules) > 0 {
6666
if len(rules) > 1 {
@@ -73,7 +73,7 @@ func parseJavaBazel(googleapisDir, dir string) (*javaGAPICInfo, error) {
7373
log.Printf("Warning: multiple java_gapic_assembly_gradle_pkg in %s/BUILD.bazel, using first", dir)
7474
}
7575
rule := rules[0]
76-
info.Samples = rule.AttrLiteral("include_samples") != "False"
76+
info.Samples = rule.AttrLiteral("include_samples") == "True"
7777
}
7878
// 3. From proto_library_with_info
7979
if rules := file.Rules("proto_library_with_info"); len(rules) > 0 {
@@ -240,7 +240,7 @@ func buildConfig(gen *GenerationConfig, repoPath string, src *config.Source, ver
240240
Path: g.ProtoPath,
241241
AdditionalProtos: info.AdditionalProtos,
242242
}
243-
if !info.Samples {
243+
if shouldExcludeSamples(name, info) {
244244
javaAPI.Samples = new(false)
245245
}
246246
applyJavaArtifactOverrides(name, javaAPI)
@@ -366,6 +366,10 @@ func applyJavaArtifactOverrides(name string, api *config.JavaAPI) {
366366
}
367367
}
368368

369+
func shouldExcludeSamples(name string, info *javaGAPICInfo) bool {
370+
return !info.Samples || excludedSamplesLibraries[name]
371+
}
372+
369373
func invertBoolPtr(p *bool) bool {
370374
return p != nil && !*p
371375
}

tool/cmd/migrate/java_module.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,11 @@ var (
2525
"distributedcloudedge": "distributedcloudedge",
2626
"gke-backup": "gke-backup",
2727
}
28+
29+
excludedSamplesLibraries = map[string]bool{
30+
"bigquerystorage": true,
31+
"datastore": true,
32+
"storage": true,
33+
"spanner": true,
34+
}
2835
)

tool/cmd/migrate/java_test.go

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,40 @@ func TestBuildConfig(t *testing.T) {
362362
}
363363
}
364364

365+
func TestShouldExcludeSamples(t *testing.T) {
366+
for _, test := range []struct {
367+
name string
368+
lib string
369+
info *javaGAPICInfo
370+
want bool
371+
}{
372+
{
373+
name: "exclude if info.Samples is false",
374+
lib: "any-lib",
375+
info: &javaGAPICInfo{Samples: false},
376+
want: true,
377+
},
378+
{
379+
name: "exclude if lib is in excludedSamplesLibraries",
380+
lib: "datastore",
381+
info: &javaGAPICInfo{Samples: true},
382+
want: true,
383+
},
384+
{
385+
name: "include if info.Samples is true and lib not in map",
386+
lib: "any-lib",
387+
info: &javaGAPICInfo{Samples: true},
388+
want: false,
389+
},
390+
} {
391+
t.Run(test.name, func(t *testing.T) {
392+
if got := shouldExcludeSamples(test.lib, test.info); got != test.want {
393+
t.Errorf("shouldExcludeSamples(%q, %+v) = %v, want %v", test.lib, test.info, got, test.want)
394+
}
395+
})
396+
}
397+
}
398+
365399
func TestBuildConfig_ArtifactIDOverrides(t *testing.T) {
366400
gen := &GenerationConfig{
367401
Libraries: []LibraryConfig{
@@ -401,6 +435,7 @@ func TestBuildConfig_ArtifactIDOverrides(t *testing.T) {
401435
JavaAPIs: []*config.JavaAPI{
402436
{
403437
Path: "google/datastore/admin/v1",
438+
Samples: func(b bool) *bool { return &b }(false),
404439
ProtoArtifactIDOverride: "proto-google-cloud-datastore-admin-v1",
405440
GRPCArtifactIDOverride: "grpc-google-cloud-datastore-admin-v1",
406441
},
@@ -527,7 +562,7 @@ func TestParseJavaBazel(t *testing.T) {
527562
name: "no GAPIC rules",
528563
googleapisDir: "testdata/parse-bazel/no-gapic-rule",
529564
want: &javaGAPICInfo{
530-
Samples: true,
565+
Samples: false,
531566
AdditionalProtos: []string{
532567
"google/cloud/common_resources.proto",
533568
},

0 commit comments

Comments
 (0)