fix(internal/librarian): make response files optional for generate/build (#1788)

readLibraryState() now returns a nil pointer (but no error) if the
container response file doesn't exist. When the response is required
(e.g. for configure) this should be checked explicitly.

Fixes #1741

Author: jskeet

internal/librarian/generate.go

@@ -506,6 +506,9 @@ func (r *generateRunner) runConfigureCommand(ctx context.Context) (string, error
 	if err != nil {
 		return "", err
 	}
+	if libraryState == nil {
+		return "", errors.New("no response file for configure container command")
+	}
 
 	if libraryState.Version == "" {
 		slog.Info("library doesn't receive a version, apply the default version", "id", r.cfg.Library)

internal/librarian/generate_test.go

@@ -59,6 +59,25 @@ func TestRunGenerateCommand(t *testing.T) {
 			wantLibraryID:     "some-library",
 			wantGenerateCalls: 1,
 		},
+		{
+			name:     "works with no response",
+			api:      "some/api",
+			repo:     newTestGitRepo(t),
+			ghClient: &mockGitHubClient{},
+			state: &config.LibrarianState{
+				Libraries: []*config.LibraryState{
+					{
+						ID:   "some-library",
+						APIs: []*config.API{{Path: "some/api"}},
+					},
+				},
+			},
+			container: &mockContainerClient{
+				noGenerateResponse: true,
+			},
+			wantLibraryID:     "some-library",
+			wantGenerateCalls: 1,
+		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			t.Parallel()
@@ -143,7 +162,7 @@ func TestRunBuildCommand(t *testing.T) {
 			container: &mockContainerClient{
 				noBuildResponse: true,
 			},
-			wantErr: true,
+			wantBuildCalls: 1,
 		},
 		{
 			name:      "build with error response in response",
@@ -271,7 +290,7 @@ func TestRunConfigureCommand(t *testing.T) {
 			},
 			wantConfigureCalls: 1,
 			wantErr:            true,
-			wantErrMsg:         "failed to read response file",
+			wantErrMsg:         "no response file for configure container command",
 		},
 		{
 			name: "configures library without initial version",

internal/librarian/mocks_test.go

@@ -103,6 +103,7 @@ type mockContainerClient struct {
 	requestLibraryID    string
 	noBuildResponse     bool
 	noConfigureResponse bool
+	noGenerateResponse  bool
 	noInitVersion       bool
 	wantErrorMsg        bool
 }
@@ -112,8 +113,7 @@ func (m *mockContainerClient) Build(ctx context.Context, request *docker.BuildRe
 	if m.noBuildResponse {
 		return m.buildErr
 	}
-	// Write a build-response.json because it is required by generate
-	// command.
+	// Write a build-response.json unless we're configured not to.
 	if err := os.MkdirAll(filepath.Join(request.RepoDir, ".librarian"), 0755); err != nil {
 		return err
 	}
@@ -135,8 +135,7 @@ func (m *mockContainerClient) Configure(ctx context.Context, request *docker.Con
 		return "", m.configureErr
 	}
 
-	// Write a configure-response.json because it is required by configure
-	// command.
+	// Write a configure-response.json unless we're configured not to.
 	if err := os.MkdirAll(filepath.Join(request.RepoDir, config.LibrarianDir), 0755); err != nil {
 		return "", err
 	}
@@ -158,8 +157,12 @@ func (m *mockContainerClient) Configure(ctx context.Context, request *docker.Con
 
 func (m *mockContainerClient) Generate(ctx context.Context, request *docker.GenerateRequest) error {
 	m.generateCalls++
-	// Write a generate-response.json because it is required by generate
-	// command.
+
+	if m.noGenerateResponse {
+		return m.generateErr
+	}
+
+	// // Write a generate-response.json unless we're configured not to.
 	if err := os.MkdirAll(filepath.Join(request.RepoDir, config.LibrarianDir), 0755); err != nil {
 		return err
 	}

internal/librarian/state.go

@@ -158,17 +158,24 @@ func saveLibrarianState(repoDir string, state *config.LibrarianState) error {
 	return os.WriteFile(path, bytes, 0644)
 }
 
-// readLibraryState reads the library state from a container response.
+// readLibraryState reads the library state from a container response, if it exists.
+// If the response file does not exist, readLibraryState succeeds but returns a nil pointer.
 //
 // The response file is removed afterwards.
 func readLibraryState(jsonFilePath string) (*config.LibraryState, error) {
 	data, err := os.ReadFile(jsonFilePath)
 	defer func() {
-		if err := os.Remove(jsonFilePath); err != nil {
+		if err := os.Remove(jsonFilePath); err != nil && !errors.Is(err, os.ErrNotExist) {
 			slog.Warn("fail to remove file", slog.String("name", jsonFilePath), slog.Any("err", err))
 		}
 	}()
 	if err != nil {
+		// If we only failed to read the file because it didn't exist, just succeed
+		// with a nil pointer.
+
+		if errors.Is(err, os.ErrNotExist) {
+			return nil, nil
+		}
 		return nil, fmt.Errorf("failed to read response file, path: %s, error: %w", jsonFilePath, err)
 	}
 

internal/librarian/state_test.go

@@ -346,7 +346,7 @@ func TestSaveLibrarianState(t *testing.T) {
 	}
 }
 
-func TestReadConfigureResponseJSON(t *testing.T) {
+func TestReadLibraryState(t *testing.T) {
 	t.Parallel()
 	for _, test := range []struct {
 		name         string
@@ -385,6 +385,10 @@ func TestReadConfigureResponseJSON(t *testing.T) {
 			name:      "invalid file name",
 			wantState: nil,
 		},
+		{
+			name:      "missing file",
+			wantState: nil,
+		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			tempDir := t.TempDir()
@@ -402,6 +406,18 @@ func TestReadConfigureResponseJSON(t *testing.T) {
 				return
 			}
 
+			if test.name == "missing file" {
+				filePath := filepath.Join(tempDir, "missing.json")
+				gotState, err := readLibraryState(filePath)
+				if err != nil {
+					t.Fatalf("readLibraryState() unexpected error: %v", err)
+				}
+				if diff := cmp.Diff(test.wantState, gotState); diff != "" {
+					t.Errorf("Response library state mismatch (-want +got):\n%s", diff)
+				}
+				return
+			}
+
 			if test.name == "invalid content loader" {
 				dst := fmt.Sprintf("%s/copy.json", os.TempDir())
 				if err := copyFile(dst, test.jsonFilePath); err != nil {