refactor: makes fetch.LatestSha and fetch.Sha256 private and refactor to tests (#3170)

This is a followup to #3143 that:
- adds a test for failures in `LatestCommitAndChecksum`
- refactor `internal/sidekick/config/update_root_config.go` to use, and
simplify it's test.
- renames to make fetch.LatestSha and fetch.Sha256 private as they are
not used in sidekick anymore.


Fix #3068

Author: zhumin8

internal/fetch/fetch.go

@@ -72,9 +72,9 @@ func RepoFromTarballLink(githubDownload, tarballLink string) (*Repo, error) {
 	return repo, nil
 }
 
-// Sha256 downloads the content from the given URL and returns its SHA256
+// urlSha256 downloads the content from the given URL and returns its SHA256
 // checksum as a hex string.
-func Sha256(query string) (string, error) {
+func urlSha256(query string) (string, error) {
 	response, err := http.Get(query)
 	if err != nil {
 		return "", err
@@ -92,9 +92,9 @@ func Sha256(query string) (string, error) {
 	return got, nil
 }
 
-// LatestSha fetches the latest commit SHA from the GitHub API for the given
+// latestSha fetches the latest commit SHA from the GitHub API for the given
 // repository URL.
-func LatestSha(query string) (string, error) {
+func latestSha(query string) (string, error) {
 	client := &http.Client{}
 	request, err := http.NewRequest(http.MethodGet, query, nil)
 	if err != nil {
@@ -120,13 +120,13 @@ func LatestSha(query string) (string, error) {
 // commit from the GitHub API for the given repository.
 func LatestCommitAndChecksum(endpoints *Endpoints, repo *Repo) (commit, sha256 string, err error) {
 	apiURL := fmt.Sprintf("%s/repos/%s/%s/commits/%s", endpoints.API, repo.Org, repo.Repo, branch)
-	commit, err = LatestSha(apiURL)
+	commit, err = latestSha(apiURL)
 	if err != nil {
 		return "", "", err
 	}
 
 	tarballURL := TarballLink(endpoints.Download, repo, commit)
-	sha256, err = Sha256(tarballURL)
+	sha256, err = urlSha256(tarballURL)
 	if err != nil {
 		return "", "", err
 	}

internal/fetch/fetch_test.go

@@ -26,6 +26,7 @@ import (
 	"net/http/httptest"
 	"os"
 	"path"
+	"strings"
 	"testing"
 	"time"
 
@@ -89,7 +90,7 @@ func TestSha256(t *testing.T) {
 	}))
 	defer server.Close()
 
-	got, err := Sha256(server.URL + tarballPath)
+	got, err := urlSha256(server.URL + tarballPath)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -120,7 +121,7 @@ func TestSha256Error(t *testing.T) {
 		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
-			if _, err := Sha256(test.url); err == nil {
+			if _, err := urlSha256(test.url); err == nil {
 				t.Error("expected an error from Sha256()")
 			}
 		})
@@ -129,8 +130,8 @@ func TestSha256Error(t *testing.T) {
 
 func TestLatestSha(t *testing.T) {
 	const (
-		getLatestShaPath = "/repos/googleapis/googleapis/commits/master"
-		latestSha        = "5d5b1bf126485b0e2c972bac41b376438601e266"
+		getLatestShaPath  = "/repos/googleapis/googleapis/commits/master"
+		expectedCommitSha = "5d5b1bf126485b0e2c972bac41b376438601e266"
 	)
 
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -143,16 +144,16 @@ func TestLatestSha(t *testing.T) {
 			t.Fatalf("mismatched Accept header for %q, got=%q, want=%s", r.URL.Path, got, want)
 		}
 		w.WriteHeader(http.StatusOK)
-		w.Write([]byte(latestSha))
+		w.Write([]byte(expectedCommitSha))
 	}))
 	defer server.Close()
 
-	got, err := LatestSha(server.URL + getLatestShaPath)
+	got, err := latestSha(server.URL + getLatestShaPath)
 	if err != nil {
 		t.Fatal(err)
 	}
-	if got != latestSha {
-		t.Errorf("LatestSha() = %q, want %q", got, latestSha)
+	if got != expectedCommitSha {
+		t.Errorf("LatestSha() = %q, want %q", got, expectedCommitSha)
 	}
 }
 
@@ -178,7 +179,7 @@ func TestLatestShaError(t *testing.T) {
 		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
-			if _, err := LatestSha(test.url); err == nil {
+			if _, err := latestSha(test.url); err == nil {
 				t.Error("expected an error from LatestSha()")
 			}
 		})
@@ -619,3 +620,49 @@ func TestDownloadTarballRetry(t *testing.T) {
 		}
 	})
 }
+
+func TestLatestCommitAndChecksumFailure(t *testing.T) {
+	const (
+		commit   = "test-commit-sha"
+		testOrg  = "test-org"
+		testRepo = "test-repo"
+	)
+
+	t.Run("LatestSha fails", func(t *testing.T) {
+		server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			// Fail the first call, which is to get the latest SHA
+			http.Error(w, "failed to get latest sha", http.StatusInternalServerError)
+		}))
+		defer server.Close()
+
+		endpoints := &Endpoints{API: server.URL, Download: server.URL}
+		repo := &Repo{Org: testOrg, Repo: testRepo}
+
+		_, _, err := LatestCommitAndChecksum(endpoints, repo)
+		if err == nil {
+			t.Error("expected an error when LatestSha fails, but got nil")
+		}
+	})
+
+	t.Run("Sha256 fails", func(t *testing.T) {
+		server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			// The first call is for the latest SHA, which should succeed.
+			if strings.HasSuffix(r.URL.Path, "/commits/master") {
+				w.WriteHeader(http.StatusOK)
+				w.Write([]byte(commit))
+				return
+			}
+			// The second call is for the tarball, which should fail.
+			http.Error(w, "failed to download tarball", http.StatusInternalServerError)
+		}))
+		defer server.Close()
+
+		endpoints := &Endpoints{API: server.URL, Download: server.URL}
+		repo := &Repo{Org: testOrg, Repo: testRepo}
+
+		_, _, err := LatestCommitAndChecksum(endpoints, repo)
+		if err == nil {
+			t.Error("expected an error when Sha256 fails, but got nil")
+		}
+	})
+}

internal/sidekick/config/update_root_config.go

@@ -22,10 +22,11 @@ import (
 	"github.com/googleapis/librarian/internal/fetch"
 )
 
+var latestCommitAndChecksum = fetch.LatestCommitAndChecksum
+
 const (
 	defaultGitHubAPI = "https://api.github.com"
 	defaultGitHubDn  = "https://github.com"
-	branch           = "master"
 	defaultRoot      = "googleapis"
 )
 
@@ -40,16 +41,7 @@ func UpdateRootConfig(rootConfig *Config, rootName string) error {
 		return err
 	}
 
-	query := fmt.Sprintf("%s/repos/%s/%s/commits/%s", endpoints.API, repo.Org, repo.Repo, branch)
-	fmt.Printf("getting latest SHA from %q\n", query)
-	latestSha, err := fetch.LatestSha(query)
-	if err != nil {
-		return err
-	}
-
-	newLink := fetch.TarballLink(endpoints.Download, repo, latestSha)
-	fmt.Printf("computing SHA256 for %q\n", newLink)
-	newSha256, err := fetch.Sha256(newLink)
+	latestSha, newSha256, err := latestCommitAndChecksum(endpoints, repo)
 	if err != nil {
 		return err
 	}