chore(internal/librarian): extend configure contract to include version check (#1659)

JoeWang1127 · web-flow · commit 05a817bd2a9b · 2025-08-07T16:46:51.000-04:00
Fixes #1010
diff --git a/internal/librarian/generate.go b/internal/librarian/generate.go
@@ -558,6 +558,11 @@ func (r *generateRunner) runConfigureCommand(ctx context.Context) (string, error
 		return "", err
 	}
 
+	if libraryState.Version == "" {
+		slog.Info("library doesn't receive a version, apply the default version", "id", r.cfg.Library)
+		libraryState.Version = "0.0.0"
+	}
+
 	// Update the library state in the librarian state.
 	for i, library := range r.state.Libraries {
 		if library.ID != libraryState.ID {
diff --git a/internal/librarian/generate_test.go b/internal/librarian/generate_test.go
@@ -17,7 +17,6 @@ package librarian
 import (
 	"context"
 	"errors"
-	"fmt"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -204,6 +203,7 @@ func TestRunConfigureCommand(t *testing.T) {
 		container          *mockContainerClient
 		wantConfigureCalls int
 		wantErr            bool
+		wantErrMsg         string
 	}{
 		{
 			name: "configures library successfully",
@@ -235,6 +235,7 @@ func TestRunConfigureCommand(t *testing.T) {
 			container:          &mockContainerClient{},
 			wantConfigureCalls: 1,
 			wantErr:            true,
+			wantErrMsg:         "failed to read dir",
 		},
 		{
 			name: "configures library with error message in response",
@@ -253,6 +254,7 @@ func TestRunConfigureCommand(t *testing.T) {
 			},
 			wantConfigureCalls: 1,
 			wantErr:            true,
+			wantErrMsg:         "failed with error message",
 		},
 		{
 			name: "configures library with no response",
@@ -271,6 +273,24 @@ func TestRunConfigureCommand(t *testing.T) {
 			},
 			wantConfigureCalls: 1,
 			wantErr:            true,
+			wantErrMsg:         "failed to read response file",
+		},
+		{
+			name: "configures library without initial version",
+			api:  "some/api",
+			repo: newTestGitRepo(t),
+			state: &config.LibrarianState{
+				Libraries: []*config.LibraryState{
+					{
+						ID:   "some-library",
+						APIs: []*config.API{{Path: "some/api"}},
+					},
+				},
+			},
+			container: &mockContainerClient{
+				noInitVersion: true,
+			},
+			wantConfigureCalls: 1,
 		},
 		{
 			name: "configure command failed",
@@ -285,10 +305,12 @@ func TestRunConfigureCommand(t *testing.T) {
 				},
 			},
 			container: &mockContainerClient{
-				configureErr: errors.New("simulated configure command error"),
+				configureErr:        errors.New("simulated configure command error"),
+				noConfigureResponse: true,
 			},
 			wantConfigureCalls: 1,
 			wantErr:            true,
+			wantErrMsg:         "simulated configure command error",
 		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
@@ -305,30 +327,22 @@ func TestRunConfigureCommand(t *testing.T) {
 				containerClient: test.container,
 			}
 
-			if test.name == "configures library successfully" ||
-				test.name == "configures library with error message in response" {
-				if err := os.MkdirAll(filepath.Join(cfg.APISource, test.api), 0755); err != nil {
-					t.Fatal(err)
-				}
-
-				data := []byte("type: google.api.Service")
-				if err := os.WriteFile(filepath.Join(cfg.APISource, test.api, "example_service_v2.yaml"), data, 0755); err != nil {
-					t.Fatal(err)
-				}
+			// Create a service config
+			if err := os.MkdirAll(filepath.Join(cfg.APISource, test.api), 0755); err != nil {
+				t.Fatal(err)
 			}
 
-			if test.name == "configures library with no response" ||
-				test.name == "configure command failed" {
-				if err := os.MkdirAll(filepath.Join(cfg.APISource, test.api), 0755); err != nil {
-					t.Fatal(err)
-				}
+			data := []byte("type: google.api.Service")
+			if err := os.WriteFile(filepath.Join(cfg.APISource, test.api, "example_service_v2.yaml"), data, 0755); err != nil {
+				t.Fatal(err)
+			}
 
-				data := []byte("type: google.api.Service")
-				if err := os.WriteFile(filepath.Join(cfg.APISource, test.api, "example_service_v2.yaml"), data, 0755); err != nil {
+			if test.name == "configures library with non-existent api source" {
+				// This test verifies the scenario of no service config is found
+				// in api path.
+				if err := os.RemoveAll(filepath.Join(cfg.APISource)); err != nil {
 					t.Fatal(err)
 				}
-
-				// Do not create response file
 			}
 
 			_, err := r.runConfigureCommand(context.Background())
@@ -338,6 +352,10 @@ func TestRunConfigureCommand(t *testing.T) {
 					t.Errorf("runConfigureCommand() should return error")
 				}
 
+				if !strings.Contains(err.Error(), test.wantErrMsg) {
+					t.Errorf("runConfigureCommand() err = %v, want error containing %q", err, test.wantErrMsg)
+				}
+
 				return
 			}
 
@@ -501,52 +519,6 @@ func TestNewGenerateRunner(t *testing.T) {
 	}
 }
 
-// newTestGitRepo creates a new git repository in a temporary directory.
-func newTestGitRepo(t *testing.T) gitrepo.Repository {
-	return newTestGitRepoWithState(t, true)
-}
-
-// newTestGitRepo creates a new git repository in a temporary directory.
-func newTestGitRepoWithState(t *testing.T, writeState bool) gitrepo.Repository {
-	t.Helper()
-	dir := t.TempDir()
-	remoteURL := "https://github.com/googleapis/librarian.git"
-	runGit(t, dir, "init")
-	runGit(t, dir, "config", "user.email", "test@example.com")
-	runGit(t, dir, "config", "user.name", "Test User")
-	if err := os.WriteFile(filepath.Join(dir, "README.md"), []byte("test"), 0644); err != nil {
-		t.Fatalf("os.WriteFile: %v", err)
-	}
-	if writeState {
-		// Create an empty state.yaml file
-		stateDir := filepath.Join(dir, config.LibrarianDir)
-		if err := os.MkdirAll(stateDir, 0755); err != nil {
-			t.Fatalf("os.MkdirAll: %v", err)
-		}
-		stateFile := filepath.Join(stateDir, "state.yaml")
-		if err := os.WriteFile(stateFile, []byte(""), 0644); err != nil {
-			t.Fatalf("os.WriteFile: %v", err)
-		}
-	}
-	runGit(t, dir, "add", ".")
-	runGit(t, dir, "commit", "-m", "initial commit")
-	runGit(t, dir, "remote", "add", "origin", remoteURL)
-	repo, err := gitrepo.NewRepository(&gitrepo.RepositoryOptions{Dir: dir})
-	if err != nil {
-		t.Fatalf("gitrepo.Open(%q) = %v", dir, err)
-	}
-	return repo
-}
-
-func runGit(t *testing.T, dir string, args ...string) {
-	t.Helper()
-	cmd := exec.Command("git", args...)
-	cmd.Dir = dir
-	if err := cmd.Run(); err != nil {
-		t.Fatalf("git %v: %v", args, err)
-	}
-}
-
 func TestGenerateRun(t *testing.T) {
 	t.Parallel()
 	for _, test := range []struct {
@@ -967,28 +939,15 @@ func TestGenerateScenarios(t *testing.T) {
 				workRoot:        t.TempDir(),
 			}
 
+			// Create a service config in api path.
 			if err := os.MkdirAll(filepath.Join(cfg.APISource, test.api), 0755); err != nil {
 				t.Fatal(err)
 			}
-
 			data := []byte("type: google.api.Service")
 			if err := os.WriteFile(filepath.Join(cfg.APISource, test.api, "example_service_v2.yaml"), data, 0755); err != nil {
 				t.Fatal(err)
 			}
 
-			// Write a configure-response.json because it is required by configure
-			// command.
-			if err := os.MkdirAll(filepath.Join(r.repo.GetDir(), config.LibrarianDir), 0755); err != nil {
-				t.Fatal(err)
-			}
-
-			libraryStr := fmt.Sprintf(`{
-	"ID": "%s"
-}`, test.library)
-			if err := os.WriteFile(filepath.Join(r.repo.GetDir(), config.LibrarianDir, config.ConfigureResponse), []byte(libraryStr), 0755); err != nil {
-				t.Fatal(err)
-			}
-
 			err := r.run(context.Background())
 			if test.wantErr {
 				if err == nil {
@@ -1662,3 +1621,50 @@ func TestCleanAndCopyLibrary(t *testing.T) {
 		})
 	}
 }
+
+// newTestGitRepo creates a new git repository in a temporary directory.
+func newTestGitRepo(t *testing.T) gitrepo.Repository {
+	t.Helper()
+	return newTestGitRepoWithState(t, true)
+}
+
+// newTestGitRepo creates a new git repository in a temporary directory.
+func newTestGitRepoWithState(t *testing.T, writeState bool) gitrepo.Repository {
+	t.Helper()
+	dir := t.TempDir()
+	remoteURL := "https://github.com/googleapis/librarian.git"
+	runGit(t, dir, "init")
+	runGit(t, dir, "config", "user.email", "test@example.com")
+	runGit(t, dir, "config", "user.name", "Test User")
+	if err := os.WriteFile(filepath.Join(dir, "README.md"), []byte("test"), 0644); err != nil {
+		t.Fatalf("os.WriteFile: %v", err)
+	}
+	if writeState {
+		// Create an empty state.yaml file
+		stateDir := filepath.Join(dir, config.LibrarianDir)
+		if err := os.MkdirAll(stateDir, 0755); err != nil {
+			t.Fatalf("os.MkdirAll: %v", err)
+		}
+		stateFile := filepath.Join(stateDir, "state.yaml")
+		if err := os.WriteFile(stateFile, []byte(""), 0644); err != nil {
+			t.Fatalf("os.WriteFile: %v", err)
+		}
+	}
+	runGit(t, dir, "add", ".")
+	runGit(t, dir, "commit", "-m", "initial commit")
+	runGit(t, dir, "remote", "add", "origin", remoteURL)
+	repo, err := gitrepo.NewRepository(&gitrepo.RepositoryOptions{Dir: dir})
+	if err != nil {
+		t.Fatalf("gitrepo.Open(%q) = %v", dir, err)
+	}
+	return repo
+}
+
+func runGit(t *testing.T, dir string, args ...string) {
+	t.Helper()
+	cmd := exec.Command("git", args...)
+	cmd.Dir = dir
+	if err := cmd.Run(); err != nil {
+		t.Fatalf("git %v: %v", args, err)
+	}
+}
diff --git a/internal/librarian/mocks_test.go b/internal/librarian/mocks_test.go
@@ -19,6 +19,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/go-git/go-git/v5"
 	"github.com/googleapis/librarian/internal/config"
@@ -69,6 +70,7 @@ type mockContainerClient struct {
 	requestLibraryID    string
 	noBuildResponse     bool
 	noConfigureResponse bool
+	noInitVersion       bool
 	wantErrorMsg        bool
 }
 
@@ -132,19 +134,16 @@ func (m *mockContainerClient) Configure(ctx context.Context, request *docker.Con
 		return "", err
 	}
 
-	libraryStr := ""
+	var libraryBuilder strings.Builder
+	libraryBuilder.WriteString(fmt.Sprintf("{\"id\":\"%s\"", request.State.Libraries[0].ID))
+	if !m.noInitVersion {
+		libraryBuilder.WriteString(",\"version\": \"0.1.0\"")
+	}
 	if m.wantErrorMsg {
-		libraryStr = fmt.Sprintf(`{
-	"ID": "%s",
-  "error": "simulated error message"
-}`, request.State.Libraries[0].ID)
-	} else {
-		libraryStr = fmt.Sprintf(`{
-	"ID": "%s"
-}`, request.State.Libraries[0].ID)
+		libraryBuilder.WriteString(",\"error\": \"simulated error message\"")
 	}
-
-	if err := os.WriteFile(filepath.Join(request.RepoDir, config.LibrarianDir, config.ConfigureResponse), []byte(libraryStr), 0755); err != nil {
+	libraryBuilder.WriteString("}")
+	if err := os.WriteFile(filepath.Join(request.RepoDir, config.LibrarianDir, config.ConfigureResponse), []byte(libraryBuilder.String()), 0755); err != nil {
 		return "", err
 	}
 	return "", m.configureErr
diff --git a/internal/librarian/state.go b/internal/librarian/state.go
@@ -103,7 +103,7 @@ func populateServiceConfigIfEmpty(state *config.LibrarianState, contentLoader fu
 func findServiceConfigIn(contentLoader func(file string) ([]byte, error), path string) (string, error) {
 	entries, err := os.ReadDir(path)
 	if err != nil {
-		return "", err
+		return "", fmt.Errorf("failed to read dir %q: %w", path, err)
 	}
 
 	for _, entry := range entries {
@@ -135,7 +135,7 @@ func saveLibrarianState(repoDir string, state *config.LibrarianState) error {
 	return os.WriteFile(path, bytes, 0644)
 }
 
-// readLibraryState reads the library state from configure-response.json.
+// readLibraryState reads the library state from a container response.
 //
 // The response file is removed afterwards.
 func readLibraryState(contentLoader func(data []byte, state *config.LibraryState) error, jsonFilePath string) (*config.LibraryState, error) {
