Skip to content

Commit

Permalink
internal: move templatecheck tests to their own package
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
matloob committed Dec 19, 2023
1 parent b2a01f8 commit c63424d
Show file tree
Hide file tree
Showing 10 changed files with 226 additions and 184 deletions.
4 changes: 2 additions & 2 deletions internal/frontend/badge.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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",
Expand Down
81 changes: 0 additions & 81 deletions internal/frontend/frontend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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("."))
Expand Down
4 changes: 2 additions & 2 deletions internal/frontend/homepage.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)),
Expand Down
91 changes: 6 additions & 85 deletions internal/frontend/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@ import (
"io/fs"
"net/http"
hpprof "net/http/pprof"
"net/url"
"os"
"path"
"strings"
"sync"
"time"
Expand All @@ -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"
Expand All @@ -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.
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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)
}
Expand All @@ -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)))
}
Expand Down
92 changes: 92 additions & 0 deletions internal/frontend/templates/templates.go
Original file line number Diff line number Diff line change
@@ -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
}
25 changes: 25 additions & 0 deletions internal/frontend/templates/templates_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
}
Loading

0 comments on commit c63424d

Please sign in to comment.