amadan

oficina / sheets Public

Test the shared chrome: errors, bar, account, titles, menus, sign-in

Each was seen failing under a mutation of the code it covers
(polish-audit/scripts/sheets-r2-mutations.py): 27 tests, all red.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Paul Campbell pushed by paul@keymail.dev 523a877bacea4014966f660aa186b60d5ad7c9b0 parent 76d4e48
5 files changed, +753 −1
  • internal/sheets/chrome_menu_test.go +204 −0
  • internal/sheets/chrome_signin_test.go +105 −0
  • internal/sheets/chrome_test.go +187 −0
  • internal/sheets/errors_test.go +249 −0
  • internal/sheets/home_test.go +8 −1

CI failed — run details

CI log 
amadan: sync for CI failed: exit status 128: error: RPC failed; HTTP 429 curl 22 The requested URL returned error: 429
fatal: expected flush after ref listing
 (and could not reset the workspace: unlinkat /home/paulca/.local/state/amadan/runner/repos/oficina__sheets/cache/go-mod/modernc.org/libc@v1.74.1/ioutil_darwin.go: permission denied)
diff --git a/internal/sheets/chrome_menu_test.go b/internal/sheets/chrome_menu_test.go
new file mode 100644
index 0000000..1574919
--- /dev/null
+++ b/internal/sheets/chrome_menu_test.go
@@ -0,0 +1,204 @@
+package sheets
+
+import (
+ "html"
+ "net/http"
+ "net/http/httptest"
+ "net/url"
+ "os"
+ "path/filepath"
+ "regexp"
+ "strconv"
+ "strings"
+ "testing"
+)
+
+// §5.4: the rename is the bar's editable title — a real form posting to
+// the rename route, only for someone who may write. A reader sees the
+// title read-only.
+func TestTheBarRenamesTheSheet(t *testing.T) {
+ app, cookies := signIn(t)
+ s, _ := newSheet(t, app, cookies, "Budget")
+ body := page(t, app, cookies, "/sheets/"+strconv.FormatUint(s.ID, 10)).Body.String()
+ for _, want := range []string{
+ `<form estilo-bar-title method="post" action="/sheets/` + strconv.FormatUint(s.ID, 10) + `/rename">`,
+ `<input estilo-bar-title-input name="title" value="Budget" aria-label="Sheet title"`,
+ `<button rst-btn="ghost" type="submit" estilo-bar-title-submit>Rename</button>`,
+ } {
+ if !strings.Contains(body, want) {
+ t.Errorf("the writer's bar lacks %s", want)
+ }
+ }
+ if strings.Contains(body, "data-doc-title") {
+ t.Error("the old in-bar rename form is still rendered")
+ }
+ // A reader: a viewer grant only, signed in through the real link.
+ if _, err := app.grant(s.ID, "reader@example.com", SheetViewer, signedInMember(t, app)); err != nil {
+ t.Fatal(err)
+ }
+ postSignin(t, app, "reader@example.com")
+ u, err := url.Parse(app.testMailer(t).lastLink(t))
+ if err != nil {
+ t.Fatal(err)
+ }
+ reader := do(t, app, httptest.NewRequest(http.MethodGet, u.RequestURI(), nil)).Result().Cookies()
+ body = page(t, app, reader, "/sheets/"+strconv.FormatUint(s.ID, 10)).Body.String()
+ if strings.Contains(body, "estilo-bar-title") || !strings.Contains(body, `<span estilo-bar-context dir="auto" title="Budget">Budget</span>`) {
+ t.Error("a reader is offered the rename, or does not see the title")
+ }
+}
+
+// The rename works through the bar with scripting off: Enter posts the
+// form, and the sheet comes back renamed.
+func TestTheBarsRenameFormRenames(t *testing.T) {
+ app, cookies := signIn(t)
+ s, _ := newSheet(t, app, cookies, "Budget")
+ rec := do(t, app, as(postForm("/sheets/"+strconv.FormatUint(s.ID, 10)+"/rename", map[string][]string{"title": {"Commission Q2"}}), cookies))
+ if rec.Code != http.StatusSeeOther {
+ t.Fatalf("rename = %d", rec.Code)
+ }
+ if body := page(t, app, cookies, "/sheets/"+strconv.FormatUint(s.ID, 10)).Body.String(); !strings.Contains(body, `value="Commission Q2"`) || titleOf(t, body) != "Commission Q2 · Sheets" {
+ t.Error("the bar does not show the new title")
+ }
+}
+
+// §0.5 and §12.1 item 2: <meta charset> first, then the stylesheets in
+// the suite's order — tokens, theme, brand, estilo base, menu, chrome,
+// then Sheets' own.
+func TestEstiloChromeAssetsAreLinkedInOrder(t *testing.T) {
+ body := page(t, New(Config{}), nil, "/signin").Body.String()
+ head := body[strings.Index(body, "<head>"):strings.Index(body, "</head>")]
+ if !strings.HasPrefix(strings.TrimSpace(strings.TrimPrefix(head, "<head>")), `<meta charset="utf-8">`) {
+ t.Error("<meta charset> is not the first thing in <head>")
+ }
+ last := -1
+ for _, css := range []string{"/static/tokens.", "/static/theme.", "/static/brand/brand.", "/static/estilo/base.", "/static/estilo/menu.", "/static/estilo/chrome.", "/static/sheets."} {
+ i := strings.Index(head, css)
+ if i < 0 || i < last {
+ t.Fatalf("%s is missing or out of order", css)
+ }
+ last = i
+ }
+ for _, asset := range []string{"/static/estilo/base.", "/static/estilo/chrome."} {
+ href := regexp.MustCompile(regexp.QuoteMeta(asset) + `[0-9a-f]+\.(css|js)`).FindString(head)
+ if href == "" {
+ t.Fatalf("%s is not fingerprinted", asset)
+ }
+ if rec := do(t, New(Config{}), httptest.NewRequest(http.MethodGet, href, nil)); rec.Code != http.StatusOK || rec.Body.Len() == 0 {
+ t.Errorf("%s does not serve (%d)", href, rec.Code)
+ }
+ }
+ if rec := do(t, New(Config{}), httptest.NewRequest(http.MethodGet, "/static/estilo/brand/sheets.svg", nil)); rec.Code != http.StatusOK || rec.Header().Get("Content-Type") != "image/svg+xml" {
+ t.Errorf("the bar's brand icon does not serve (%d)", rec.Code)
+ }
+}
+
+// §7.3: shortcuts are labelled for the request's platform, server-side,
+// in the menus, the toolbar's tips and the shortcuts dialog alike — and
+// the page says it varies by User-Agent.
+func TestShortcutLabelsFollowThePlatform(t *testing.T) {
+ app, cookies := signIn(t)
+ s, _ := newSheet(t, app, cookies, "Budget")
+ path := "/sheets/" + strconv.FormatUint(s.ID, 10)
+ for ua, want := range map[string][]string{
+ "Mozilla/5.0 (Macintosh; Intel Mac OS X 14_0) AppleWebKit/537.36": {`<kbd estilo-menu-key>⌘Z</kbd>`, `data-tip="Undo (⌘Z)"`, `data-tip="Bold (⌘B)"`, `<dt><kbd>⌘C</kbd></dt>`, `data-key-copy="⌘C"`},
+ "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36": {`<kbd estilo-menu-key>Ctrl+Z</kbd>`, `data-tip="Undo (Ctrl+Z)"`, `data-tip="Bold (Ctrl+B)"`, `<dt><kbd>Ctrl+C</kbd></dt>`, `data-key-copy="Ctrl+C"`},
+ } {
+ req := httptest.NewRequest(http.MethodGet, path, nil)
+ req.Header.Set("User-Agent", ua)
+ rec := do(t, app, as(req, cookies))
+ // html/template writes "+" as &#43;; the browser reads it back.
+ body := html.UnescapeString(rec.Body.String())
+ for _, w := range want {
+ if !strings.Contains(body, w) {
+ t.Errorf("%s: missing %s", ua[13:30], w)
+ }
+ }
+ for _, stale := range []string{"Ctrl Z", "(Ctrl+Z)</", "Ctrl + C"} {
+ if strings.Contains(body, stale) {
+ t.Errorf("%s: an old label %q survives", ua[13:30], stale)
+ }
+ }
+ if !strings.Contains(strings.Join(rec.Header().Values("Vary"), ","), "User-Agent") {
+ t.Error("a page with platform labels does not Vary on User-Agent")
+ }
+ }
+}
+
+// §7.3: an item appears only if it works in this build. The Help menu
+// (a list of keys, not a menu) left the menu bar for the account menu's
+// "Keyboard shortcuts…" dialog; Format keeps no inline submenus; items
+// that need script are hidden until it runs.
+func TestUnbuiltMenuItemsAreHidden(t *testing.T) {
+ app, cookies := signIn(t)
+ s, _ := newSheet(t, app, cookies, "Budget")
+ body := page(t, app, cookies, "/sheets/"+strconv.FormatUint(s.ID, 10)).Body.String()
+ menubar := body[strings.Index(body, "<nav estilo-menubar"):]
+ menubar = menubar[:strings.Index(menubar, "</nav>")]
+ titles := regexp.MustCompile(`<summary estilo-menubar-title>([^<]+)</summary>`).FindAllStringSubmatch(menubar, -1)
+ var got []string
+ for _, m := range titles {
+ got = append(got, m[1])
+ }
+ if strings.Join(got, ",") != "File,Edit,Insert,Format,Data" {
+ t.Errorf("menu bar titles %v, want File, Edit, Insert, Format, Data", got)
+ }
+ if strings.Count(menubar, "<details") != len(titles) {
+ t.Error("a menu holds a submenu; menus here are one level")
+ }
+ for _, gone := range []string{"rst-dropdown", "rst-menu-group", "Fill colour", "Number format"} {
+ if strings.Contains(menubar, gone) {
+ t.Errorf("the menu bar still carries %q", gone)
+ }
+ }
+ for _, cmd := range regexp.MustCompile(`<button estilo-menu-item type="button" data-cmd="(cut|copy|paste|clear|fill-down|fill-right|select-all|fn)"[^>]*>`).FindAllString(menubar, -1) {
+ if !strings.Contains(cmd, " hidden") {
+ t.Errorf("%s is shown before the script that runs it", cmd)
+ }
+ }
+ // The shortcuts moved to the account menu, as a dialog that exists.
+ if !strings.Contains(body, `<button estilo-menu-item type="button" commandfor="sheets-shortcuts" command="show-modal"><span estilo-menu-label>Keyboard shortcuts…</span></button>`) {
+ t.Error("the account menu's Help does not open the shortcuts")
+ }
+ if !strings.Contains(body, `<dialog id="sheets-shortcuts"`) {
+ t.Error("the shortcuts dialog the account menu opens is not on the page")
+ }
+}
+
+// §4: the only notice vocabulary is rastrillo's callout with a real tone.
+// The grep gate: no rst-notice, no valued rst-callout="…" (the old
+// "warn" matched nothing and painted grey), no rst-tone="warn".
+func TestTemplatesUseOnlyCalloutTones(t *testing.T) {
+ files, err := filepath.Glob("templates/*.html")
+ if err != nil || len(files) == 0 {
+ t.Fatal("no templates found", err)
+ }
+ strip := regexp.MustCompile(`(?s)\{\{/\*.*?\*/\}\}`)
+ bad := regexp.MustCompile(`rst-notice|rst-callout="|rst-tone="warn"`)
+ for _, f := range files {
+ body, err := os.ReadFile(f)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if m := bad.FindString(strip.ReplaceAllString(string(body), "")); m != "" {
+ t.Errorf("%s uses %q", f, m)
+ }
+ }
+ // And the tones reach the page: a created link's warning, a grant's
+ // positive notice, a refused grant's negative alert.
+ app, cookies := signIn(t)
+ s, _ := newSheet(t, app, cookies, "Budget")
+ share := "/sheets/" + strconv.FormatUint(s.ID, 10) + "/share"
+ for query, want := range map[string]string{
+ "?notice=shared": `rst-tone="positive"`,
+ "?notice=bad-role": `rst-tone="negative" role="alert"`,
+ } {
+ if body := page(t, app, cookies, share+query).Body.String(); !strings.Contains(body, want) {
+ t.Errorf("%s: no %s callout", query, want)
+ }
+ }
+ rec := do(t, app, as(postForm(share+"/link", map[string][]string{"role": {"viewer"}}), cookies))
+ if body := rec.Body.String(); !strings.Contains(body, `<div rst-callout rst-tone="warning">`) || strings.Count(body, "Copy it now") != 1 {
+ t.Errorf("a new link is not one warning callout (status %d)", rec.Code)
+ }
+}
diff --git a/internal/sheets/chrome_signin_test.go b/internal/sheets/chrome_signin_test.go
new file mode 100644
index 0000000..287df87
--- /dev/null
+++ b/internal/sheets/chrome_signin_test.go
@@ -0,0 +1,105 @@
+package sheets
+
+import (
+ "net/http"
+ "strings"
+ "testing"
+)
+
+// §8.2 item 2: "Connect your Oficina account" is offered with Home
+// configured, to a local session, and never to a Home session, whose
+// account is already the connection. Sheets used to offer it to both.
+func TestConnectYourOficinaAccountIsHiddenForHomeSessions(t *testing.T) {
+ f := newHomeFixture(t)
+ f.local(t, signedInAddress)
+ if body := f.request(t, "GET", "/sheets", nil, false).Body.String(); !strings.Contains(body, `<a estilo-menu-item href="/account/home"><span estilo-menu-label>Connect your Oficina account</span></a>`) {
+ t.Error("a local session is not offered the connection")
+ }
+ g := newHomeFixture(t)
+ homeSignedIn(t, g)
+ body := g.request(t, "GET", "/sheets", nil, false).Body.String()
+ if !strings.Contains(body, "<details estilo-account") {
+ t.Fatal("the Home session is not signed in")
+ }
+ if strings.Contains(body, "/account/home") || strings.Contains(body, "Home account") {
+ t.Error("a Home session is offered to connect the account it signed in with")
+ }
+ app, cookies := signIn(t)
+ if body := page(t, app, cookies, "/sheets").Body.String(); strings.Contains(body, "/account/home") {
+ t.Error("a standalone instance offers a connection to a Home it does not have")
+ }
+}
+
+// Home #74, decision 2: with Home configured, a signed-out "/" starts
+// sign-in instead of showing a landing page — the switcher's single-team
+// hop lands on the root. Signed in, it is the list, both ways.
+func TestTheRootStartsSignInWhenHomeIsConfigured(t *testing.T) {
+ f := newHomeFixture(t)
+ if rec := f.request(t, "GET", "/", nil, false); rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/signin" {
+ t.Fatalf("signed-out / with Home = %d %q, want 303 /signin", rec.Code, rec.Header().Get("Location"))
+ }
+ homeSignedIn(t, f)
+ if rec := f.request(t, "GET", "/", nil, false); rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/sheets" {
+ t.Fatalf("signed-in / with Home = %d %q, want 303 /sheets", rec.Code, rec.Header().Get("Location"))
+ }
+ g := newHomeFixture(t)
+ g.local(t, signedInAddress)
+ if rec := g.request(t, "GET", "/", nil, false); rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/sheets" {
+ t.Fatalf("locally signed-in / with Home = %d %q, want 303 /sheets", rec.Code, rec.Header().Get("Location"))
+ }
+ // Standalone keeps its landing page for a stranger.
+ if rec := page(t, New(Config{}), nil, "/"); rec.Code != http.StatusOK || !strings.Contains(rec.Body.String(), "<h1>Sheets</h1>") {
+ t.Fatalf("standalone signed-out / = %d, want the landing page", rec.Code)
+ }
+}
+
+// §9.2's last row: somebody already signed in never sees the form. With
+// Home, connect's /signin does it (home/connect 01166fc); standalone,
+// Sheets' own handler does, to a safe return_to or the list, and never
+// to a sign-in page (which would loop).
+func TestSigninWhenAlreadySignedIn(t *testing.T) {
+ app, cookies := signIn(t)
+ for query, want := range map[string]string{
+ "": "/sheets",
+ "?return_to=%2Fsheets%2F4": "/sheets/4",
+ "?return_to=%2Fsignin": "/sheets",
+ "?return_to=%2F%2Fevil.test": "/sheets",
+ } {
+ rec := page(t, app, cookies, "/signin"+query)
+ if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != want {
+ t.Errorf("signed-in /signin%s = %d %q, want 303 %s", query, rec.Code, rec.Header().Get("Location"), want)
+ }
+ }
+ // The outcome pages still render: a keymail failure must be able to
+ // offer the email path to somebody holding a session.
+ if rec := page(t, app, cookies, "/signin?force=1&err=keymail"); rec.Code != http.StatusOK {
+ t.Errorf("?force=1&err=keymail = %d, want the form", rec.Code)
+ }
+ f := newHomeFixture(t)
+ homeSignedIn(t, f)
+ if rec := f.request(t, "GET", "/signin?return_to=%2Fsheets%2F4", nil, false); rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/sheets/4" {
+ t.Errorf("Home session /signin = %d %q, want 303 /sheets/4", rec.Code, rec.Header().Get("Location"))
+ }
+}
+
+// §9.2: Sign out lands on the signed-out page, "You’re signed out", with
+// an empty corner and a way back in — standalone too, where rastrillo's
+// auth sends it to the sign-in form.
+func TestSignOutLandsOnTheSignedOutPage(t *testing.T) {
+ app, cookies := signIn(t)
+ req := as(postForm("/signout", nil), cookies)
+ req.Header.Set("Origin", testOrigin)
+ rec := do(t, app, req)
+ if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/signed-out" {
+ t.Fatalf("sign out = %d %q, want 303 /signed-out", rec.Code, rec.Header().Get("Location"))
+ }
+ if after := page(t, app, cookies, "/sheets"); after.Code == http.StatusOK {
+ t.Fatal("the session survived sign-out")
+ }
+ body := page(t, app, nil, "/signed-out").Body.String()
+ for _, want := range []string{"<h1>You’re signed out</h1>", "You’ve signed out of Sheets on this device.", `<a rst-btn="primary" href="/signin">Sign in</a>`} {
+ if !strings.Contains(body, want) {
+ t.Errorf("the signed-out page lacks %s", want)
+ }
+ }
+}
diff --git a/internal/sheets/chrome_test.go b/internal/sheets/chrome_test.go
new file mode 100644
index 0000000..720aac6
--- /dev/null
+++ b/internal/sheets/chrome_test.go
@@ -0,0 +1,187 @@
+package sheets
+
+// The shared chrome, as Sheets renders it (estilo spec/shared-chrome,
+// §5, §6, §7, §8, §9, §10). Each test names the section it holds.
+
+import (
+ "net/http"
+ "net/http/httptest"
+ "regexp"
+ "strconv"
+ "strings"
+ "testing"
+)
+
+var pageTitle = regexp.MustCompile(`<title>([^<]*)</title>`)
+
+// page GETs path as a browser does, with the session cookies.
+func page(t *testing.T, app *App, cookies []*http.Cookie, path string) *httptest.ResponseRecorder {
+ t.Helper()
+ req := httptest.NewRequest(http.MethodGet, path, nil)
+ req.Header.Set("Accept", "text/html,application/xhtml+xml;q=0.9,*/*;q=0.8")
+ return do(t, app, as(req, cookies))
+}
+
+func titleOf(t *testing.T, body string) string {
+ t.Helper()
+ m := pageTitle.FindStringSubmatch(body)
+ if m == nil {
+ t.Fatal("no <title>")
+ }
+ return m[1]
+}
+
+// homeSignedIn signs the fixture's browser in THROUGH Home: a linked
+// account, then an ordinary sign-in by assertion, so the session's
+// method is Home's.
+func homeSignedIn(t *testing.T, f *homeFixture) Member {
+ t.Helper()
+ member := f.local(t, signedInAddress)
+ f.link(t, "opaque-home-subject")
+ f.jar = map[string]*http.Cookie{}
+ signin := f.request(t, "GET", "/signin?return_to=%2Fsheets", nil, false)
+ start := f.request(t, "GET", signin.Header().Get("Location"), nil, false)
+ if finish := f.exchange(t, start, "opaque-home-subject"); finish.Code != 303 {
+ t.Fatal("Home sign-in", finish.Code)
+ }
+ return member
+}
+
+// §10: every <title> is "{part} · {part} · Sheets", built in the layout,
+// and a page's {Page} is its h1. None says "Oficina Sheets", "—" or "|".
+func TestPageTitlesFollowTheSuitePattern(t *testing.T) {
+ app, cookies := signIn(t)
+ s, _ := newSheet(t, app, cookies, "Commission Q2")
+ id := strconv.FormatUint(s.ID, 10)
+ for path, want := range map[string]string{
+ "/sheets": "Sheets",
+ "/sheets/" + id: "Commission Q2 · Sheets",
+ "/sheets/" + id + "/table": "Table view · Commission Q2 · Sheets",
+ "/sheets/" + id + "/history": "Version history · Commission Q2 · Sheets",
+ "/sheets/" + id + "/comments": "Comments · Commission Q2 · Sheets",
+ "/sheets/" + id + "/share": "Share · Commission Q2 · Sheets",
+ "/sheets/import": "Import a spreadsheet · Sheets",
+ "/sheets/99999": "Sheet not available · Sheets",
+ "/no/such/page": "Page not found · Sheets",
+ } {
+ if got := titleOf(t, page(t, app, cookies, path).Body.String()); got != want {
+ t.Errorf("%s: <title> %q, want %q", path, got, want)
+ }
+ }
+ // A sub-page's h1 is its {Page}; the sheet's title is in the bar.
+ for path, h1 := range map[string]string{
+ "/sheets/" + id + "/table": "Table view",
+ "/sheets/" + id + "/history": "Version history",
+ "/sheets/" + id + "/comments": "Comments",
+ "/sheets/" + id + "/share": "Share",
+ "/sheets": "All sheets",
+ } {
+ body := page(t, app, cookies, path).Body.String()
+ if !strings.Contains(body, "<h1>"+h1+"</h1>") {
+ t.Errorf("%s: the h1 is not %q", path, h1)
+ }
+ if path != "/sheets" && !strings.Contains(body, `<span estilo-bar-context dir="auto" title="Commission Q2">Commission Q2</span>`) {
+ t.Errorf("%s: the sheet's title is not the bar's context", path)
+ }
+ }
+ for path, want := range map[string]string{
+ "/": "Sheets",
+ "/signin": "Sign in · Sheets",
+ "/signin?sent=1": "Check your email · Sheets",
+ "/signed-out": "Signed out · Sheets",
+ "/no/such/page": "Page not found · Sheets",
+ "/signin?err=bad": "Sign in · Sheets",
+ } {
+ if got := titleOf(t, page(t, app, nil, path).Body.String()); got != want {
+ t.Errorf("%s signed out: <title> %q, want %q", path, got, want)
+ }
+ }
+}
+
+// §5.1 / §8.2: on every signed-in page the account menu is the last
+// thing in the bar, and "Sign out" appears exactly once in the page —
+// it used to be the account list's fourth flat row.
+func TestEverySignedInPageHasOneSignOutInTheSameCorner(t *testing.T) {
+ app, cookies := signIn(t)
+ s, _ := newSheet(t, app, cookies, "Budget")
+ id := strconv.FormatUint(s.ID, 10)
+ for _, path := range []string{"/sheets", "/sheets/" + id, "/sheets/" + id + "/table", "/sheets/" + id + "/share", "/sheets/" + id + "/history", "/sheets/" + id + "/comments", "/sheets/import", "/no/such/page", "/sheets/99999"} {
+ body := page(t, app, cookies, path).Body.String()
+ if n := strings.Count(body, ">Sign out<"); n != 1 {
+ t.Errorf("%s: %d Sign out, want exactly one", path, n)
+ }
+ end := body[strings.Index(body, "<div estilo-bar-end>"):strings.Index(body, "</header>")]
+ tail := end[strings.LastIndex(end, "</details>"):]
+ if !strings.Contains(end, "<details estilo-account") || strings.Count(tail, "<") != 2 { // </details> </div>
+ t.Errorf("%s: the account is not the last thing in the bar", path)
+ }
+ if strings.Contains(body, "rst-shell-") {
+ t.Errorf("%s: the rastrillo topbar is still rendered", path)
+ }
+ }
+}
+
+// §5.1 / §8.4: signed out, the corner is "Sign in" carrying the page to
+// come back to; on the sign-in pages themselves it is empty; on the
+// public link page the brand is plain text and Sign in carries no path
+// (the path is a capability).
+func TestTheSignedOutCorner(t *testing.T) {
+ app, cookies := signIn(t)
+ s, _ := newSheet(t, app, cookies, "Budget")
+ token, err := app.createLink(s.ID, SheetViewer, signedInMember(t, app))
+ if err != nil {
+ t.Fatal(err)
+ }
+ body := page(t, app, nil, "/no/such/page?x=1").Body.String()
+ if !strings.Contains(body, `<a rst-btn href="/signin?return_to=%2Fno%2Fsuch%2Fpage%3Fx%3D1" estilo-bar-signin>Sign in</a>`) {
+ t.Error("a signed-out page's corner is not Sign in with its return_to")
+ }
+ for _, path := range []string{"/signin", "/signin?sent=1", "/signed-out"} {
+ body := page(t, app, nil, path).Body.String()
+ if strings.Contains(body, "estilo-bar-signin") || strings.Contains(body, "estilo-account") {
+ t.Errorf("%s: the corner is not empty on a sign-in page", path)
+ }
+ }
+ link := page(t, app, nil, "/l/"+token).Body.String()
+ if !strings.Contains(link, "<span estilo-bar-brand>") || strings.Contains(link, "<a estilo-bar-brand") {
+ t.Error("the public link page's brand is a link a stranger would be bounced through")
+ }
+ if !strings.Contains(link, `<a rst-btn href="/signin" estilo-bar-signin>Sign in</a>`) || strings.Contains(link, "return_to=%2Fl%2F") {
+ t.Error("the link page's Sign in is missing, or copies the capability path into a URL")
+ }
+}
+
+// §6: the switcher shows only when Home names a hub directory, and then
+// holds "All apps" alone (Paul's §13 answer 2) — no tiles yet.
+func TestTheSwitcherIsAllAppsOnly(t *testing.T) {
+ app, cookies := signIn(t)
+ if body := page(t, app, cookies, "/sheets").Body.String(); strings.Contains(body, "estilo-switcher") {
+ t.Error("a standalone instance shows a switcher; there is no directory to switch through")
+ }
+ f := newHomeFixtureWith(t, "https://oficina.example.com")
+ f.local(t, signedInAddress)
+ body := f.request(t, "GET", "/sheets", nil, false).Body.String()
+ for _, want := range []string{`<details estilo-switcher name="estilo-menus">`, `aria-label="Oficina apps"`, `<a estilo-menu-item href="https://oficina.example.com/"><span estilo-menu-label>All apps</span></a>`} {
+ if !strings.Contains(body, want) {
+ t.Errorf("the switcher lacks %s", want)
+ }
+ }
+ if strings.Contains(body, "estilo-switcher-app") {
+ t.Error("the switcher renders app tiles before Home's /apps/<slug> hop is ready")
+ }
+}
+
+// §5.1 / §5.3: the grid is the one page whose shell fills the viewport;
+// every other page's shell is plain, and every page has the bar.
+func TestTheGridFillsTheViewport(t *testing.T) {
+ app, cookies := signIn(t)
+ s, _ := newSheet(t, app, cookies, "Budget")
+ grid := page(t, app, cookies, "/sheets/"+strconv.FormatUint(s.ID, 10)).Body.String()
+ if !strings.Contains(grid, `<div estilo-shell="fill">`) || !strings.Contains(grid, `<header estilo-bar>`) {
+ t.Error("the grid's shell does not fill the viewport, or its bar is sticky in a page that never scrolls")
+ }
+ list := page(t, app, cookies, "/sheets").Body.String()
+ if !strings.Contains(list, "<div estilo-shell>") || !strings.Contains(list, `<header estilo-bar="sticky">`) {
+ t.Error("an ordinary page's shell or bar is wrong")
+ }
+}
diff --git a/internal/sheets/errors_test.go b/internal/sheets/errors_test.go
new file mode 100644
index 0000000..7574922
--- /dev/null
+++ b/internal/sheets/errors_test.go
@@ -0,0 +1,249 @@
+package sheets
+
+import (
+ "fmt"
+ "go/ast"
+ "go/parser"
+ "go/token"
+ "net/http"
+ "net/http/httptest"
+ "path/filepath"
+ "regexp"
+ "sort"
+ "strconv"
+ "strings"
+ "testing"
+)
+
+// Every error a browser can reach is the error page, inside the normal
+// layout and bar, showing the viewer's own chrome (spec §3). The table is
+// {unknown route, a GET of a POST-only path, a missing sheet, a missing
+// link, a bad parameter, a panic} × {signed in, signed out}.
+func TestHTMLErrorsRenderTheErrorPage(t *testing.T) {
+ app, cookies := signIn(t)
+ s, _ := newSheet(t, app, cookies, "Budget")
+ id := strconv.FormatUint(s.ID, 10)
+
+ type result struct {
+ code int
+ ct string
+ body string
+ }
+ get := func(path string) func([]*http.Cookie) result {
+ return func(c []*http.Cookie) result {
+ rec := page(t, app, c, path)
+ return result{rec.Code, rec.Header().Get("Content-Type"), rec.Body.String()}
+ }
+ }
+ tooLong := func(c []*http.Cookie) result {
+ req := postForm("/sheets/"+id+"/table", map[string][]string{"r": {"0"}, "c": {"0"}, "raw": {strings.Repeat("x", maxRawBytes+1)}})
+ req.Header.Set("Accept", "text/html")
+ rec := do(t, app, as(req, c))
+ return result{rec.Code, rec.Header().Get("Content-Type"), rec.Body.String()}
+ }
+ panics := func(c []*http.Cookie) result {
+ h := app.recoverToErrorPage(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { panic("boom") }))
+ req := httptest.NewRequest(http.MethodGet, "/sheets/"+id, nil)
+ req.Header.Set("Accept", "text/html")
+ rec := httptest.NewRecorder()
+ h.ServeHTTP(rec, as(req, c))
+ return result{rec.Code, rec.Header().Get("Content-Type"), rec.Body.String()}
+ }
+
+ cases := []struct {
+ name string
+ signedIn bool
+ run func([]*http.Cookie) result
+ status int
+ title string
+ }{
+ {"unknown route", true, get("/no/such/page"), 404, "Page not found · Sheets"},
+ {"unknown route", false, get("/no/such/page"), 404, "Page not found · Sheets"},
+ {"GET of a POST-only path", true, get("/signout"), 404, "Page not found · Sheets"},
+ {"GET of a POST-only path", false, get("/appearance"), 404, "Page not found · Sheets"},
+ {"missing sheet", true, get("/sheets/99999"), 404, "Sheet not available · Sheets"},
+ {"not a sheet id", true, get("/sheets/nope/table"), 404, "Sheet not available · Sheets"},
+ {"missing link", false, get("/l/doesnotexist"), 404, "Sheet not available · Sheets"},
+ {"missing link", true, get("/l/doesnotexist"), 404, "Sheet not available · Sheets"},
+ {"bad parameter", true, tooLong, 413, "Too long · Sheets"},
+ {"panic", true, panics, 500, "Something went wrong · Sheets"},
+ {"panic", false, panics, 500, "Something went wrong · Sheets"},
+ }
+ for _, c := range cases {
+ who, jar := "signed out", []*http.Cookie(nil)
+ if c.signedIn {
+ who, jar = "signed in", cookies
+ }
+ t.Run(c.name+", "+who, func(t *testing.T) {
+ got := c.run(jar)
+ if got.code != c.status {
+ t.Fatalf("status %d, want %d", got.code, c.status)
+ }
+ if !strings.HasPrefix(got.ct, "text/html") {
+ t.Fatalf("Content-Type %q, want text/html", got.ct)
+ }
+ if title := titleOf(t, got.body); title != c.title {
+ t.Fatalf("<title> %q, want %q", title, c.title)
+ }
+ if !strings.Contains(got.body, "<header estilo-bar") || !strings.Contains(got.body, "<div rst-error>") {
+ t.Fatal("the error page is not inside the layout and bar")
+ }
+ if account := strings.Contains(got.body, "<details estilo-account"); account != c.signedIn {
+ t.Fatalf("account menu present = %v, want %v", account, c.signedIn)
+ }
+ if c.status == 500 && !regexp.MustCompile(`rst-error-ref dir="auto">Reference: [a-z0-9]{6}<`).MatchString(got.body) {
+ t.Fatal("a 500 page carries no reference")
+ }
+ })
+ }
+
+ // Non-HTML stays non-HTML: a.notFound is shared by the grid's page
+ // and its /api fetches (spec §3.3), so the split is by Accept. The
+ // grid's fetch and a native client never ask for a page and never
+ // get one.
+ for _, p := range []string{"/no/such/page", "/sheets/99999", fmt.Sprintf("/api/sheets/99999/tabs/%d/cells", s.TabID)} {
+ for _, accept := range []string{"", "application/json", "*/*"} {
+ req := httptest.NewRequest(http.MethodGet, p, nil)
+ if accept != "" {
+ req.Header.Set("Accept", accept)
+ }
+ rec := do(t, app, as(req, cookies))
+ if rec.Code != http.StatusNotFound || strings.HasPrefix(rec.Header().Get("Content-Type"), "text/html") || strings.Contains(rec.Body.String(), "<html") {
+ t.Errorf("%s Accept %q: %d %q, want a plain 404", p, accept, rec.Code, rec.Header().Get("Content-Type"))
+ }
+ }
+ }
+ // Any other method to a known path is 405 in plain text, with Allow.
+ req := httptest.NewRequest(http.MethodDelete, "/signout", nil)
+ rec := do(t, app, req)
+ if rec.Code != http.StatusMethodNotAllowed || rec.Header().Get("Allow") != "POST" {
+ t.Errorf("DELETE /signout: %d Allow=%q", rec.Code, rec.Header().Get("Allow"))
+ }
+}
+
+// The error page's Back link is offered only for a same-origin Referer
+// that is not this page (rastrillo's rule; otherwise it is an open
+// redirect with better manners), and never for a capability path.
+func TestErrorPageBackLinkIsSameOriginOnly(t *testing.T) {
+ app, _ := appWithOrigin(t, testOrigin)
+ for referer, want := range map[string]string{
+ testOrigin + "/sheets?page=2": `<a rst-btn href="/sheets?page=2">Back</a>`,
+ "https://evil.example/sheets": "",
+ testOrigin + "/no/such/page": "",
+ "//evil.example/x": "",
+ testOrigin + "/l/secret-token": "",
+ "https://u@sheets.example.com/": "",
+ } {
+ req := httptest.NewRequest(http.MethodGet, "/no/such/page", nil)
+ req.Header.Set("Accept", "text/html")
+ req.Header.Set("Referer", referer)
+ body := do(t, app, req).Body.String()
+ has := strings.Contains(body, `>Back</a>`)
+ if (want == "") == has || (want != "" && !strings.Contains(body, want)) {
+ t.Errorf("Referer %q: Back link present = %v, want %q", referer, has, want)
+ }
+ }
+}
+
+// Every http.Error left in the package is classified: a plain answer on
+// purpose, for a fetch, a stream or a native client, or a middleware
+// refusal no navigation meets. A new unclassified site fails here — a
+// page handler should call a.notFound, a.fail, a.forbidden or
+// a.badRequest, which answer a browser with the page.
+func TestEveryPlainTextErrorIsClassified(t *testing.T) {
+ classified := map[string]struct {
+ n int
+ why string
+ }{
+ "errors.go:App.fail": {1, "the non-HTML half of the negotiated answer"},
+ "errors.go:App.answer": {1, "the non-HTML half of the negotiated answer"},
+ "errors.go:App.errorPage": {1, "the error page itself could not render"},
+ "errors.go:App.methodNotAllowed": {1, "405 for a non-GET method, which no navigation sends"},
+ "errors.go:App.recoverToErrorPage": {1, "the non-HTML half of a recovered panic"},
+ "errors.go:App.ErrorPage": {1, "the non-HTML half of rastrillo's recovered panic"},
+ "handlers_cell_batch.go:App.handleCellBatchPost": {7, "grid JSON API"},
+ "handlers_format.go:App.handleFormatPost": {6, "grid JSON API"},
+ "handlers_grid.go:App.handleCellsGet": {1, "grid JSON API"},
+ "handlers_grid.go:App.handleCellPost": {7, "grid JSON API"},
+ "handlers_size.go:App.handleSizePost": {7, "grid JSON API"},
+ "handlers_rename.go:App.handleSheetRename": {1, "native only: a browser's blank title redirects back"},
+ "handlers_view.go:App.handleViewSave": {1, "native only: a browser's bad spec redirects back"},
+ "home.go:App.homeRequire": {2, "shared session guard for pages and fetches; a page GET redirects to /signin (R3)"},
+ "home.go:App.homeLocalComplete": {2, "Home link completion refusals (R3: moves with home/connect)"},
+ "home.go:App.rejectDisabledHome": {1, "shared session guard for pages and fetches (R3)"},
+ "jobpages.go:App.renderJobFragment": {2, "the job page's polled fragment, not a page"},
+ }
+ got := httpErrorSites(t)
+ var problems []string
+ for site, n := range got {
+ c, ok := classified[site]
+ switch {
+ case !ok:
+ problems = append(problems, fmt.Sprintf("%s: %d unclassified plain-text error(s)", site, n))
+ case c.n != n:
+ problems = append(problems, fmt.Sprintf("%s: %d plain-text error(s), classified %d (%s)", site, n, c.n, c.why))
+ }
+ }
+ for site := range classified {
+ if _, ok := got[site]; !ok {
+ problems = append(problems, site+": classified but no longer present — drop it from the list")
+ }
+ }
+ sort.Strings(problems)
+ for _, p := range problems {
+ t.Error(p)
+ }
+ if len(got) == 0 {
+ t.Fatal("found no http.Error sites at all; the walk is looking at nothing")
+ }
+}
+
+// httpErrorSites counts http.Error and http.NotFound calls per
+// file:function in this package's non-test sources.
+func httpErrorSites(t *testing.T) map[string]int {
+ t.Helper()
+ fset := token.NewFileSet()
+ files, err := filepath.Glob("*.go")
+ if err != nil {
+ t.Fatal(err)
+ }
+ sites := map[string]int{}
+ for _, f := range files {
+ if strings.HasSuffix(f, "_test.go") {
+ continue
+ }
+ file, err := parser.ParseFile(fset, f, nil, 0)
+ if err != nil {
+ t.Fatal(err)
+ }
+ for _, d := range file.Decls {
+ fn, ok := d.(*ast.FuncDecl)
+ if !ok {
+ continue
+ }
+ name := fn.Name.Name
+ if fn.Recv != nil {
+ typ := fn.Recv.List[0].Type
+ if s, ok := typ.(*ast.StarExpr); ok {
+ typ = s.X
+ }
+ if id, ok := typ.(*ast.Ident); ok {
+ name = id.Name + "." + name
+ }
+ }
+ ast.Inspect(fn, func(n ast.Node) bool {
+ call, ok := n.(*ast.CallExpr)
+ if !ok {
+ return true
+ }
+ if sel, ok := call.Fun.(*ast.SelectorExpr); ok {
+ if x, ok := sel.X.(*ast.Ident); ok && x.Name == "http" && (sel.Sel.Name == "Error" || sel.Sel.Name == "NotFound") {
+ sites[f+":"+name]++
+ }
+ }
+ return true
+ })
+ }
+ }
+ return sites
+}
diff --git a/internal/sheets/home_test.go b/internal/sheets/home_test.go
index 7e4b6e5..8526e34 100644
--- a/internal/sheets/home_test.go
+++ b/internal/sheets/home_test.go
@@ -26,6 +26,13 @@ type homeFixture struct {
}
func newHomeFixture(t *testing.T) *homeFixture {
+ t.Helper()
+ return newHomeFixtureWith(t, "")
+}
+
+// newHomeFixtureWith is newHomeFixture with the hub directory origin set
+// (HOME_DIRECTORY_ORIGIN), which is what turns the app switcher on.
+func newHomeFixtureWith(t *testing.T, directory string) *homeFixture {
t.Helper()
app, _ := appWithOrigin(t, testOrigin)
key, err := assertioncrypto.Generate()
@@ -33,7 +40,7 @@ func newHomeFixture(t *testing.T) *homeFixture {
t.Fatal(err)
}
keys, _ := json.Marshal(map[string]string{"key": base64.StdEncoding.EncodeToString(key.SignPub())})
- cfg := connect.Credentials{Issuer: homeTestIssuer, ConsumerID: "sheets-preview", KeysBase64: base64.StdEncoding.EncodeToString(keys), DEKBase64: base64.StdEncoding.EncodeToString(bytes.Repeat([]byte{81}, 32))}
+ cfg := connect.Credentials{Issuer: homeTestIssuer, ConsumerID: "sheets-preview", KeysBase64: base64.StdEncoding.EncodeToString(keys), DEKBase64: base64.StdEncoding.EncodeToString(bytes.Repeat([]byte{81}, 32)), Directory: directory}
d, err := app.db.DB()
if err != nil {
t.Fatal(err)