amadan
Add content: close the four guard gaps from the re-review (GPT-6 Astra)
Astra verified the runtime fixes in Chromium and found four mutations that passed make ci. The refresh header was spelled twice, once in drop.js and once in notice.go, and the tests used a third copy, so the script could drift unnoticed. The drop zone now publishes the name as data-refresh-header, the way it already publishes every sentence and cap, and drop.js sends what it reads. The notice tests send whatever header the page names, and a shape test pins that drop.js reads the attribute and spells no header of its own. Containment matched rst-refresh="..." with a regex, so single quotes went unseen — and Chromium honoured them, erasing a half-written paste. The tests now parse the page with x/net/html (a test-only import, already in the module graph) and check DOM ancestry. Clearing the wrong cookie went unnoticed. A new test seeds both notices and visits the pages in both orders, twice each. The tray link's destination and its staying visible after the refresh are now pinned. Fourteen mutations, each built before testing and each red: Astra's four, plus the script reading another attribute, the template naming a header the server does not check, a double-quoted region, the drop zone and the passages region removed, put() hiding the pointer again, and the four from the previous round.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
10 files changed,
+349
−121
go.mod+1 −0go.sum+2 −8internal/memoria/delete_test.go+3 −22internal/memoria/handlers.go+7 −1internal/memoria/notice_test.go+86 −0internal/memoria/static/drop.js+6 −1internal/memoria/templates/add.html+1 −0internal/memoriatest/dom_test.go+92 −0internal/memoriatest/notice_test.go+104 −8internal/memoriatest/refresh_test.go+47 −81
CI passed — run details
CI log
=== vet === go vet ./... go: downloading amadan.net/oficina/estilo.git v0.0.0-20260929165053-846b39772f3c go: downloading golang.org/x/net v0.57.0 === fmt === === test === go test ./... ? amadan.net/oficina/memoria/cmd/memoria [no test files] ok amadan.net/oficina/memoria/internal/memoria 3.383s ok amadan.net/oficina/memoria/internal/memoria/chunk 0.006s ok amadan.net/oficina/memoria/internal/memoria/fact 0.566s ? amadan.net/oficina/memoria/internal/memoria/icons [no test files] ok amadan.net/oficina/memoria/internal/memoria/index 0.034s ok amadan.net/oficina/memoria/internal/memoria/notion 0.023s ok amadan.net/oficina/memoria/internal/memoria/provider 1.134s ok amadan.net/oficina/memoria/internal/memoria/query 0.003s ok amadan.net/oficina/memoria/internal/memoria/seal 0.003s ok amadan.net/oficina/memoria/internal/memoria/sibling 3.028s ok amadan.net/oficina/memoria/internal/memoriatest 1.718s ? amadan.net/oficina/memoria/scripts/writeestilo [no test files] ? amadan.net/oficina/memoria/tools/bench [no test files] ? amadan.net/oficina/memoria/tools/drive [no test files] ? amadan.net/oficina/memoria/tools/seed [no test files] ? amadan.net/oficina/memoria/tools/vendorassets [no test files] === migration-check === go run github.com/carlosframework/rastrillo/cmd/rastrillo migration check rastrillo migration check: models and migrations agree
diff --git a/go.mod b/go.mod| index 6998b6e..2b55040 100644 |
| --- a/go.mod |
| +++ b/go.mod |
| @@ -14,6 +14,7 @@ require ( |
| github.com/go-chi/chi/v5 v5.3.2 |
| github.com/google/uuid v1.6.0 |
| golang.org/x/crypto v0.55.0 |
| + golang.org/x/net v0.57.0 |
| gorm.io/gorm v1.31.2 |
| modernc.org/sqlite v1.55.0 |
| ) |
diff --git a/go.sum b/go.sum| index df349a7..f4ea6fa 100644 |
| --- a/go.sum |
| +++ b/go.sum |
| @@ -1,11 +1,3 @@ |
| -amadan.net/oficina/estilo.git v0.0.0-20260911182719-0bcb9287988d h1:3HsB96tGs+P0tGf2VzqpqA8yqYBhJ+apBh2txAoz9qI= |
| -amadan.net/oficina/estilo.git v0.0.0-20260911182719-0bcb9287988d/go.mod h1:0R45/vCUAT+kpS7S6K/ZxBdE8f7YxFN4HT3HyDJ3nPY= |
| -amadan.net/oficina/estilo.git v0.0.0-20260929070530-5b2ce835e773 h1:9DIAhG3TaGG+LE2+8LvU73ZEhfK+oNn2y2p4kFjAAi0= |
| -amadan.net/oficina/estilo.git v0.0.0-20260929070530-5b2ce835e773/go.mod h1:4u0jNBtIdMLOsJyrScfqoEuYDJF7BBM3qM9YWbzt6aw= |
| -amadan.net/oficina/estilo.git v0.0.0-20260929095046-d0664ee49914 h1:V0AyxXYCxqGZClOLqvppw+cCTXsn3b9NmRQ8CV5ZlXY= |
| -amadan.net/oficina/estilo.git v0.0.0-20260929095046-d0664ee49914/go.mod h1:4u0jNBtIdMLOsJyrScfqoEuYDJF7BBM3qM9YWbzt6aw= |
| -amadan.net/oficina/estilo.git v0.0.0-20260929095838-0994675f496e h1:LTSmIWUS8RjPb6m94Ynf7moyFYeBXr08rdOr3nmNqvg= |
| -amadan.net/oficina/estilo.git v0.0.0-20260929095838-0994675f496e/go.mod h1:4u0jNBtIdMLOsJyrScfqoEuYDJF7BBM3qM9YWbzt6aw= |
| amadan.net/oficina/estilo.git v0.0.0-20260929165053-846b39772f3c h1:YYocIz3r3b4oIA0K/58JkSsmyTOCzMT/2GaW/MFbbPo= |
| amadan.net/oficina/estilo.git v0.0.0-20260929165053-846b39772f3c/go.mod h1:4u0jNBtIdMLOsJyrScfqoEuYDJF7BBM3qM9YWbzt6aw= |
| amadan.net/oficina/home/connect v0.0.0-20260910000424-5b8d8410775f h1:lkw3tjzHfZxe74pNfo+tGDz4mkZLg+D6R+FTkmsym6M= |
| @@ -66,6 +58,8 @@ golang.org/x/crypto v0.55.0 h1:+KWHjbgOaAQ66dh/YlkZKHlz9ZUlq61AFirAR9ntP8M= |
| golang.org/x/crypto v0.55.0/go.mod h1:uq0V9dE/fzQuJtbnL+2EhWOE63vo164FY8xqEnV9xis= |
| golang.org/x/mod v0.38.0 h1:MECBjubtXD7yj4HrhIUcywNaGeNVUdfVnxmPajOk4yk= |
| golang.org/x/mod v0.38.0/go.mod h1:V6Xz0pq8TQ3dGqVQ1FVHuelZpAL0uNhSkk9ogYP3c40= |
| +golang.org/x/net v0.57.0 h1:K5+3DljvIuDG9/Jv9rvyMywYNFCQ9RSUY6OOTTkT+tE= |
| +golang.org/x/net v0.57.0/go.mod h1:KpXc8iv+r3XplLAG/f7Jsf9RPszJzdR0f58q9vGOuEU= |
| golang.org/x/sync v0.22.0 h1:SZjpbeLmrCk4xhRSZFNZW5gFUeCeFgjekvI/+gfScek= |
| golang.org/x/sync v0.22.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= |
| golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= |
diff --git a/internal/memoria/delete_test.go b/internal/memoria/delete_test.go| index 9794315..b92a93c 100644 |
| --- a/internal/memoria/delete_test.go |
| +++ b/internal/memoria/delete_test.go |
| @@ -9,9 +9,7 @@ package memoria |
| import ( |
| "context" |
| "net/http" |
| - "net/http/cookiejar" |
| "net/http/httptest" |
| - "net/url" |
| "strconv" |
| "strings" |
| "testing" |
| @@ -344,26 +342,9 @@ func TestAFailedDeleteNoticeWaitsForTheIndex(t *testing.T) { |
| t.Fatalf("closing the index: %v", err) |
| } |
| - jar, err := cookiejar.New(nil) |
| - if err != nil { |
| - t.Fatal(err) |
| - } |
| - base := &url.URL{Scheme: "http", Host: "example.com", Path: "/"} |
| - jar.SetCookies(base, postDelete(t, h, id).Result().Cookies()) |
| - visit := func(path string) string { |
| - t.Helper() |
| - req := httptest.NewRequest(http.MethodGet, path, nil) |
| - for _, c := range jar.Cookies(base) { |
| - req.AddCookie(c) |
| - } |
| - rec := httptest.NewRecorder() |
| - h.ServeHTTP(rec, req) |
| - if rec.Code != http.StatusOK { |
| - t.Fatalf("GET %s answered %d, want 200", path, rec.Code) |
| - } |
| - jar.SetCookies(base, rec.Result().Cookies()) |
| - return rec.Body.String() |
| - } |
| + b := newCookieBrowser(t, h) |
| + b.keep(postDelete(t, h, id)) |
| + visit := b.visit |
| failed := newDropZone().DeleteFailed |
| if strings.Contains(visit("/add"), failed) { |
diff --git a/internal/memoria/handlers.go b/internal/memoria/handlers.go| index facd4ce..bc9e64f 100644 |
| --- a/internal/memoria/handlers.go |
| +++ b/internal/memoria/handlers.go |
| @@ -239,6 +239,11 @@ type dropZone struct { |
| // reads it. |
| MaxInFlight int |
| + // RefreshHeader is the header the tray's re-fetch sends so the page |
| + // leaves its notice unspent (notice.go). Published here so the script |
| + // and the server cannot spell it differently. |
| + RefreshHeader string |
| + |
| // ---- the tray ----------------------------------------------------- |
| // |
| // A drop no longer fills the form's input and wait for Store it: each |
| @@ -395,7 +400,8 @@ func newDropZone() dropZone { |
| // silent nothing is the one outcome that must not happen. |
| Blocked: "This browser would not let a dropped file into the form, so nothing was taken. Choose the files above instead.", |
| - MaxInFlight: maxInFlight, |
| + MaxInFlight: maxInFlight, |
| + RefreshHeader: refreshHeader, |
| TrayTitle: "Uploads", |
| TrayDismiss: "Dismiss", |
diff --git a/internal/memoria/notice_test.go b/internal/memoria/notice_test.go| new file mode 100644 |
| index 0000000..9c94e8d |
| --- /dev/null |
| +++ b/internal/memoria/notice_test.go |
| @@ -0,0 +1,86 @@ |
| +package memoria |
| + |
| +import ( |
| + "net/http" |
| + "net/http/cookiejar" |
| + "net/http/httptest" |
| + "net/url" |
| + "strings" |
| + "testing" |
| +) |
| + |
| +// cookieBrowser carries cookies between requests the way a browser does, |
| +// deletions included, so a test can visit pages in an order and see what |
| +// each visit spent. |
| +type cookieBrowser struct { |
| + t *testing.T |
| + h http.Handler |
| + jar *cookiejar.Jar |
| +} |
| + |
| +var cookieBase = &url.URL{Scheme: "http", Host: "example.com", Path: "/"} |
| + |
| +func newCookieBrowser(t *testing.T, h http.Handler) *cookieBrowser { |
| + jar, err := cookiejar.New(nil) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + return &cookieBrowser{t: t, h: h, jar: jar} |
| +} |
| + |
| +func (b *cookieBrowser) keep(rec *httptest.ResponseRecorder) { |
| + b.jar.SetCookies(cookieBase, rec.Result().Cookies()) |
| +} |
| + |
| +func (b *cookieBrowser) visit(path string) string { |
| + b.t.Helper() |
| + req := httptest.NewRequest(http.MethodGet, path, nil) |
| + for _, c := range b.jar.Cookies(cookieBase) { |
| + req.AddCookie(c) |
| + } |
| + rec := httptest.NewRecorder() |
| + b.h.ServeHTTP(rec, req) |
| + if rec.Code != http.StatusOK { |
| + b.t.Fatalf("GET %s answered %d, want 200", path, rec.Code) |
| + } |
| + b.keep(rec) |
| + return rec.Body.String() |
| +} |
| + |
| +// With both notices pending, each page shows its own once and leaves the |
| +// other alone. |
| +// |
| +// Astra, 2026-09-29: clearing the wrong cookie passed make ci. Taking the |
| +// index's notice then erased a pending upload refusal and left the delete |
| +// notice to be said again. Both orders are run because that mutation fails |
| +// differently in each. |
| +// |
| +// MUTATION-TESTED 2026-09-29, reverted: the clearing cookie in |
| +// notice.take named string(addNotice) instead of string(n) → "/add no |
| +// longer shows its notice after a visit to /" and "/ showed its notice a |
| +// second time (/ first)". |
| +func TestEachPageSpendsOnlyItsOwnNotice(t *testing.T) { |
| + const uploaded, deleted = "scan.png is not text", "The delete did not happen" |
| + pages := map[string]string{"/": deleted, "/add": uploaded} |
| + for _, order := range [][2]string{{"/", "/add"}, {"/add", "/"}} { |
| + h, _ := appWithAWorkingIndex(t) |
| + b := newCookieBrowser(t, h) |
| + seed := httptest.NewRecorder() |
| + addNotice.set(seed, "danger", uploaded) |
| + indexNotice.set(seed, "danger", deleted) |
| + b.keep(seed) |
| + |
| + first, second := order[0], order[1] |
| + if page := b.visit(first); !strings.Contains(page, pages[first]) { |
| + t.Fatalf("%s did not show its notice with both pending", first) |
| + } |
| + if page := b.visit(second); !strings.Contains(page, pages[second]) { |
| + t.Errorf("%s no longer shows its notice after a visit to %s", second, first) |
| + } |
| + for path, said := range pages { |
| + if strings.Contains(b.visit(path), said) { |
| + t.Errorf("%s showed its notice a second time (%s first); it is one-shot", path, first) |
| + } |
| + } |
| + } |
| +} |
diff --git a/internal/memoria/static/drop.js b/internal/memoria/static/drop.js| index 306a77b..fc7742d 100644 |
| --- a/internal/memoria/static/drop.js |
| +++ b/internal/memoria/static/drop.js |
| @@ -110,6 +110,11 @@ |
| var maxInFlight = number("data-max-in-flight"); |
| if (maxBytes === null || maxTotal === null || maxFiles === null) return; |
| if (sniffBytes === null || maxInFlight === null) return; |
| + // The header that tells the server a re-fetch is not a visit. Its name |
| + // is the server's, read off the page for the same reason the numbers |
| + // are. |
| + var refreshHeader = zone.getAttribute("data-refresh-header"); |
| + if (!refreshHeader) return; |
| var msg = {}; |
| var keys = [ |
| @@ -412,7 +417,7 @@ |
| req.setRequestHeader("Cache-Control", "no-cache"); |
| // Tells the server this is not a visit, so it leaves the page's |
| // one-shot notice for the visit that will show it (notice.go). |
| - req.setRequestHeader("X-Memoria-Refresh", "1"); |
| + req.setRequestHeader(refreshHeader, "1"); |
| req.onload = function () { |
| // A newer fetch is already out, or a drop started while this one |
| // was in the air: either way this answer is older than the page it |
diff --git a/internal/memoria/templates/add.html b/internal/memoria/templates/add.html| index b42a86a..7af1689 100644 |
| --- a/internal/memoria/templates/add.html |
| +++ b/internal/memoria/templates/add.html |
| @@ -46,6 +46,7 @@ |
| data-max-files="{{.MaxFiles}}" |
| data-sniff-bytes="{{.SniffBytes}}" |
| data-max-in-flight="{{.MaxInFlight}}" |
| + data-refresh-header="{{.RefreshHeader}}" |
| data-msg-toomany="{{.TooManyFiles}}" |
| data-msg-toototal="{{.TooMuchAtOnce}}" |
| data-msg-folder="{{.Folder}}" |
diff --git a/internal/memoriatest/dom_test.go b/internal/memoriatest/dom_test.go| new file mode 100644 |
| index 0000000..377a783 |
| --- /dev/null |
| +++ b/internal/memoriatest/dom_test.go |
| @@ -0,0 +1,92 @@ |
| +package memoriatest |
| + |
| +import ( |
| + "strings" |
| + "testing" |
| + |
| + "golang.org/x/net/html" |
| +) |
| + |
| +// The helpers in this file read a page the way a browser does, through |
| +// an HTML parser, so a check cannot be passed by spelling the same markup |
| +// differently: rst-refresh='x', RST-REFRESH="x" and rst-refresh=x are one |
| +// attribute to a browser and three strings to a regex. |
| + |
| +func parsePage(t *testing.T, body string) *html.Node { |
| + t.Helper() |
| + doc, err := html.Parse(strings.NewReader(body)) |
| + if err != nil { |
| + t.Fatalf("parsing the page: %v", err) |
| + } |
| + return doc |
| +} |
| + |
| +func attrOf(n *html.Node, key string) (string, bool) { |
| + for _, a := range n.Attr { |
| + if a.Key == key { |
| + return a.Val, true |
| + } |
| + } |
| + return "", false |
| +} |
| + |
| +func findAll(root *html.Node, match func(*html.Node) bool) []*html.Node { |
| + var out []*html.Node |
| + var walk func(*html.Node) |
| + walk = func(n *html.Node) { |
| + if n.Type == html.ElementNode && match(n) { |
| + out = append(out, n) |
| + } |
| + for c := n.FirstChild; c != nil; c = c.NextSibling { |
| + walk(c) |
| + } |
| + } |
| + walk(root) |
| + return out |
| +} |
| + |
| +func hasAttr(key string) func(*html.Node) bool { |
| + return func(n *html.Node) bool { _, ok := attrOf(n, key); return ok } |
| +} |
| + |
| +func elementWith(tag, key, val string) func(*html.Node) bool { |
| + return func(n *html.Node) bool { |
| + v, ok := attrOf(n, key) |
| + return n.Data == tag && ok && v == val |
| + } |
| +} |
| + |
| +// within reports whether n is inside ancestor, or is it. |
| +func within(n, ancestor *html.Node) bool { |
| + for ; n != nil; n = n.Parent { |
| + if n == ancestor { |
| + return true |
| + } |
| + } |
| + return false |
| +} |
| + |
| +func textOf(n *html.Node) string { |
| + var b strings.Builder |
| + var walk func(*html.Node) |
| + walk = func(n *html.Node) { |
| + if n.Type == html.TextNode { |
| + b.WriteString(n.Data) |
| + } |
| + for c := n.FirstChild; c != nil; c = c.NextSibling { |
| + walk(c) |
| + } |
| + } |
| + walk(n) |
| + return strings.Join(strings.Fields(b.String()), " ") |
| +} |
| + |
| +// region returns the one element carrying rst-refresh=name. |
| +func region(t *testing.T, doc *html.Node, name string) *html.Node { |
| + t.Helper() |
| + found := findAll(doc, func(n *html.Node) bool { v, ok := attrOf(n, "rst-refresh"); return ok && v == name }) |
| + if len(found) != 1 { |
| + t.Fatalf("%d regions named %q on the page, want 1", len(found), name) |
| + } |
| + return found[0] |
| +} |
diff --git a/internal/memoriatest/notice_test.go b/internal/memoriatest/notice_test.go| index 0f637ee..77367cf 100644 |
| --- a/internal/memoriatest/notice_test.go |
| +++ b/internal/memoriatest/notice_test.go |
| @@ -7,6 +7,8 @@ import ( |
| "net/url" |
| "strings" |
| "testing" |
| + |
| + "golang.org/x/net/html" |
| ) |
| // browser carries cookies from one request to the next the way a browser |
| @@ -33,16 +35,16 @@ func (b *browser) keep(rec *httptest.ResponseRecorder) *httptest.ResponseRecorde |
| return rec |
| } |
| -// visit GETs path with the jar's cookies; refresh marks it as drop.js's |
| -// background re-fetch. |
| -func (b *browser) visit(path string, refresh bool) string { |
| +// visit GETs path with the jar's cookies. A non-empty header marks it as |
| +// drop.js's background re-fetch. |
| +func (b *browser) visit(path, header string) string { |
| b.t.Helper() |
| req := httptest.NewRequest(http.MethodGet, path, nil) |
| for _, c := range b.jar.Cookies(browserBase) { |
| req.AddCookie(c) |
| } |
| - if refresh { |
| - req.Header.Set("X-Memoria-Refresh", "1") |
| + if header != "" { |
| + req.Header.Set(header, "1") |
| } |
| rec := httptest.NewRecorder() |
| b.h.ServeHTTP(rec, req) |
| @@ -67,6 +69,9 @@ func (b *browser) visit(path string, refresh bool) string { |
| // (b) the refresh-header check deleted from notice.take → "drop.js's |
| // background refresh showed the upload's notice, which belongs on |
| // /add" |
| +// (c) the drop zone's data-refresh-header hard-coded to a name notice.go |
| +// does not check → the same line as (b). The refresh is sent with the |
| +// header the PAGE names, which is what drop.js sends. |
| func TestAnUploadNoticeWaitsForTheAddPage(t *testing.T) { |
| for _, first := range []struct { |
| what string |
| @@ -86,14 +91,105 @@ func TestAnUploadNoticeWaitsForTheAddPage(t *testing.T) { |
| t.Fatalf("the submission redirected to %q, want /add", loc) |
| } |
| - if page := b.visit(first.path, first.refresh); strings.Contains(page, "scan.png") { |
| + header := "" |
| + if first.refresh { |
| + header = refreshHeaderOn(t, get(t, h, "/add").Body.String()) |
| + } |
| + if page := b.visit(first.path, header); strings.Contains(page, "scan.png") { |
| t.Errorf("%s showed the upload's notice, which belongs on /add", first.what) |
| } |
| - if said := flashText(t, b.visit(loc, false)); !strings.Contains(said, "scan.png") { |
| + if said := flashText(t, b.visit(loc, "")); !strings.Contains(said, "scan.png") { |
| t.Errorf("after %s, /add no longer names the refused file: %q", first.what, said) |
| } |
| - if page := b.visit("/add", false); strings.Contains(page, "scan.png") { |
| + if page := b.visit("/add", ""); strings.Contains(page, "scan.png") { |
| t.Error("the notice was shown twice; it is one-shot") |
| } |
| } |
| } |
| + |
| +// refreshHeaderOn is the header name /add tells drop.js to send with its |
| +// re-fetch. Tests send what the page names, never a name of their own, so |
| +// a page and a server that disagree are caught here. |
| +func refreshHeaderOn(t *testing.T, addPage string) string { |
| + t.Helper() |
| + zones := findAll(parsePage(t, addPage), hasAttr("rst-dropzone")) |
| + if len(zones) != 1 { |
| + t.Fatalf("%d drop zones on /add, want 1", len(zones)) |
| + } |
| + name, _ := attrOf(zones[0], "data-refresh-header") |
| + if name == "" { |
| + t.Fatal("the drop zone names no refresh header, so drop.js stops before it starts") |
| + } |
| + return name |
| +} |
| + |
| +// drop.js sends the header the page names, and no other. |
| +// |
| +// Go cannot run the script, so this reads it. There is no header name in |
| +// drop.js to drift: it reads data-refresh-header off the drop zone and |
| +// passes that to its one setRequestHeader for the re-fetch. What this |
| +// pins is that shape. TestAnUploadNoticeWaitsForTheAddPage pins that the |
| +// server honours the name the page publishes. |
| +// |
| +// MUTATION-TESTED 2026-09-29, each reverted: |
| +// |
| +// (a) setRequestHeader(refreshHeader, "1") → setRequestHeader( |
| +// "X-Memoria-Refresh-Count", "1"), Astra's divergence → "drop.js does |
| +// not send the header it read off the page" |
| +// (b) the attribute read changed to data-refresh → "drop.js does not |
| +// read its refresh header off the drop zone" |
| +func TestTheDropScriptSendsTheHeaderThePageNames(t *testing.T) { |
| + h := newApp(t) |
| + rec := get(t, h, "/static/drop.js") |
| + if rec.Code != http.StatusOK { |
| + t.Fatalf("GET /static/drop.js: status %d, want 200", rec.Code) |
| + } |
| + js := rec.Body.String() |
| + if !strings.Contains(js, `var refreshHeader = zone.getAttribute("data-refresh-header");`) { |
| + t.Error("drop.js does not read its refresh header off the drop zone") |
| + } |
| + if !strings.Contains(js, `req.setRequestHeader(refreshHeader, "1");`) { |
| + t.Error("drop.js does not send the header it read off the page") |
| + } |
| + if strings.Contains(strings.ToLower(js), "x-memoria") { |
| + t.Error("drop.js spells a header name of its own; the page is where it reads one") |
| + } |
| +} |
| + |
| +// The tray points at the document list, which is not on /add, and keeps |
| +// pointing there. |
| +// |
| +// Astra, 2026-09-29: the link went back to /add, which has no list, and |
| +// changing it back passed make ci. Visibility after the refresh is the |
| +// script's, so the script is read: the pointer is shown in one place and |
| +// hidden in one place, when the tray is dismissed. |
| +// |
| +// MUTATION-TESTED 2026-09-29, each reverted: |
| +// |
| +// (a) href="/" → href="/add" → "the tray's link goes to "/add"" |
| +// (b) `listed.hidden = true;` added back to put() → "drop.js hides the |
| +// tray's pointer in 2 places" |
| +func TestTheTrayPointsAtTheDocumentList(t *testing.T) { |
| + h := newApp(t) |
| + doc := parsePage(t, get(t, h, "/add").Body.String()) |
| + listed := findAll(doc, hasAttr("rst-tray-listed")) |
| + if len(listed) != 1 { |
| + t.Fatalf("%d tray pointers on /add, want 1", len(listed)) |
| + } |
| + links := findAll(listed[0], func(n *html.Node) bool { return n.Data == "a" }) |
| + if len(links) != 1 { |
| + t.Fatalf("the tray pointer holds %d links, want 1", len(links)) |
| + } |
| + if href, _ := attrOf(links[0], "href"); href != "/" { |
| + t.Errorf("the tray's link goes to %q; the document list is on \"/\"", href) |
| + } |
| + |
| + js := get(t, h, "/static/drop.js").Body.String() |
| + if n := strings.Count(js, "listed.hidden = false;"); n != 1 { |
| + t.Errorf("drop.js shows the tray's pointer in %d places, want 1", n) |
| + } |
| + if n := strings.Count(js, "listed.hidden = true;"); n != 1 { |
| + t.Errorf("drop.js hides the tray's pointer in %d places, want 1 (dismissing the tray). "+ |
| + "It is true whether or not the refresh arrives.", n) |
| + } |
| +} |
diff --git a/internal/memoriatest/refresh_test.go b/internal/memoriatest/refresh_test.go| index 6b629eb..5d280e6 100644 |
| --- a/internal/memoriatest/refresh_test.go |
| +++ b/internal/memoriatest/refresh_test.go |
| @@ -3,9 +3,10 @@ package memoriatest |
| import ( |
| "net/http" |
| "net/http/httptest" |
| - "regexp" |
| "strings" |
| "testing" |
| + |
| + "golang.org/x/net/html" |
| ) |
| // The tests in this file cover the list that stops being stale. |
| @@ -84,52 +85,6 @@ import ( |
| // it navigated to "/?stored=2" with the list rendered fresh by the |
| // server. No script ran at any point in that. |
| -// refreshRegion returns one whole rst-refresh region, opening tag to |
| -// matching close. |
| -// |
| -// It counts nested elements of the same name rather than looking for the |
| -// first close tag, and that is not fussiness: the results region contains |
| -// a <div rst-callout> when nothing matched, so a scan that stopped at the |
| -// first </div> would return half a region and every containment assertion |
| -// below would pass for the wrong reason. |
| -func refreshRegion(t *testing.T, body, name string) string { |
| - t.Helper() |
| - at := strings.Index(body, `rst-refresh="`+name+`"`) |
| - if at < 0 { |
| - t.Fatalf("no region named %q on the page:\n%s", name, body) |
| - } |
| - open := strings.LastIndex(body[:at], "<") |
| - if open < 0 { |
| - t.Fatalf("the %q region's attribute is not inside a tag", name) |
| - } |
| - el := regexp.MustCompile(`^<([a-zA-Z][a-zA-Z0-9]*)`).FindStringSubmatch(body[open:]) |
| - if el == nil { |
| - t.Fatalf("the %q region's opening tag has no element name: %.60s", name, body[open:]) |
| - } |
| - tag := regexp.MustCompile(`</?` + el[1] + `\b`) |
| - depth, i := 0, open |
| - for { |
| - loc := tag.FindStringIndex(body[i:]) |
| - if loc == nil { |
| - t.Fatalf("the %q region (<%s>) never closes", name, el[1]) |
| - } |
| - start := i + loc[0] |
| - if body[start+1] == '/' { |
| - depth-- |
| - if depth == 0 { |
| - end := strings.Index(body[start:], ">") |
| - if end < 0 { |
| - t.Fatalf("the %q region's closing tag never closes", name) |
| - } |
| - return body[open : start+end+1] |
| - } |
| - } else { |
| - depth++ |
| - } |
| - i = start + 2 |
| - } |
| -} |
| - |
| // The regions the tray swaps hold NOTHING the person is in the middle of |
| // using. |
| // |
| @@ -151,7 +106,9 @@ func refreshRegion(t *testing.T, body, name string) string { |
| // |
| // (a) rst-refresh="upload" added to /add's <section rst-section> → "the |
| // "upload" region on /add contains the upload form", and the same |
| -// for the drop zone, the tray and the paste box |
| +// for the drop zone, the tray and the paste box. Astra's |
| +// single-quoted rst-refresh='upload' passed the regex this test |
| +// used to be; it reads the parsed page now, and is red on it too. |
| // (b) the rst-dropzone attribute deleted from add.html → "/add has no |
| // drop zone", the presence check, which fatals before any |
| // containment line runs |
| @@ -161,44 +118,51 @@ func TestTheRefreshedRegionsCannotTakeTheFormOrTheTrayWithThem(t *testing.T) { |
| h := newApp(t) |
| submit(t, h, "", "", file{"revenue.txt", "Revenue was 100 across the first region we serve."}) |
| - type marker struct{ what, marker string } |
| + type kept struct { |
| + what string |
| + match func(*html.Node) bool |
| + } |
| for _, pg := range []struct { |
| path string |
| want []string |
| - keep []marker |
| + keep []kept |
| }{ |
| - {"/add", []string{"passages"}, []marker{ |
| - {"upload form", `action="/documents"`}, |
| - {"drop zone", "<div rst-dropzone"}, |
| - {"tray", "<div rst-tray"}, |
| - {"paste box", `<textarea id="body"`}, |
| + {"/add", []string{"passages"}, []kept{ |
| + {"upload form", elementWith("form", "action", "/documents")}, |
| + {"drop zone", hasAttr("rst-dropzone")}, |
| + {"tray", hasAttr("rst-tray")}, |
| + {"paste box", elementWith("textarea", "id", "body")}, |
| }}, |
| // search.js writes into the results region, so the search box |
| // must not be inside it. A searched page, so the region has |
| // something in it to over-reach with. |
| - {"/?q=revenue", []string{"results", "passages"}, []marker{ |
| - {"search box", `<input type="search"`}, |
| + {"/?q=revenue", []string{"results", "passages"}, []kept{ |
| + {"search box", elementWith("input", "type", "search")}, |
| }}, |
| } { |
| - body := get(t, h, pg.path).Body.String() |
| + doc := parsePage(t, get(t, h, pg.path).Body.String()) |
| + protected := map[string][]*html.Node{} |
| for _, k := range pg.keep { |
| - if !strings.Contains(body, k.marker) { |
| - t.Fatalf("%s has no %s (%q), so the containment check below would pass for a "+ |
| - "page without it:\n%s", pg.path, k.what, k.marker, body) |
| + protected[k.what] = findAll(doc, k.match) |
| + if len(protected[k.what]) == 0 { |
| + t.Fatalf("%s has no %s, so the containment check below would pass for a page "+ |
| + "without it", pg.path, k.what) |
| } |
| } |
| + regions := findAll(doc, hasAttr("rst-refresh")) |
| seen := map[string]bool{} |
| - for _, f := range regexp.MustCompile(`rst-refresh="([^"]*)"`).FindAllStringSubmatch(body, -1) { |
| - if f[1] == "" { |
| + for _, r := range regions { |
| + name, _ := attrOf(r, "rst-refresh") |
| + if name == "" { |
| t.Errorf("%s: a region carries an empty rst-refresh; the script pairs regions "+ |
| "by name and an unnamed one can be paired with anything", pg.path) |
| } |
| - if seen[f[1]] { |
| + if seen[name] { |
| t.Errorf("%s: two regions are both named %q, so a swap puts one region's "+ |
| - "content into the other's place", pg.path, f[1]) |
| + "content into the other's place", pg.path, name) |
| } |
| - seen[f[1]] = true |
| + seen[name] = true |
| } |
| for _, want := range pg.want { |
| if !seen[want] { |
| @@ -206,21 +170,22 @@ func TestTheRefreshedRegionsCannotTakeTheFormOrTheTrayWithThem(t *testing.T) { |
| } |
| } |
| - for name := range seen { |
| - region := refreshRegion(t, body, name) |
| - for _, k := range pg.keep { |
| - if strings.Contains(region, k.marker) { |
| - t.Errorf("the %q region on %s contains the %s. A swap replaces the region "+ |
| - "whole, so it would be destroyed and rebuilt from the server's answer — "+ |
| - "a half-typed paste, a drop in progress, or a tray row whose outcome has "+ |
| - "not been said yet, gone without a word.", name, pg.path, k.what) |
| + for _, r := range regions { |
| + name, _ := attrOf(r, "rst-refresh") |
| + for what, nodes := range protected { |
| + for _, n := range nodes { |
| + if within(n, r) { |
| + t.Errorf("the %q region on %s contains the %s. A swap replaces the "+ |
| + "region whole, so it would be destroyed and rebuilt from the server's "+ |
| + "answer — a half-typed paste, a drop in progress, or a tray row whose "+ |
| + "outcome has not been said yet, gone without a word.", name, pg.path, what) |
| + } |
| } |
| } |
| } |
| } |
| - add := get(t, h, "/add").Body.String() |
| - count := refreshRegion(t, add, "passages") |
| + count := textOf(region(t, parsePage(t, get(t, h, "/add").Body.String()), "passages")) |
| if !strings.Contains(count, "indexed") { |
| t.Errorf("the %q region does not hold the passage count: %q", "passages", count) |
| } |
| @@ -232,15 +197,16 @@ func TestTheRefreshedRegionsCannotTakeTheFormOrTheTrayWithThem(t *testing.T) { |
| // The re-fetch drop.js makes of /add brings the passage count up to date. |
| // |
| -// It is made the way drop.js makes it, with the refresh header, so what is |
| -// tested is what the script is given. |
| +// It is made the way drop.js makes it, with the header the page tells the |
| +// script to send. |
| // |
| // MUTATION-TESTED 2026-09-29, reverted: `Chunks: a.chunkCount()` → |
| // `Chunks: 0` in addContent → "the passage count on /add did not change |
| // after 1 document was stored". |
| func TestARefetchOfAddMovesThePassageCount(t *testing.T) { |
| h := newApp(t) |
| - before := refreshRegion(t, get(t, h, "/add").Body.String(), "passages") |
| + add := get(t, h, "/add").Body.String() |
| + before := textOf(region(t, parsePage(t, add), "passages")) |
| if !strings.Contains(before, "indexed") { |
| t.Fatalf("/add's passage region holds no count to compare: %q", before) |
| } |
| @@ -248,13 +214,13 @@ func TestARefetchOfAddMovesThePassageCount(t *testing.T) { |
| readReply(t, submitJSON(t, h, file{"hosting.txt", "Hosting costs fell after the migration completed."})) |
| req := httptest.NewRequest(http.MethodGet, "/add", nil) |
| - req.Header.Set("X-Memoria-Refresh", "1") |
| + req.Header.Set(refreshHeaderOn(t, add), "1") |
| rec := httptest.NewRecorder() |
| h.ServeHTTP(rec, req) |
| if rec.Code != http.StatusOK { |
| t.Fatalf("the re-fetch of /add answered %d, want 200", rec.Code) |
| } |
| - if after := refreshRegion(t, rec.Body.String(), "passages"); after == before { |
| + if after := textOf(region(t, parsePage(t, rec.Body.String()), "passages")); after == before { |
| t.Errorf("the passage count on /add did not change after 1 document was stored: %q. "+ |
| "The tray's re-fetch is the only thing that brings it up to date.", after) |
| } |