test(veans): pin runUpdate's call-order invariants

The two ordering rules in commands/update.go::runUpdate aren't enforced
by anything beyond the lines being written in that sequence:

  1. MoveTaskToBucket runs AFTER UpdateTask, so a status transition
     doesn't clobber freshly attached labels.
  2. The scrapped-reason comment posts BEFORE the bucket move, so the
     audit trail reads chronologically.

Both are documented in CLAUDE.md but neither is exercised by the e2e
suite: TestUpdate_DescriptionReplaceUniqueness is the only update-side
e2e and it only covers --description-replace-old/new.

Add two unit tests that drive runUpdate against an httptest.Server and
assert the exact (method, path) sequence. Sanity-checked locally by
swapping the field-update and bucket-move blocks — both tests fail with
a clear order diff, confirming they catch the regression that's most
likely to slip through review.
This commit is contained in:
Tink bot 2026-05-26 19:41:03 +00:00 committed by kolaente
parent ba6615f378
commit f04930137e
1 changed files with 146 additions and 0 deletions

View File

@ -17,8 +17,17 @@
package commands
import (
"context"
"encoding/json"
"net/http"
"net/http/httptest"
"reflect"
"strings"
"sync"
"testing"
"code.vikunja.io/veans/internal/client"
"code.vikunja.io/veans/internal/config"
)
func TestComposeDescription_FullReplace(t *testing.T) {
@ -119,3 +128,140 @@ func TestNormalizeLabelTitle(t *testing.T) {
}
}
}
// recordedCall captures one HTTP request made during a runUpdate invocation.
// The fake server appends these in order; tests assert against the sequence.
type recordedCall struct {
method string
path string
}
// startRecordingServer spins up an httptest.Server that answers every
// Vikunja endpoint runUpdate touches with the minimum payload needed to
// keep the call chain alive, while appending each (method, path) to the
// returned slice. The server intentionally does NOT validate request
// bodies — the goal here is to pin call ORDER, not wire shape (which the
// e2e suite already covers).
func startRecordingServer(t *testing.T) (*httptest.Server, *[]recordedCall) {
t.Helper()
var (
mu sync.Mutex
calls []recordedCall
)
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
mu.Lock()
calls = append(calls, recordedCall{method: r.Method, path: r.URL.Path})
mu.Unlock()
w.Header().Set("Content-Type", "application/json")
switch {
case r.Method == http.MethodGet && r.URL.Path == "/api/v1/tasks/42":
// Initial fetch + the final refetch both land here. Return a
// fixed task with an empty label set — labels.go's
// findLabelOnTask only iterates t.Labels.
_ = json.NewEncoder(w).Encode(map[string]any{
"id": 42, "title": "t", "updated": "2026-01-01T00:00:00Z",
})
case r.Method == http.MethodPut && r.URL.Path == "/api/v1/tasks/42/comments":
_ = json.NewEncoder(w).Encode(map[string]any{"id": 1, "comment": ""})
case r.Method == http.MethodPost && r.URL.Path == "/api/v1/tasks/42":
// UpdateTask. Echo back the id so the encoder downstream is
// happy with a non-nil Task.
_ = json.NewEncoder(w).Encode(map[string]any{"id": 42})
case r.Method == http.MethodPost && strings.HasPrefix(r.URL.Path, "/api/v1/projects/") && strings.HasSuffix(r.URL.Path, "/tasks"):
_ = json.NewEncoder(w).Encode(map[string]any{"id": 42})
case r.Method == http.MethodGet && r.URL.Path == "/api/v1/labels":
// getOrCreateLabelByTitle's lookup. Empty array → falls through
// to label creation.
_ = json.NewEncoder(w).Encode([]any{})
case r.Method == http.MethodPut && r.URL.Path == "/api/v1/labels":
_ = json.NewEncoder(w).Encode(map[string]any{"id": 99, "title": "veans:bug"})
case r.Method == http.MethodPut && r.URL.Path == "/api/v1/tasks/42/labels":
_ = json.NewEncoder(w).Encode(map[string]any{"id": 99})
default:
t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path)
http.Error(w, "unexpected", http.StatusInternalServerError)
}
}))
t.Cleanup(srv.Close)
return srv, &calls
}
// newTestRuntime returns a *runtime suitable for runUpdate tests. The
// bucket IDs match the order the canonical statuses appear in
// status.BucketTitleAliases — Todo=10, InProgress=11, etc. — so test
// assertions can name the moved-to bucket by id.
func newTestRuntime(serverURL string) *runtime {
return &runtime{
cfg: &config.Config{
Server: serverURL,
ProjectID: 7,
ViewID: 1,
Buckets: config.Buckets{
Todo: 10, InProgress: 11, InReview: 12, Done: 13, Scrapped: 14,
},
},
client: client.New(serverURL, "tk"),
}
}
// TestRunUpdate_ScrappedOrdersCommentUpdateMove pins the audit-trail
// ordering invariant from CLAUDE.md ("Comments for `--status scrapped`
// post BEFORE the bucket move so the audit trail reads in chronological
// order"). A refactor that hoists the bucket move ahead of the comment
// would silently swap the timeline; this test fails if that happens.
func TestRunUpdate_ScrappedOrdersCommentUpdateMove(t *testing.T) {
srv, calls := startRecordingServer(t)
rt := newTestRuntime(srv.URL)
if _, err := runUpdate(context.Background(), rt, 42, &updateFlags{
statusName: "scrapped",
reason: "obsolete",
}); err != nil {
t.Fatalf("runUpdate: %v", err)
}
want := []recordedCall{
{http.MethodGet, "/api/v1/tasks/42"}, // current task fetch
{http.MethodPut, "/api/v1/tasks/42/comments"}, // "Scrapped: obsolete"
{http.MethodPost, "/api/v1/tasks/42"}, // field update (done=true)
{http.MethodPost, "/api/v1/projects/7/views/1/buckets/14/tasks"}, // bucket move to Scrapped
{http.MethodGet, "/api/v1/tasks/42"}, // refetch with new bucket
}
if !reflect.DeepEqual(*calls, want) {
t.Fatalf("call order mismatch:\nwant: %#v\ngot: %#v", want, *calls)
}
}
// TestRunUpdate_BucketMoveBeforeLabelAdd pins the second ordering
// invariant from CLAUDE.md ("MoveTaskToBucket runs AFTER the field
// update so a status transition can't clobber freshly attached labels").
// Equivalently — and what this test asserts — labels are attached AFTER
// the bucket move, so the post-move state is the one we then refetch.
// A refactor that consolidates "all bucket-related work" by moving the
// labels-add loop ahead of MoveTaskToBucket would compile and silently
// regress label visibility; this test catches that.
func TestRunUpdate_BucketMoveBeforeLabelAdd(t *testing.T) {
srv, calls := startRecordingServer(t)
rt := newTestRuntime(srv.URL)
if _, err := runUpdate(context.Background(), rt, 42, &updateFlags{
statusName: "in-progress",
addLabels: []string{"bug"},
}); err != nil {
t.Fatalf("runUpdate: %v", err)
}
want := []recordedCall{
{http.MethodGet, "/api/v1/tasks/42"}, // current task fetch
{http.MethodPost, "/api/v1/tasks/42"}, // field update (done=false)
{http.MethodPost, "/api/v1/projects/7/views/1/buckets/11/tasks"}, // bucket move to In Progress
{http.MethodGet, "/api/v1/labels"}, // getOrCreateLabelByTitle lookup
{http.MethodPut, "/api/v1/labels"}, // create veans:bug
{http.MethodPut, "/api/v1/tasks/42/labels"}, // attach label
{http.MethodGet, "/api/v1/tasks/42"}, // refetch
}
if !reflect.DeepEqual(*calls, want) {
t.Fatalf("call order mismatch:\nwant: %#v\ngot: %#v", want, *calls)
}
}