internal/godoc/dochtml: stop adding /pkg prefix to links
This prefix is removed when reading from the DB, but not after a
frontend render. I think we can just stop adding it.
Caught by an improved integration test which now diffs the worker and
frontend docs, also included.
Change-Id: I835f18ca5aa602700ad6d3ce34bc57e56c60f2d1
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/259101
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Julie Qiu <julie@golang.org>
diff --git a/internal/fetch/fetchdata_test.go b/internal/fetch/fetchdata_test.go
index 1651cdb..9ee671c 100644
--- a/internal/fetch/fetchdata_test.go
+++ b/internal/fetch/fetchdata_test.go
@@ -239,7 +239,7 @@
},
Documentation: &internal.Documentation{
Synopsis: "Package good is inside a module that has bad packages.",
- HTML: html(`const Good = <a href="/pkg/builtin#true">true</a>`),
+ HTML: html(`const Good = <a href="/builtin#true">true</a>`),
},
},
},
diff --git a/internal/godoc/dochtml/dochtml.go b/internal/godoc/dochtml/dochtml.go
index 01cb580..2967289 100644
--- a/internal/godoc/dochtml/dochtml.go
+++ b/internal/godoc/dochtml/dochtml.go
@@ -18,7 +18,6 @@
"go/ast"
"go/printer"
"go/token"
- pathpkg "path"
"sort"
"strings"
@@ -106,7 +105,7 @@
if opt.ModInfo != nil {
versionedPath = versionedPkgPath(path, opt.ModInfo)
}
- return pathpkg.Join("/pkg", versionedPath)
+ return "/" + versionedPath
},
DisableHotlinking: true,
})
diff --git a/internal/postgres/details.go b/internal/postgres/details.go
index da5b1a7..7250bc4 100644
--- a/internal/postgres/details.go
+++ b/internal/postgres/details.go
@@ -201,6 +201,9 @@
// TestRemovePkgPrefix for examples. It preserves the safety of its argument.
// That is, if docHTML is safe from XSS attacks, so is
// removePkgPrefix(docHTML).
+//
+// Although we don't add "/pkg" to links after https://golang.org/cl/259101,
+// do not remove this function until all databases have been reprocessed.
func removePkgPrefix(docHTML string) string {
return packageLinkRegexp.ReplaceAllString(docHTML, `$1$2$3`)
}
diff --git a/internal/testing/integration/frontend_doc_render_test.go b/internal/testing/integration/frontend_doc_render_test.go
index 502a3c7..3c53e2e 100644
--- a/internal/testing/integration/frontend_doc_render_test.go
+++ b/internal/testing/integration/frontend_doc_render_test.go
@@ -6,14 +6,15 @@
import (
"context"
+ "io/ioutil"
"net/http"
"testing"
+ "github.com/google/go-cmp/cmp"
"golang.org/x/pkgsite/internal"
"golang.org/x/pkgsite/internal/experiment"
"golang.org/x/pkgsite/internal/postgres"
"golang.org/x/pkgsite/internal/proxy"
- hc "golang.org/x/pkgsite/internal/testing/htmlcheck"
"golang.org/x/pkgsite/internal/testing/testhelper"
)
@@ -22,10 +23,6 @@
func TestFrontendDocRender(t *testing.T) {
defer postgres.ResetTestDB(testDB, t)
- ctx := experiment.NewContext(context.Background(),
- internal.ExperimentRemoveUnusedAST,
- internal.ExperimentInsertPackageSource,
- internal.ExperimentFrontendRenderDoc)
m := &proxy.Module{
ModulePath: "github.com/golang/fdoc",
Version: "v1.2.3",
@@ -36,32 +33,64 @@
// Package fdoc is a test of frontend doc rendering.
package fdoc
+ import "time"
+
// C is a constant.
const C = 123
// F is a function.
- func F() {
- // lots of code
- print(1, 2, 3)
+ func F(t time.Time, s string) T {
_ = C
- // more
}
`,
+ "file2.go": `
+ package fdoc
+
+ var V = C
+ type T int
+ `,
},
}
- ts := setupFrontend(ctx, t, nil)
+ commonExps := []string{
+ internal.ExperimentRemoveUnusedAST,
+ internal.ExperimentInsertPackageSource,
+ internal.ExperimentSidenav,
+ internal.ExperimentUnitPage,
+ }
+
+ ctx := experiment.NewContext(context.Background(), commonExps...)
processVersions(ctx, t, []*proxy.Module{m})
- validateResponse(t, http.MethodGet, ts.URL+"/"+m.ModulePath, 200,
- hc.In(".Documentation-content",
- hc.In(".Documentation-overview", hc.HasText("Package fdoc is a test of frontend doc rendering.")),
- hc.In(".Documentation-constants", hc.HasText("C is a constant.")),
- hc.In(".Documentation-functions",
- hc.In("a",
- hc.HasHref("https://github.com/golang/fdoc/blob/v1.2.3/file.go#L9"),
- hc.HasText("F")),
- hc.In("pre", hc.HasExactText("func F()")),
- hc.In("p", hc.HasText("F is a function.")))))
+ getDoc := func(exps ...string) string {
+ t.Helper()
+ ectx := experiment.NewContext(ctx, append(exps, commonExps...)...)
+ ts := setupFrontend(ectx, t, nil)
+ url := ts.URL + "/" + m.ModulePath
+ return getPage(ectx, t, url)
+ }
+
+ workerDoc := getDoc()
+ frontendDoc := getDoc(internal.ExperimentFrontendRenderDoc)
+
+ if diff := cmp.Diff(workerDoc, frontendDoc); diff != "" {
+ t.Errorf("mismatch (-worker, +frontend):\n%s", diff)
+ }
+}
+
+func getPage(ctx context.Context, t *testing.T, url string) string {
+ resp, err := http.Get(url)
+ if err != nil {
+ t.Fatalf("%s: %v", url, err)
+ }
+ defer resp.Body.Close()
+ if resp.StatusCode != http.StatusOK {
+ t.Fatalf("%s: status code %d", url, resp.StatusCode)
+ }
+ content, err := ioutil.ReadAll(resp.Body)
+ if err != nil {
+ t.Fatalf("%s: %v", url, err)
+ }
+ return string(content)
}