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 <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
kokoro-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
diff --git a/internal/frontend/badge.go b/internal/frontend/badge.go
index c5ee393..6e94f56 100644
--- a/internal/frontend/badge.go
+++ b/internal/frontend/badge.go
@@ -11,7 +11,7 @@
"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 @@
path = path[urlSchemeIdx+3:]
}
- page := badgePage{
+ page := BadgePage{
BasePage: s.newBasePage(r, "Badge"),
LinkPath: path,
BadgePath: "badge/" + path + ".svg",
diff --git a/internal/frontend/frontend_test.go b/internal/frontend/frontend_test.go
index 2fe6426..0533ed1 100644
--- a/internal/frontend/frontend_test.go
+++ b/internal/frontend/frontend_test.go
@@ -14,10 +14,7 @@
"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 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("."))
diff --git a/internal/frontend/homepage.go b/internal/frontend/homepage.go
index 75e6e92..1ec5a72 100644
--- a/internal/frontend/homepage.go
+++ b/internal/frontend/homepage.go
@@ -39,7 +39,7 @@
}
// 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 @@
}
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)),
diff --git a/internal/frontend/server.go b/internal/frontend/server.go
index b68d815..e0183e4 100644
--- a/internal/frontend/server.go
+++ b/internal/frontend/server.go
@@ -14,9 +14,7 @@
"io/fs"
"net/http"
hpprof "net/http/pprof"
- "net/url"
"os"
- "path"
"strings"
"sync"
"time"
@@ -28,6 +26,7 @@
"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 @@
"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 @@
// 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 @@
}
}
-// 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 @@
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 @@
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 @@
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)))
}
diff --git a/internal/frontend/templates/templates.go b/internal/frontend/templates/templates.go
new file mode 100644
index 0000000..3787460
--- /dev/null
+++ b/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
+}
diff --git a/internal/frontend/templates/templates_test.go b/internal/frontend/templates/templates_test.go
new file mode 100644
index 0000000..7bd2d9d
--- /dev/null
+++ b/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)
+ }
+ }
+}
diff --git a/internal/godoc/dochtml/dochtml.go b/internal/godoc/dochtml/dochtml.go
index 95348bb..74870da 100644
--- a/internal/godoc/dochtml/dochtml.go
+++ b/internal/godoc/dochtml/dochtml.go
@@ -66,8 +66,8 @@
BuildContext internal.BuildContext
}
-// templateData holds the data passed to the HTML templates in this package.
-type templateData struct {
+// TemplateData holds the data passed to the HTML templates in this package.
+type TemplateData struct {
RootURL string
Package *doc.Package
Consts, Vars, Funcs, Types []*item
@@ -226,7 +226,7 @@
}
// renderInfo returns the functions and data needed to render the doc.
-func renderInfo(ctx context.Context, fset *token.FileSet, p *doc.Package, opt RenderOptions) (map[string]any, templateData, func() []render.Link) {
+func renderInfo(ctx context.Context, fset *token.FileSet, p *doc.Package, opt RenderOptions) (map[string]any, TemplateData, func() []render.Link) {
// Make a copy to avoid modifying caller's *doc.Package.
p2 := *p
p = &p2
@@ -288,7 +288,7 @@
"since_version": sinceVersion,
}
examples := collectExamples(p)
- data := templateData{
+ data := TemplateData{
Package: p,
RootURL: "/pkg",
Examples: examples,
diff --git a/internal/godoc/dochtml/dochtml_test.go b/internal/godoc/dochtml/dochtml_test.go
index 9b443b4..617f4f7 100644
--- a/internal/godoc/dochtml/dochtml_test.go
+++ b/internal/godoc/dochtml/dochtml_test.go
@@ -23,7 +23,6 @@
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/google/safehtml/template"
- "github.com/jba/templatecheck"
"golang.org/x/net/html"
"golang.org/x/pkgsite/internal/godoc/dochtml/internal/render"
"golang.org/x/pkgsite/internal/testing/testhelper"
@@ -44,15 +43,6 @@
},
}
-func TestCheckTemplates(t *testing.T) {
- LoadTemplates(templateFS)
- for _, tm := range []*template.Template{bodyTemplate, outlineTemplate, sidenavTemplate} {
- if err := templatecheck.CheckSafe(tm, templateData{}); err != nil {
- t.Fatal(err)
- }
- }
-}
-
func TestRender(t *testing.T) {
ctx := context.Background()
LoadTemplates(templateFS)
diff --git a/internal/godoc/dochtml/template.go b/internal/godoc/dochtml/template.go
index 9e81ff3..0e5650e 100644
--- a/internal/godoc/dochtml/template.go
+++ b/internal/godoc/dochtml/template.go
@@ -23,6 +23,10 @@
bodyTemplate, outlineTemplate, sidenavTemplate *template.Template
)
+func Templates() []*template.Template {
+ return []*template.Template{bodyTemplate, outlineTemplate, sidenavTemplate}
+}
+
// LoadTemplates reads and parses the templates used to generate documentation.
func LoadTemplates(fsys template.TrustedFS) {
const dir = "doc"
diff --git a/internal/tests/templates/templates_test.go b/internal/tests/templates/templates_test.go
new file mode 100644
index 0000000..30e43a7
--- /dev/null
+++ b/internal/tests/templates/templates_test.go
@@ -0,0 +1,91 @@
+// 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"
+
+ "github.com/google/safehtml/template"
+ "github.com/jba/templatecheck"
+ "golang.org/x/pkgsite/internal/frontend"
+ "golang.org/x/pkgsite/internal/frontend/page"
+ "golang.org/x/pkgsite/internal/frontend/templates"
+ "golang.org/x/pkgsite/internal/frontend/versions"
+ "golang.org/x/pkgsite/internal/godoc/dochtml"
+ "golang.org/x/pkgsite/static"
+)
+
+func TestCheckFrontendTemplates(t *testing.T) {
+ // Perform additional checks on parsed templates.
+ staticFS := template.TrustedFSFromEmbed(static.FS)
+ templates, err := templates.ParsePageTemplates(staticFS)
+ if err != nil {
+ t.Fatal(err)
+ }
+ for _, c := range []struct {
+ name string
+ subs []string
+ typeval any
+ }{
+ {"badge", nil, frontend.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, frontend.Homepage{}},
+ {"license-policy", nil, frontend.LicensePolicyPage{}},
+ {"search", nil, frontend.SearchPage{}},
+ {"search-help", nil, page.BasePage{}},
+ {"unit/main", nil, frontend.UnitPage{}},
+ {
+ "unit/main",
+ []string{"unit-outline", "unit-readme", "unit-doc", "unit-files", "unit-directories"},
+ frontend.MainDetails{},
+ },
+ {"unit/importedby", nil, frontend.UnitPage{}},
+ {"unit/importedby", []string{"importedby"}, frontend.ImportedByDetails{}},
+ {"unit/imports", nil, frontend.UnitPage{}},
+ {"unit/imports", []string{"imports"}, frontend.ImportsDetails{}},
+ {"unit/licenses", nil, frontend.UnitPage{}},
+ {"unit/licenses", []string{"licenses"}, frontend.LicensesDetails{}},
+ {"unit/versions", nil, frontend.UnitPage{}},
+ {"unit/versions", []string{"versions"}, versions.VersionsDetails{}},
+ {"vuln", nil, page.BasePage{}},
+ {"vuln/list", nil, frontend.VulnListPage{}},
+ {"vuln/entry", nil, frontend.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)
+ }
+ }
+ }
+ })
+ }
+}
+
+var templateFS = template.TrustedFSFromTrustedSource(template.TrustedSourceFromConstant("../../../static"))
+
+func TestCheckDocHTMLTemplates(t *testing.T) {
+ dochtml.LoadTemplates(templateFS)
+ for _, tm := range dochtml.Templates() {
+ if err := templatecheck.CheckSafe(tm, dochtml.TemplateData{}); err != nil {
+ t.Fatal(err)
+ }
+ }
+}