chore(internal/godocfx): test a specific golden package version (#8759)

I also updated another test to be less restrictive with the number of files that get generated.

Fixes #8130.

internal/godocfx/go.mod

@@ -6,9 +6,7 @@ require (
 	cloud.google.com/go v0.110.2
 	cloud.google.com/go/bigquery v1.51.2
 	cloud.google.com/go/datastore v1.11.0
-	cloud.google.com/go/storage v1.30.1
 	github.com/google/go-cmp v0.5.9
-	golang.org/x/sync v0.2.0
 	golang.org/x/tools v0.9.3
 	gopkg.in/yaml.v2 v2.2.8
 )
@@ -17,6 +15,7 @@ require (
 	cloud.google.com/go/compute v1.19.3 // indirect
 	cloud.google.com/go/compute/metadata v0.2.3 // indirect
 	cloud.google.com/go/iam v0.13.0 // indirect
+	cloud.google.com/go/storage v1.30.1 // indirect
 	github.com/andybalholm/brotli v1.0.4 // indirect
 	github.com/apache/arrow/go/v12 v12.0.0 // indirect
 	github.com/apache/thrift v0.16.0 // indirect
@@ -43,6 +42,7 @@ require (
 	golang.org/x/mod v0.10.0 // indirect
 	golang.org/x/net v0.17.0 // indirect
 	golang.org/x/oauth2 v0.8.0 // indirect
+	golang.org/x/sync v0.2.0 // indirect
 	golang.org/x/sys v0.13.0 // indirect
 	golang.org/x/text v0.13.0 // indirect
 	golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect

internal/godocfx/godocfx_test.go

@@ -19,7 +19,7 @@ import (
 	"encoding/json"
 	"flag"
 	"fmt"
-	"io/ioutil"
+	"io/fs"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -28,9 +28,7 @@ import (
 	"time"
 
 	_ "cloud.google.com/go/bigquery" // Implicitly required by test.
-	_ "cloud.google.com/go/storage"  // Implicitly required by test.
 	"github.com/google/go-cmp/cmp"
-	_ "golang.org/x/sync/semaphore" // Implicitly required by test.
 	"golang.org/x/tools/go/packages"
 )
 
@@ -70,8 +68,8 @@ func TestParse(t *testing.T) {
 	if got, want := len(r.toc), 1; got != want {
 		t.Fatalf("Parse got len(toc) = %d, want %d", got, want)
 	}
-	if got, want := len(r.pages), 33; got != want {
-		t.Errorf("Parse got len(pages) = %d, want %d", got, want)
+	if got, want := len(r.pages), 33; got < want {
+		t.Errorf("Parse got len(pages) = %d, want at least %d", got, want)
 	}
 	if got := r.module.Path; got != mod {
 		t.Fatalf("Parse got module = %q, want %q", got, mod)
@@ -124,77 +122,77 @@ func TestParse(t *testing.T) {
 }
 
 func TestGoldens(t *testing.T) {
-	t.Skip("test is too flaky with dep bumps, consider removing")
 	gotDir := "testdata/out"
 	goldenDir := "testdata/golden"
+	if updateGoldens {
+		os.RemoveAll(gotDir)
+		os.RemoveAll(goldenDir)
+		gotDir = goldenDir
+	}
 	extraFiles := []string{"README.md"}
 
-	testPath := "cloud.google.com/go/storage"
+	testMod := indexEntry{Path: "cloud.google.com/go/storage", Version: "v1.33.0"}
 	metaServer := fakeMetaServer()
 	defer metaServer.Close()
-	r, err := parse(testPath, ".", extraFiles, nil, &friendlyAPINamer{metaURL: metaServer.URL})
-	if err != nil {
-		t.Fatalf("parse: %v", err)
+	namer := &friendlyAPINamer{metaURL: metaServer.URL}
+	ok := processMods([]indexEntry{testMod}, gotDir, namer, extraFiles, false)
+	if !ok {
+		t.Fatalf("failed to process modules")
 	}
 
-	ignoreFiles := map[string]bool{"docs.metadata": true}
+	ignoreGoldens := []string{fmt.Sprintf("%s@%s/docs.metadata", testMod.Path, testMod.Version)}
 
 	if updateGoldens {
-		os.RemoveAll(goldenDir)
-
-		if err := write(goldenDir, r); err != nil {
-			t.Fatalf("write: %v", err)
-		}
-
-		for ignore := range ignoreFiles {
+		for _, ignore := range ignoreGoldens {
 			if err := os.Remove(filepath.Join(goldenDir, ignore)); err != nil {
 				t.Fatalf("Remove: %v", err)
 			}
 		}
-
 		t.Logf("Successfully updated goldens in %s", goldenDir)
-
 		return
 	}
 
-	if err := write(gotDir, r); err != nil {
-		t.Fatalf("write: %v", err)
-	}
-
-	gotFiles, err := ioutil.ReadDir(gotDir)
-	if err != nil {
-		t.Fatalf("ReadDir: %v", err)
-	}
-
-	goldens, err := ioutil.ReadDir(goldenDir)
-	if err != nil {
-		t.Fatalf("ReadDir: %v", err)
-	}
-
-	if got, want := len(gotFiles)-len(ignoreFiles), len(goldens); got != want {
-		t.Fatalf("parse & write got %d files in %s, want %d ignoring %v", got, gotDir, want, ignoreFiles)
-	}
-
-	for _, golden := range goldens {
-		if golden.IsDir() {
-			continue
+	goldenCount := 0
+	err := filepath.WalkDir(goldenDir, func(goldenPath string, d fs.DirEntry, err error) error {
+		goldenCount++
+		if d.IsDir() {
+			return nil
 		}
-		gotPath := filepath.Join(gotDir, golden.Name())
-		goldenPath := filepath.Join(goldenDir, golden.Name())
+		if err != nil {
+			return err
+		}
+
+		gotPath := filepath.Join(gotDir, goldenPath[len(goldenDir):])
 
-		gotContent, err := ioutil.ReadFile(gotPath)
+		gotContent, err := os.ReadFile(gotPath)
 		if err != nil {
-			t.Fatalf("ReadFile: %v", err)
+			t.Fatalf("failed to read got: %v", err)
 		}
 
-		goldenContent, err := ioutil.ReadFile(goldenPath)
+		goldenContent, err := os.ReadFile(goldenPath)
 		if err != nil {
-			t.Fatalf("ReadFile: %v", err)
+			t.Fatalf("failed to read golden: %v", err)
 		}
 
 		if string(gotContent) != string(goldenContent) {
 			t.Errorf("got %s is different from expected %s", gotPath, goldenPath)
 		}
+
+		return nil
+	})
+	if err != nil {
+		t.Fatalf("failed to compare goldens: %v", err)
+	}
+	gotCount := 0
+	err = filepath.WalkDir(gotDir, func(_ string, _ fs.DirEntry, _ error) error {
+		gotCount++
+		return nil
+	})
+	if err != nil {
+		t.Fatalf("failed to count got: %v", err)
+	}
+	if gotCount-len(ignoreGoldens) != goldenCount {
+		t.Fatalf("processMods got %d files in %s, want %d ignoring %v", gotCount, gotDir, goldenCount, ignoreGoldens)
 	}
 }
 

internal/godocfx/main.go

@@ -102,30 +102,14 @@ func main() {
 		os.RemoveAll(*outDir)
 	}
 	if len(mods) == 0 {
-		log.Println("No new modules to process")
+		log.Println("No modules to process")
 		return
 	}
-	// Create a temp module so we can get the exact version asked for.
-	workingDir, err := ioutil.TempDir("", "godocfx-*")
-	if err != nil {
-		log.Fatalf("ioutil.TempDir: %v", err)
-	}
-	// Use a fake module that doesn't start with cloud.google.com/go.
-	runCmd(workingDir, "go", "mod", "init", "cloud.google.com/lets-build-some-docs")
-
-	failed := false
-	for _, m := range mods {
-		log.Printf("Processing %s@%s", m.Path, m.Version)
-
-		// Always output to specific directory.
-		path := filepath.Join(*outDir, fmt.Sprintf("%s@%s", m.Path, m.Version))
-		if err := process(m, workingDir, path, *print); err != nil {
-			log.Printf("Failed to process %v: %v", m, err)
-			failed = true
-		}
-		log.Printf("Done with %s@%s", m.Path, m.Version)
+	namer := &friendlyAPINamer{
+		metaURL: "https://raw.githubusercontent.com/googleapis/google-cloud-go/main/internal/.repo-metadata-full.json",
 	}
-	if failed {
+	optionalExtraFiles := []string{}
+	if ok := processMods(mods, *outDir, namer, optionalExtraFiles, *print); !ok {
 		os.Exit(1)
 	}
 }
@@ -145,7 +129,31 @@ func runCmd(dir, name string, args ...string) error {
 	return nil
 }
 
-func process(mod indexEntry, workingDir, outDir string, print bool) error {
+func processMods(mods []indexEntry, outDir string, namer *friendlyAPINamer, optionalExtraFiles []string, print bool) bool {
+	// Create a temp module so we can get the exact version asked for.
+	workingDir, err := ioutil.TempDir("", "godocfx-*")
+	if err != nil {
+		log.Fatalf("ioutil.TempDir: %v", err)
+	}
+	// Use a fake module that doesn't start with cloud.google.com/go.
+	runCmd(workingDir, "go", "mod", "init", "cloud.google.com/lets-build-some-docs")
+
+	ok := true
+	for _, m := range mods {
+		log.Printf("Processing %s@%s", m.Path, m.Version)
+
+		// Always output to specific directory.
+		path := filepath.Join(outDir, fmt.Sprintf("%s@%s", m.Path, m.Version))
+		if err := process(m, workingDir, path, namer, optionalExtraFiles, print); err != nil {
+			log.Printf("Failed to process %v: %v", m, err)
+			ok = false
+		}
+		log.Printf("Done with %s@%s", m.Path, m.Version)
+	}
+	return ok
+}
+
+func process(mod indexEntry, workingDir, outDir string, namer *friendlyAPINamer, optionalExtraFiles []string, print bool) error {
 	filter := []string{
 		"cloud.google.com/go/analytics",
 		"cloud.google.com/go/area120",
@@ -168,10 +176,6 @@ func process(mod indexEntry, workingDir, outDir string, print bool) error {
 	}
 
 	log.Println("Starting to parse")
-	optionalExtraFiles := []string{}
-	namer := &friendlyAPINamer{
-		metaURL: "https://raw.githubusercontent.com/googleapis/google-cloud-go/main/internal/.repo-metadata-full.json",
-	}
 	r, err := parse(mod.Path+"/...", workingDir, optionalExtraFiles, filter, namer)
 	if err != nil {
 		return fmt.Errorf("parse: %v", err)

internal/godocfx/testdata/golden/index.yml → internal/godocfx/testdata/golden/cloud.google.com/go/storage@v1.33.0/index.yml

@@ -15,7 +15,7 @@ items:
     application credentials. Clients should be\nreused instead of created as needed.
     The methods of <a href=\"#Client\">Client</a> are safe for\nconcurrent use by
     multiple goroutines.\n<p>You may configure the client by passing in options from
-    the <a href=\"/google.golang.org/api/option\">google.golang.org/api/option</a>\npackage.
+    the <a href=\"https://pkg.go.dev/google.golang.org/api/option\">google.golang.org/api/option</a>\npackage.
     You may also use options defined in this package, such as <a href=\"#WithJSONReads\">WithJSONReads</a>.\n<p>If
     you only wish to access public data, you can create\nan unauthenticated client
     with\n<pre class=\"prettyprint\">client, err := storage.NewClient(ctx, option.WithoutAuthentication())\n</pre>\n<p>To
@@ -159,7 +159,7 @@ items:
     cancel()\n\n// Delete an object using the specified strategy and timeout.\nif
     err := o.Delete(ctx); err != nil {\n\t// Handle err.\n}\n</pre>\n<h2 id=\"hdr-Sending_Custom_Headers\">Sending
     Custom Headers</h2>\n<p>You can add custom headers to any API call made by this
-    package by using\n<a href=\"/github.com/googleapis/gax-go/v2/callctx#SetHeaders\">callctx.SetHeaders</a>
+    package by using\n<a href=\"https://pkg.go.dev/github.com/googleapis/gax-go/v2/callctx#SetHeaders\">callctx.SetHeaders</a>
     on the context which is passed to the method. For example,\nto add a <a href=\"https://cloud.google.com/storage/docs/audit-logging#add-custom-metadata\">custom
     audit logging</a> header:\n<pre class=\"prettyprint\">ctx := context.Background()\nctx
     = callctx.SetHeaders(ctx, &quot;x-goog-custom-audit-&lt;key&gt;&quot;, &quot;&lt;value&gt;&quot;)\n//