internal: move templatecheck tests to their own package

The new package the templatecheck tests are in is not a transitive
dependency of cmd/pkgsite, so that cmd/pkgsite no longer has a
transitive test dependency on github.com/jba/templatecheck.

To make this work we had to expose the templates from the
internal/frontend and internal/godoc packages for the tests to use.

For golang/go#61399

Change-Id: I1290ec24b53af77a82671c8fc068867e12857ce3
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/550936
LUCI-TryBot-Result: Go LUCI <[email protected]>
Run-TryBot: Michael Matloob <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
kokoro-CI: kokoro <[email protected]>
Reviewed-by: Jonathan Amsterdam <[email protected]>

Author: matloob

internal/frontend/badge.go

@@ -11,7 +11,7 @@ import (
 	"golang.org/x/pkgsite/internal/frontend/page"
 )
 
-type badgePage struct {
+type BadgePage struct {
 	page.BasePage
 	// LinkPath is the URL path of the badge will link to.
 	LinkPath string
@@ -36,7 +36,7 @@ func (s *Server) badgeHandler(w http.ResponseWriter, r *http.Request) {
 		path = path[urlSchemeIdx+3:]
 	}
 
-	page := badgePage{
+	page := BadgePage{
 		BasePage:  s.newBasePage(r, "Badge"),
 		LinkPath:  path,
 		BadgePath: "badge/" + path + ".svg",

internal/frontend/frontend_test.go

@@ -14,10 +14,7 @@ import (
 	"time"
 
 	"github.com/google/safehtml/template"
-	"github.com/jba/templatecheck"
 	"golang.org/x/pkgsite/internal"
-	"golang.org/x/pkgsite/internal/frontend/page"
-	"golang.org/x/pkgsite/internal/frontend/versions"
 	"golang.org/x/pkgsite/internal/testing/fakedatasource"
 	"golang.org/x/pkgsite/static"
 	thirdparty "golang.org/x/pkgsite/third_party"
@@ -132,84 +129,6 @@ func TestTagRoute(t *testing.T) {
 	}
 }
 
-func TestCheckTemplates(t *testing.T) {
-	// Perform additional checks on parsed templates.
-	staticFS := template.TrustedFSFromEmbed(static.FS)
-	templates, err := parsePageTemplates(staticFS)
-	if err != nil {
-		t.Fatal(err)
-	}
-	for _, c := range []struct {
-		name    string
-		subs    []string
-		typeval any
-	}{
-		{"badge", nil, badgePage{}},
-		// error.tmpl omitted because relies on an associated "message" template
-		// that's parsed on demand; see renderErrorPage above.
-		{"fetch", nil, page.ErrorPage{}},
-		{"homepage", nil, homepage{}},
-		{"license-policy", nil, licensePolicyPage{}},
-		{"search", nil, SearchPage{}},
-		{"search-help", nil, page.BasePage{}},
-		{"unit/main", nil, UnitPage{}},
-		{
-			"unit/main",
-			[]string{"unit-outline", "unit-readme", "unit-doc", "unit-files", "unit-directories"},
-			MainDetails{},
-		},
-		{"unit/importedby", nil, UnitPage{}},
-		{"unit/importedby", []string{"importedby"}, ImportedByDetails{}},
-		{"unit/imports", nil, UnitPage{}},
-		{"unit/imports", []string{"imports"}, ImportsDetails{}},
-		{"unit/licenses", nil, UnitPage{}},
-		{"unit/licenses", []string{"licenses"}, LicensesDetails{}},
-		{"unit/versions", nil, UnitPage{}},
-		{"unit/versions", []string{"versions"}, versions.VersionsDetails{}},
-		{"vuln", nil, page.BasePage{}},
-		{"vuln/list", nil, VulnListPage{}},
-		{"vuln/entry", nil, VulnEntryPage{}},
-	} {
-		t.Run(c.name, func(t *testing.T) {
-			tm := templates[c.name]
-			if tm == nil {
-				t.Fatalf("no template %q", c.name)
-			}
-			if c.subs == nil {
-				if err := templatecheck.CheckSafe(tm, c.typeval); err != nil {
-					t.Fatal(err)
-				}
-			} else {
-				for _, n := range c.subs {
-					s := tm.Lookup(n)
-					if s == nil {
-						t.Fatalf("no sub-template %q of %q", n, c.name)
-					}
-					if err := templatecheck.CheckSafe(s, c.typeval); err != nil {
-						t.Fatalf("%s: %v", n, err)
-					}
-				}
-			}
-		})
-	}
-}
-
-func TestStripScheme(t *testing.T) {
-	for _, test := range []struct {
-		url, want string
-	}{
-		{"http://github.com", "github.com"},
-		{"https://github.com/path/to/something", "github.com/path/to/something"},
-		{"example.com", "example.com"},
-		{"chrome-extension://abcd", "abcd"},
-		{"nonwellformed.com/path?://query=1", "query=1"},
-	} {
-		if got := stripScheme(test.url); got != test.want {
-			t.Errorf("%q: got %q, want %q", test.url, got, test.want)
-		}
-	}
-}
-
 func TestInstallFS(t *testing.T) {
 	s, handler := newTestServer(t, nil)
 	s.InstallFS("/dir", os.DirFS("."))

internal/frontend/homepage.go

@@ -39,7 +39,7 @@ var searchTips = []searchTip{
 }
 
 // Homepage contains fields used in rendering the homepage template.
-type homepage struct {
+type Homepage struct {
 	page.BasePage
 
 	// TipIndex is the index of the initial search tip to render.
@@ -62,7 +62,7 @@ type LocalModule struct {
 }
 
 func (s *Server) serveHomepage(ctx context.Context, w http.ResponseWriter, r *http.Request) {
-	s.servePage(ctx, w, "homepage", homepage{
+	s.servePage(ctx, w, "homepage", Homepage{
 		BasePage:     s.newBasePage(r, "Go Packages"),
 		SearchTips:   searchTips,
 		TipIndex:     rand.Intn(len(searchTips)),

internal/frontend/server.go

@@ -14,9 +14,7 @@ import (
 	"io/fs"
 	"net/http"
 	hpprof "net/http/pprof"
-	"net/url"
 	"os"
-	"path"
 	"strings"
 	"sync"
 	"time"
@@ -28,6 +26,7 @@ import (
 	"golang.org/x/pkgsite/internal/experiment"
 	pagepkg "golang.org/x/pkgsite/internal/frontend/page"
 	"golang.org/x/pkgsite/internal/frontend/serrors"
+	"golang.org/x/pkgsite/internal/frontend/templates"
 	"golang.org/x/pkgsite/internal/frontend/urlinfo"
 	"golang.org/x/pkgsite/internal/godoc/dochtml"
 	"golang.org/x/pkgsite/internal/licenses"
@@ -37,8 +36,6 @@ import (
 	"golang.org/x/pkgsite/internal/queue"
 	"golang.org/x/pkgsite/internal/version"
 	"golang.org/x/pkgsite/internal/vuln"
-	"golang.org/x/text/cases"
-	"golang.org/x/text/language"
 )
 
 // Server can be installed to serve the go discovery frontend.
@@ -102,7 +99,7 @@ type ServerConfig struct {
 // NewServer creates a new Server for the given database and template directory.
 func NewServer(scfg ServerConfig) (_ *Server, err error) {
 	defer derrors.Wrap(&err, "NewServer(...)")
-	ts, err := parsePageTemplates(scfg.TemplateFS)
+	ts, err := templates.ParsePageTemplates(scfg.TemplateFS)
 	if err != nil {
 		return nil, fmt.Errorf("error parsing templates: %v", err)
 	}
@@ -434,8 +431,8 @@ func (s *Server) staticPageHandler(templateName, title string) http.HandlerFunc
 	}
 }
 
-// licensePolicyPage is used to generate the static license policy page.
-type licensePolicyPage struct {
+// LicensePolicyPage is used to generate the static license policy page.
+type LicensePolicyPage struct {
 	pagepkg.BasePage
 	LicenseFileNames []string
 	LicenseTypes     []licenses.AcceptedLicenseInfo
@@ -444,7 +441,7 @@ type licensePolicyPage struct {
 func (s *Server) licensePolicyHandler() http.HandlerFunc {
 	lics := licenses.AcceptedLicenses()
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		page := licensePolicyPage{
+		page := LicensePolicyPage{
 			BasePage:         s.newBasePage(r, "License Policy"),
 			LicenseFileNames: licenses.FileNames,
 			LicenseTypes:     lics,
@@ -645,7 +642,7 @@ func (s *Server) findTemplate(templateName string) (*template.Template, error) {
 		s.mu.Lock()
 		defer s.mu.Unlock()
 		var err error
-		s.templates, err = parsePageTemplates(s.templateFS)
+		s.templates, err = templates.ParsePageTemplates(s.templateFS)
 		if err != nil {
 			return nil, fmt.Errorf("error parsing templates: %v", err)
 		}
@@ -666,82 +663,6 @@ func executeTemplate(ctx context.Context, templateName string, tmpl *template.Te
 	return buf.Bytes(), nil
 }
 
-var templateFuncs = template.FuncMap{
-	"add":      func(i, j int) int { return i + j },
-	"subtract": func(i, j int) int { return i - j },
-	"pluralize": func(i int, s string) string {
-		if i == 1 {
-			return s
-		}
-		return s + "s"
-	},
-	"commaseparate": func(s []string) string {
-		return strings.Join(s, ", ")
-	},
-	"stripscheme": stripScheme,
-	"capitalize":  cases.Title(language.Und).String,
-	"queryescape": url.QueryEscape,
-}
-
-func stripScheme(url string) string {
-	if i := strings.Index(url, "://"); i > 0 {
-		return url[i+len("://"):]
-	}
-	return url
-}
-
-// parsePageTemplates parses html templates contained in the given filesystem in
-// order to generate a map of Name->*template.Template.
-//
-// Separate templates are used so that certain contextual functions (e.g.
-// templateName) can be bound independently for each page.
-//
-// Templates in directories prefixed with an underscore are considered helper
-// templates and parsed together with the files in each base directory.
-func parsePageTemplates(fsys template.TrustedFS) (map[string]*template.Template, error) {
-	templates := make(map[string]*template.Template)
-	htmlSets := [][]string{
-		{"about"},
-		{"badge"},
-		{"error"},
-		{"fetch"},
-		{"homepage"},
-		{"license-policy"},
-		{"search"},
-		{"search-help"},
-		{"styleguide"},
-		{"subrepo"},
-		{"unit/importedby", "unit"},
-		{"unit/imports", "unit"},
-		{"unit/licenses", "unit"},
-		{"unit/main", "unit"},
-		{"unit/versions", "unit"},
-		{"vuln"},
-		{"vuln/main", "vuln"},
-		{"vuln/list", "vuln"},
-		{"vuln/entry", "vuln"},
-	}
-
-	for _, set := range htmlSets {
-		t, err := template.New("frontend.tmpl").Funcs(templateFuncs).ParseFS(fsys, "frontend/*.tmpl")
-		if err != nil {
-			return nil, fmt.Errorf("ParseFS: %v", err)
-		}
-		helperGlob := "shared/*/*.tmpl"
-		if _, err := t.ParseFS(fsys, helperGlob); err != nil {
-			return nil, fmt.Errorf("ParseFS(%q): %v", helperGlob, err)
-		}
-		for _, f := range set {
-			if _, err := t.ParseFS(fsys, path.Join("frontend", f, "*.tmpl")); err != nil {
-				return nil, fmt.Errorf("ParseFS(%v): %v", f, err)
-			}
-		}
-		templates[set[0]] = t
-	}
-
-	return templates, nil
-}
-
 func (s *Server) staticHandler() http.Handler {
 	return http.StripPrefix("/static/", http.FileServer(http.FS(s.staticFS)))
 }

internal/frontend/templates/templates.go

@@ -0,0 +1,92 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package templates
+
+import (
+	"fmt"
+	"net/url"
+	"path"
+	"strings"
+
+	"github.com/google/safehtml/template"
+	"golang.org/x/text/cases"
+	"golang.org/x/text/language"
+)
+
+var templateFuncs = template.FuncMap{
+	"add":      func(i, j int) int { return i + j },
+	"subtract": func(i, j int) int { return i - j },
+	"pluralize": func(i int, s string) string {
+		if i == 1 {
+			return s
+		}
+		return s + "s"
+	},
+	"commaseparate": func(s []string) string {
+		return strings.Join(s, ", ")
+	},
+	"stripscheme": stripScheme,
+	"capitalize":  cases.Title(language.Und).String,
+	"queryescape": url.QueryEscape,
+}
+
+func stripScheme(url string) string {
+	if i := strings.Index(url, "://"); i > 0 {
+		return url[i+len("://"):]
+	}
+	return url
+}
+
+// ParsePageTemplates parses html templates contained in the given filesystem in
+// order to generate a map of Name->*template.Template.
+//
+// Separate templates are used so that certain contextual functions (e.g.
+// templateName) can be bound independently for each page.
+//
+// Templates in directories prefixed with an underscore are considered helper
+// templates and parsed together with the files in each base directory.
+func ParsePageTemplates(fsys template.TrustedFS) (map[string]*template.Template, error) {
+	templates := make(map[string]*template.Template)
+	htmlSets := [][]string{
+		{"about"},
+		{"badge"},
+		{"error"},
+		{"fetch"},
+		{"homepage"},
+		{"license-policy"},
+		{"search"},
+		{"search-help"},
+		{"styleguide"},
+		{"subrepo"},
+		{"unit/importedby", "unit"},
+		{"unit/imports", "unit"},
+		{"unit/licenses", "unit"},
+		{"unit/main", "unit"},
+		{"unit/versions", "unit"},
+		{"vuln"},
+		{"vuln/main", "vuln"},
+		{"vuln/list", "vuln"},
+		{"vuln/entry", "vuln"},
+	}
+
+	for _, set := range htmlSets {
+		t, err := template.New("frontend.tmpl").Funcs(templateFuncs).ParseFS(fsys, "frontend/*.tmpl")
+		if err != nil {
+			return nil, fmt.Errorf("ParseFS: %v", err)
+		}
+		helperGlob := "shared/*/*.tmpl"
+		if _, err := t.ParseFS(fsys, helperGlob); err != nil {
+			return nil, fmt.Errorf("ParseFS(%q): %v", helperGlob, err)
+		}
+		for _, f := range set {
+			if _, err := t.ParseFS(fsys, path.Join("frontend", f, "*.tmpl")); err != nil {
+				return nil, fmt.Errorf("ParseFS(%v): %v", f, err)
+			}
+		}
+		templates[set[0]] = t
+	}
+
+	return templates, nil
+}

internal/frontend/templates/templates_test.go

@@ -0,0 +1,25 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package templates
+
+import (
+	"testing"
+)
+
+func TestStripScheme(t *testing.T) {
+	for _, test := range []struct {
+		url, want string
+	}{
+		{"http://github.com", "github.com"},
+		{"https://github.com/path/to/something", "github.com/path/to/something"},
+		{"example.com", "example.com"},
+		{"chrome-extension://abcd", "abcd"},
+		{"nonwellformed.com/path?://query=1", "query=1"},
+	} {
+		if got := stripScheme(test.url); got != test.want {
+			t.Errorf("%q: got %q, want %q", test.url, got, test.want)
+		}
+	}
+}