From 14d1e15cacd755a42957b3ff563973728cb2fc25 Mon Sep 17 00:00:00 2001 From: Joe Wang <106995533+JoeWang1127@users.noreply.github.com> Date: Fri, 3 Jul 2026 22:37:42 -0400 Subject: [PATCH 1/2] refactor fillDefaults --- internal/librarian/library.go | 48 +++++++++++++++++++----------- internal/librarian/library_test.go | 25 +++++++++++----- 2 files changed, 48 insertions(+), 25 deletions(-) diff --git a/internal/librarian/library.go b/internal/librarian/library.go index 85798e95006..08d5ebdd0cb 100644 --- a/internal/librarian/library.go +++ b/internal/librarian/library.go @@ -33,8 +33,19 @@ var ( errNoExplicitOutput = errors.New("library requires an explicit output path") ) +// languageDefaultFillers maps a language to a function that fills +// language-specific library fields from the provided defaults. +var languageDefaultFillers = map[string]func(*config.Library, *config.Default) *config.Library{ + config.LanguageDart: fillDart, + config.LanguageGo: fillGo, + config.LanguageJava: fillJava, + config.LanguagePython: fillPython, + config.LanguageRust: fillRust, + config.LanguageSwift: fillSwift, +} + // fillDefaults populates empty library fields from the provided defaults. -func fillDefaults(lib *config.Library, d *config.Default) *config.Library { +func fillDefaults(language string, lib *config.Library, d *config.Default) *config.Library { if d == nil { return lib } @@ -44,22 +55,10 @@ func fillDefaults(lib *config.Library, d *config.Default) *config.Library { if lib.Output == "" { lib.Output = d.Output } - switch { - case d.Go != nil: - return fillGo(lib, d) - case d.Java != nil: - return fillJava(lib, d) - case d.Rust != nil: - return fillRust(lib, d) - case d.Dart != nil: - return fillDart(lib, d) - case d.Python != nil: - return fillPython(lib, d) - case d.Swift != nil: - return fillSwift(lib, d) - default: - return lib + if filler, ok := languageDefaultFillers[language]; ok { + return filler(lib, d) } + return lib } // fillGo populates empty Go-specific fields in lib from the provided default. @@ -99,6 +98,9 @@ func union(a, b []string) []string { // fillJava populates empty Java-specific fields in lib from the provided default. func fillJava(lib *config.Library, d *config.Default) *config.Library { + if d.Java == nil { + return lib + } if lib.Java == nil { lib.Java = &config.JavaModule{} } @@ -125,6 +127,9 @@ func fillGroupIDIfEmpty(lib *config.Library, d *config.Default) { // fillRust populates empty Rust-specific fields in lib from the provided default. func fillRust(lib *config.Library, d *config.Default) *config.Library { + if d.Rust == nil { + return lib + } if lib.Rust == nil { lib.Rust = &config.RustCrate{} } @@ -162,6 +167,9 @@ func fillRust(lib *config.Library, d *config.Default) *config.Library { } func fillDart(lib *config.Library, d *config.Default) *config.Library { + if d.Dart == nil { + return lib + } if lib.Version == "" { lib.Version = d.Dart.Version } @@ -184,6 +192,9 @@ func fillDart(lib *config.Library, d *config.Default) *config.Library { // fillPython populates empty Python-specific fields in lib from the provided // default. func fillPython(lib *config.Library, d *config.Default) *config.Library { + if d.Python == nil { + return lib + } if lib.Python == nil { lib.Python = &config.PythonPackage{} } @@ -196,6 +207,9 @@ func fillPython(lib *config.Library, d *config.Default) *config.Library { // fillSwift populates empty Swift-specific fields in lib from the provided default. func fillSwift(lib *config.Library, d *config.Default) *config.Library { + if d.Swift == nil { + return lib + } if lib.Swift == nil { lib.Swift = &config.SwiftPackage{} } @@ -331,7 +345,7 @@ func applyDefaults(language string, lib *config.Library, defaults *config.Defaul } lib.Output = defaultOutput(language, lib.Name, apiPath, defaultOut) } - return fillLibraryDefaults(language, fillDefaults(lib, defaults)) + return fillLibraryDefaults(language, fillDefaults(language, lib, defaults)) } // canDeriveAPIPath reports whether the language's library name contains enough information to diff --git a/internal/librarian/library_test.go b/internal/librarian/library_test.go index 1e1c9cd9430..b20441e2bc2 100644 --- a/internal/librarian/library_test.go +++ b/internal/librarian/library_test.go @@ -29,12 +29,14 @@ func TestFillDefaults(t *testing.T) { } for _, test := range []struct { name string + language string defaults *config.Default lib *config.Library want *config.Library }{ { name: "fills empty fields", + language: config.LanguageDart, defaults: defaults, lib: &config.Library{}, want: &config.Library{ @@ -44,6 +46,7 @@ func TestFillDefaults(t *testing.T) { }, { name: "preserves existing values", + language: config.LanguageDart, defaults: defaults, lib: &config.Library{ Output: "custom/output/", @@ -55,6 +58,7 @@ func TestFillDefaults(t *testing.T) { }, { name: "partial fill", + language: config.LanguageDart, defaults: defaults, lib: &config.Library{Output: "custom/output/"}, want: &config.Library{ @@ -64,12 +68,14 @@ func TestFillDefaults(t *testing.T) { }, { name: "nil defaults", + language: config.LanguageDart, defaults: nil, lib: &config.Library{Output: "foo/"}, want: &config.Library{Output: "foo/"}, }, { - name: "dart defaults", + name: "dart defaults", + language: config.LanguageDart, defaults: &config.Default{ Dart: &config.DartPackage{ APIKeysEnvironmentVariables: "apiKey-1,apiKey-2", @@ -109,7 +115,8 @@ func TestFillDefaults(t *testing.T) { }, }, { - name: "dart defaults do not override library params", + name: "dart defaults do not override library params", + language: config.LanguageDart, defaults: &config.Default{ Dart: &config.DartPackage{ APIKeysEnvironmentVariables: "apiKey-1,apiKey-2", @@ -171,7 +178,8 @@ func TestFillDefaults(t *testing.T) { }, }, { - name: "swift defaults", + name: "swift defaults", + language: config.LanguageSwift, defaults: &config.Default{ Swift: &config.SwiftDefault{ Dependencies: []config.SwiftDependency{ @@ -192,7 +200,8 @@ func TestFillDefaults(t *testing.T) { }, }, { - name: "swift defaults do not override library params", + name: "swift defaults do not override library params", + language: config.LanguageSwift, defaults: &config.Default{ Swift: &config.SwiftDefault{ Dependencies: []config.SwiftDependency{ @@ -225,7 +234,7 @@ func TestFillDefaults(t *testing.T) { }, } { t.Run(test.name, func(t *testing.T) { - got := fillDefaults(test.lib, test.defaults) + got := fillDefaults(test.language, test.lib, test.defaults) if diff := cmp.Diff(test.want, got); diff != "" { t.Errorf("mismatch (-want +got):\n%s", diff) } @@ -600,7 +609,7 @@ func TestFillDefaults_Rust(t *testing.T) { }, } { t.Run(test.name, func(t *testing.T) { - got := fillDefaults(test.lib, defaults) + got := fillDefaults(config.LanguageRust, test.lib, defaults) if diff := cmp.Diff(test.want, got); diff != "" { t.Errorf("mismatch (-want +got):\n%s", diff) } @@ -706,7 +715,7 @@ func TestFillDefaults_Python(t *testing.T) { defaults := &config.Default{ Python: test.defaults, } - got := fillDefaults(test.lib, defaults) + got := fillDefaults(config.LanguagePython, test.lib, defaults) if diff := cmp.Diff(test.want, got); diff != "" { t.Errorf("mismatch (-want +got):\n%s", diff) } @@ -830,7 +839,7 @@ func TestFillDefaults_Go(t *testing.T) { defaults := &config.Default{ Go: test.defaults, } - got := fillDefaults(test.lib, defaults) + got := fillDefaults(config.LanguageGo, test.lib, defaults) if diff := cmp.Diff(test.want, got); diff != "" { t.Errorf("mismatch (-want +got):\n%s", diff) } From 08629853fdf7e714056dc868aa182d3898b10b7e Mon Sep 17 00:00:00 2001 From: Joe Wang <106995533+JoeWang1127@users.noreply.github.com> Date: Fri, 3 Jul 2026 22:41:55 -0400 Subject: [PATCH 2/2] update unit tests --- internal/librarian/library_test.go | 39 ++++++++++++++++++------------ 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/internal/librarian/library_test.go b/internal/librarian/library_test.go index b20441e2bc2..42b8e2ce235 100644 --- a/internal/librarian/library_test.go +++ b/internal/librarian/library_test.go @@ -22,21 +22,19 @@ import ( "github.com/googleapis/librarian/internal/config" ) -func TestFillDefaults(t *testing.T) { +func TestFillDefaults_Dart(t *testing.T) { defaults := &config.Default{ Keep: []string{"CHANGES.md"}, Output: "src/generated/", } for _, test := range []struct { name string - language string defaults *config.Default lib *config.Library want *config.Library }{ { name: "fills empty fields", - language: config.LanguageDart, defaults: defaults, lib: &config.Library{}, want: &config.Library{ @@ -46,7 +44,6 @@ func TestFillDefaults(t *testing.T) { }, { name: "preserves existing values", - language: config.LanguageDart, defaults: defaults, lib: &config.Library{ Output: "custom/output/", @@ -58,7 +55,6 @@ func TestFillDefaults(t *testing.T) { }, { name: "partial fill", - language: config.LanguageDart, defaults: defaults, lib: &config.Library{Output: "custom/output/"}, want: &config.Library{ @@ -68,14 +64,12 @@ func TestFillDefaults(t *testing.T) { }, { name: "nil defaults", - language: config.LanguageDart, defaults: nil, lib: &config.Library{Output: "foo/"}, want: &config.Library{Output: "foo/"}, }, { - name: "dart defaults", - language: config.LanguageDart, + name: "dart defaults", defaults: &config.Default{ Dart: &config.DartPackage{ APIKeysEnvironmentVariables: "apiKey-1,apiKey-2", @@ -115,8 +109,7 @@ func TestFillDefaults(t *testing.T) { }, }, { - name: "dart defaults do not override library params", - language: config.LanguageDart, + name: "dart defaults do not override library params", defaults: &config.Default{ Dart: &config.DartPackage{ APIKeysEnvironmentVariables: "apiKey-1,apiKey-2", @@ -177,10 +170,25 @@ func TestFillDefaults(t *testing.T) { }, }, }, + } { + t.Run(test.name, func(t *testing.T) { + got := fillDefaults(config.LanguageDart, test.lib, test.defaults) + if diff := cmp.Diff(test.want, got); diff != "" { + t.Errorf("mismatch (-want +got):\n%s", diff) + } + }) + } +} + +func TestFillDefaults_Swift(t *testing.T) { + for _, test := range []struct { + name string + defaults *config.Default + lib *config.Library + want *config.Library + }{ { - name: "swift defaults", - language: config.LanguageSwift, - defaults: &config.Default{ + name: "swift defaults", defaults: &config.Default{ Swift: &config.SwiftDefault{ Dependencies: []config.SwiftDependency{ {Name: "wkt", URL: "https://github.com/googleapis/swift-protobuf"}, @@ -200,8 +208,7 @@ func TestFillDefaults(t *testing.T) { }, }, { - name: "swift defaults do not override library params", - language: config.LanguageSwift, + name: "swift defaults do not override library params", defaults: &config.Default{ Swift: &config.SwiftDefault{ Dependencies: []config.SwiftDependency{ @@ -234,7 +241,7 @@ func TestFillDefaults(t *testing.T) { }, } { t.Run(test.name, func(t *testing.T) { - got := fillDefaults(test.language, test.lib, test.defaults) + got := fillDefaults(config.LanguageSwift, test.lib, test.defaults) if diff := cmp.Diff(test.want, got); diff != "" { t.Errorf("mismatch (-want +got):\n%s", diff) }