fix(internal/librarian): update state file for each success (#1017)

Fixes: #938

Author: codyoss

generate_e2e_test.go

@@ -28,6 +28,9 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/googleapis/librarian/internal/config"
+	"gopkg.in/yaml.v3"
 )
 
 func TestRunGenerate(t *testing.T) {
@@ -57,10 +60,10 @@ func TestRunGenerate(t *testing.T) {
 			workRoot := filepath.Join(t.TempDir())
 			repo := filepath.Join(workRoot, repo)
 			APISourceRepo := filepath.Join(workRoot, APISourceRepo)
-			if err := prepareTest(t, repo, workRoot, initialRepoStateDir); err != nil {
+			if err := initRepo(t, repo, initialRepoStateDir); err != nil {
 				t.Fatalf("languageRepo prepare test error = %v", err)
 			}
-			if err := prepareTest(t, APISourceRepo, workRoot, localAPISource); err != nil {
+			if err := initRepo(t, APISourceRepo, localAPISource); err != nil {
 				t.Fatalf("APISouceRepo prepare test error = %v", err)
 			}
 
@@ -145,10 +148,10 @@ func TestRunConfigure(t *testing.T) {
 			workRoot := filepath.Join(os.TempDir(), fmt.Sprintf("rand-%d", rand.Intn(1000)))
 			repo := filepath.Join(workRoot, repo)
 			APISourceRepo := filepath.Join(workRoot, APISourceRepo)
-			if err := prepareTest(t, repo, workRoot, initialRepoStateDir); err != nil {
+			if err := initRepo(t, repo, initialRepoStateDir); err != nil {
 				t.Fatalf("prepare test error = %v", err)
 			}
-			if err := prepareTest(t, APISourceRepo, workRoot, test.apiSource); err != nil {
+			if err := initRepo(t, APISourceRepo, test.apiSource); err != nil {
 				t.Fatalf("APISouceRepo prepare test error = %v", err)
 			}
 
@@ -187,36 +190,35 @@ func TestRunConfigure(t *testing.T) {
 			if err != nil {
 				t.Fatalf("Failed to read configure response file: %v", err)
 			}
-
 			wantBytes, readErr := os.ReadFile(test.updatedState)
 			if readErr != nil {
 				t.Fatalf("Failed to read expected state for comparison: %v", readErr)
 			}
+			var gotState *config.LibrarianState
+			if err := yaml.Unmarshal(gotBytes, &gotState); err != nil {
+				t.Fatalf("Failed to unmarshal configure response file: %v", err)
+			}
+			var wantState *config.LibrarianState
+			if err := yaml.Unmarshal(wantBytes, &wantState); err != nil {
+				t.Fatalf("Failed to unmarshal expected state: %v", err)
+			}
 
-			if diff := cmp.Diff(string(wantBytes), string(gotBytes)); diff != "" {
-				t.Errorf("Generated yaml mismatch (-want +got):\n%s", diff)
+			if diff := cmp.Diff(wantState, gotState, cmpopts.IgnoreFields(config.LibraryState{}, "LastGeneratedCommit")); diff != "" {
+				t.Fatalf("Generated yaml mismatch (-want +got):\n%s", diff)
+			}
+			for _, lib := range gotState.Libraries {
+				if lib.ID == test.library && lib.LastGeneratedCommit == "" {
+					t.Fatal("LastGeneratedCommit should not be empty")
+				}
 			}
-		})
-	}
-}
 
-func prepareTest(t *testing.T, destRepoDir, workRoot, sourceRepoDir string) error {
-	if err := initTestRepo(t, destRepoDir, sourceRepoDir); err != nil {
-		return err
-	}
-	if err := os.MkdirAll(workRoot, 0755); err != nil {
-		return err
+		})
 	}
-
-	return nil
 }
 
-// initTestRepo initiates an empty git repo in the given directory, copy
+// initRepo initiates a git repo in the given directory, copy
 // files from source directory and create a commit.
-func initTestRepo(t *testing.T, dir, source string) error {
-	if err := os.MkdirAll(dir, 0755); err != nil {
-		return err
-	}
+func initRepo(t *testing.T, dir, source string) error {
 	if err := os.CopyFS(dir, os.DirFS(source)); err != nil {
 		return err
 	}

internal/docker/docker_test.go

@@ -24,9 +24,8 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/googleapis/librarian/internal/config"
-
 	"github.com/google/go-cmp/cmp"
+	"github.com/googleapis/librarian/internal/config"
 )
 
 func TestNew(t *testing.T) {
@@ -529,3 +528,33 @@ func TestToGenerateRequestJSON(t *testing.T) {
 		})
 	}
 }
+
+func TestDocker_runCommand(t *testing.T) {
+	tests := []struct {
+		name    string
+		cmdName string
+		args    []string
+		wantErr bool
+	}{
+		{
+			name:    "success",
+			cmdName: "echo",
+			args:    []string{"hello"},
+			wantErr: false,
+		},
+		{
+			name:    "failure",
+			cmdName: "some-non-existent-command",
+			args:    []string{},
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			c := &Docker{}
+			if err := c.runCommand(tt.cmdName, tt.args...); (err != nil) != tt.wantErr {
+				t.Errorf("Docker.runCommand() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}

internal/gitrepo/gitrepo.go

@@ -33,6 +33,7 @@ type Repository interface {
 	IsClean() (bool, error)
 	Remotes() ([]*git.Remote, error)
 	GetDir() string
+	HeadHash() (string, error)
 }
 
 // LocalRepository represents a git repository.
@@ -187,6 +188,15 @@ func (r *LocalRepository) Remotes() ([]*git.Remote, error) {
 	return r.repo.Remotes()
 }
 
+// HeadHash returns hash of the commit for the repository's HEAD.
+func (r *LocalRepository) HeadHash() (string, error) {
+	ref, err := r.repo.Head()
+	if err != nil {
+		return "", err
+	}
+	return ref.Hash().String(), nil
+}
+
 // GetDir returns the directory of the repository.
 func (r *LocalRepository) GetDir() string {
 	return r.Dir

internal/gitrepo/gitrepo_test.go

@@ -597,6 +597,62 @@ func TestRemotes(t *testing.T) {
 	}
 }
 
+func TestHeadHash(t *testing.T) {
+	t.Parallel()
+	for _, test := range []struct {
+		name    string
+		setup   func(t *testing.T, dir string)
+		wantErr bool
+	}{
+		{
+			name: "success",
+			setup: func(t *testing.T, dir string) {
+				gitRepo, err := git.PlainInit(dir, false)
+				if err != nil {
+					t.Fatalf("git.PlainInit failed: %v", err)
+				}
+				w, err := gitRepo.Worktree()
+				if err != nil {
+					t.Fatalf("Worktree() failed: %v", err)
+				}
+				if err := os.WriteFile(filepath.Join(dir, "README.md"), []byte("test"), 0644); err != nil {
+					t.Fatal(err)
+				}
+				if _, err := w.Add("README.md"); err != nil {
+					t.Fatal(err)
+				}
+				if _, err := w.Commit("initial commit", &git.CommitOptions{
+					Author: &object.Signature{Name: "Test", Email: "[email protected]"},
+				}); err != nil {
+					t.Fatal(err)
+				}
+			},
+		},
+		{
+			name: "error",
+			setup: func(t *testing.T, dir string) {
+				if _, err := git.PlainInit(dir, false); err != nil {
+					t.Fatalf("git.PlainInit failed: %v", err)
+				}
+			},
+			wantErr: true,
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+			dir := t.TempDir()
+			test.setup(t, dir)
+			repo, err := NewRepository(&RepositoryOptions{Dir: dir})
+			if err != nil {
+				t.Fatalf("NewRepository() failed: %v", err)
+			}
+			_, err = repo.HeadHash()
+			if (err != nil) != test.wantErr {
+				t.Errorf("HeadHash() error = %v, wantErr %v", err, test.wantErr)
+			}
+		})
+	}
+}
 func TestGetDir(t *testing.T) {
 	t.Parallel()
 	want := "/test/dir"

internal/librarian/generate.go

@@ -15,7 +15,6 @@
 package librarian
 
 import (
-	"bytes"
 	"context"
 	"encoding/json"
 	"errors"
@@ -29,8 +28,6 @@ import (
 	"strings"
 	"time"
 
-	"gopkg.in/yaml.v3"
-
 	"github.com/googleapis/librarian/internal/cli"
 	"github.com/googleapis/librarian/internal/config"
 	"github.com/googleapis/librarian/internal/docker"
@@ -94,6 +91,7 @@ func init() {
 type generateRunner struct {
 	cfg             *config.Config
 	repo            gitrepo.Repository
+	sourceRepo      gitrepo.Repository
 	state           *config.LibrarianState
 	ghClient        GitHubClient
 	containerClient ContainerClient
@@ -117,15 +115,16 @@ func newGenerateRunner(cfg *config.Config) (*generateRunner, error) {
 	if apiSource == "" {
 		apiSource = "https://github.com/googleapis/googleapis"
 	}
-	if _, err := cloneOrOpenRepo(workRoot, apiSource, cfg.CI); err != nil {
+	sourceRepo, err := cloneOrOpenRepo(workRoot, apiSource, cfg.CI)
+	if err != nil {
 		return nil, err
 	}
 
 	languageRepo, err := cloneOrOpenRepo(workRoot, cfg.Repo, cfg.CI)
 	if err != nil {
 		return nil, err
 	}
-	state, err := loadRepoState(languageRepo, cfg.APISource)
+	state, err := loadRepoState(languageRepo, sourceRepo.GetDir())
 	if err != nil {
 		return nil, err
 	}
@@ -151,6 +150,7 @@ func newGenerateRunner(cfg *config.Config) (*generateRunner, error) {
 		cfg:             cfg,
 		workRoot:        workRoot,
 		repo:            languageRepo,
+		sourceRepo:      sourceRepo,
 		state:           state,
 		image:           image,
 		ghClient:        ghClient,
@@ -189,6 +189,9 @@ func (r *generateRunner) run(ctx context.Context) error {
 		}
 	}
 
+	if err := saveLibrarianState(r.repo.GetDir(), r.state); err != nil {
+		return err
+	}
 	if err := commitAndPush(ctx, r, prBody); err != nil {
 		return err
 	}
@@ -219,13 +222,30 @@ func (r *generateRunner) generateSingleLibrary(ctx context.Context, libraryID, o
 	if err := r.runBuildCommand(ctx, generatedLibraryID); err != nil {
 		return err
 	}
+	if err := r.updateLastGeneratedCommitState(generatedLibraryID); err != nil {
+		return err
+	}
 	return nil
 }
 
 func (r *generateRunner) needsConfigure() bool {
 	return r.cfg.API != "" && r.cfg.Library != "" && findLibraryByID(r.state, r.cfg.Library) == nil
 }
 
+func (r *generateRunner) updateLastGeneratedCommitState(libraryID string) error {
+	hash, err := r.sourceRepo.HeadHash()
+	if err != nil {
+		return err
+	}
+	for _, l := range r.state.Libraries {
+		if l.ID == libraryID {
+			l.LastGeneratedCommit = hash
+			break
+		}
+	}
+	return nil
+}
+
 // runGenerateCommand attempts to perform generation for an API. It then cleans the
 // destination directory and copies the newly generated files into it.
 //
@@ -527,25 +547,5 @@ func (r *generateRunner) runConfigureCommand(ctx context.Context) (string, error
 		r.state.Libraries[i] = libraryState
 	}
 
-	// Write the updated librarian state to state.yaml.
-	if err := writeLibrarianState(
-		func(state *config.LibrarianState) ([]byte, error) {
-			data := &bytes.Buffer{}
-			encoder := yaml.NewEncoder(data)
-			encoder.SetIndent(2)
-			if err := encoder.Encode(state); err != nil {
-				return nil, err
-			}
-
-			if err := encoder.Close(); err != nil {
-				return nil, err
-			}
-			return data.Bytes(), nil
-		},
-		r.state,
-		r.repo.GetDir()); err != nil {
-		return "", err
-	}
-
 	return libraryState.ID, nil
 }