Artifact e012c67b07622b47b3bebb5d867f3e7a72a77848ad85105eb8be33aac006b957:
- File
internal/tag/tag_test.go
— part of check-in
[78a793dd83]
at
2026-07-23 11:17:26
on branch trunk
— fix: cover ScanJulianDay's ordering consumers, keep the float64 branch exact (#60) (#121)
Issue #60's triage was stale: the live truncation bug it describes was already closed on main by 11967ff (#62), which added millisecond rounding to timeToJulian. What remained was a coverage gap, not a defect.
An earlier attempt on this branch also normalized the float64 branch onto the millisecond grid, on the theory that a driver-independent helper is strictly better. That was wrong, and this commit does not do it. A scanned float64 IS the value stored in the row, so snapping it can only move it away from what the row holds. Timeline's pagination cursor depends on the opposite property: its predicate `e.mtime < ? OR (e.mtime = ? AND b.rid < ?)` requires the cursor to compare exactly equal to the row it came from. SQLite's own julianday() computes iJD/86400000.0, a different expression from TimeToJulian's julianEpoch + ms/86400000.0, and the two land one ulp apart on a real fraction of instants — so normalizing alters mtimes written by upstream Fossil, the library's primary real-world input. Raised, the cursor row repeats as the next page's first row; lowered, rows are skipped. Both are silent wrong answers.
The float64 branch therefore stays verbatim, and its doc comment now records why that exactness is load-bearing rather than incidental. The same reasoning applies to tag propagation's `tagxref.mtime < ?`, where SQLite compares the bound value against raw stored floats: an exact bound is the correct one there too, so no normalized variant is introduced for it either.
The one genuine transform stays where it belongs, in the time.Time branch, which must round because ncruces hands back DATETIME columns carrying its own sub-millisecond conversion noise. Its comment now also records the limit of that recovery: an mtime written at finer than millisecond resolution cannot be reproduced through a time.Time at all, and rounding the float64 branch to match would discard precision on both drivers rather than recover any.
Non-functional cleanup: millisPerDay and julianDayFromMillis replace two open-coded copies of the same expression.
Coverage added (no build tag; these run under both drivers): - db: TestScanJulianDayFloatBranchIsBitExact sweeps julianday()-computed mtimes and asserts each scans back bit-for-bit, with a guard that fails the test if the sweep ever stops containing divergent values. Verified to fail against the normalizing version and pass against this one. - db: TestScanJulianDayIsDriverIndependent proves the float64 and time.Time representations of one millisecond-aligned write converge, with noise in both directions. TestScanJulianDaySentinelZero pins the "no mtime" sentinel that bisect's `mtime != 0` guard depends on. - tag: TestPropagateAllOrderingAdjacentMillisecond drives the `tagxref.mtime < ?` decision through ScanJulianDay with adjacent-millisecond mtimes; it fails under a truncating conversion and passes under the rounding one, and passes under modernc regardless — the reason this class went unnoticed twice.
Consumer classification (ScanJulianDay): - manifest/timeline.go NewCursor: ordering/equality, requires exactness - tag/propagate.go PropagateAll -> `tagxref.mtime < ?`: ordering (now covered) - tag/propagate.go childMtime -> mtimeQueue.Less: ordering, traversal-order only (visited set makes it result-neutral) - manifest/log.go, bisect/bisect.go: display-only
Known limitation, left alone deliberately: julianToTime truncates where timeToJulian rounds, so the two are not inverses at sub-millisecond resolution. Pre-existing, now documented; ScanTime's consumers are display paths with no ordering boundary. This commit does not widen the gap. (user: [email protected], size: 16929) [annotate] [blame] [check-ins using]
package tag_test import ( "path/filepath" "testing" "time" libfossil "github.com/danmestas/go-libfossil/internal/fsltype" "github.com/danmestas/go-libfossil/internal/manifest" "github.com/danmestas/go-libfossil/internal/repo" "github.com/danmestas/go-libfossil/internal/tag" _ "github.com/danmestas/go-libfossil/internal/testdriver" "github.com/danmestas/go-libfossil/simio" ) func setupTestRepo(t *testing.T) *repo.Repo { t.Helper() path := filepath.Join(t.TempDir(), "test.fossil") r, err := repo.Create(path, "testuser", simio.CryptoRand{}, "") if err != nil { t.Fatalf("repo.Create: %v", err) } t.Cleanup(func() { r.Close() }) return r } func TestAddTag(t *testing.T) { r := setupTestRepo(t) // Create a checkin to tag rid, _, err := manifest.Checkin(r, manifest.CheckinOpts{ Files: []manifest.File{{Name: "hello.txt", Content: []byte("hello")}}, Comment: "initial commit", User: "testuser", Time: time.Date(2024, 1, 15, 10, 0, 0, 0, time.UTC), }) if err != nil { t.Fatalf("Checkin: %v", err) } // Add a singleton tag tagRid, err := tag.AddTag(r, tag.TagOpts{ TargetRID: rid, TagName: "testlabel", TagType: tag.TagSingleton, Value: "myvalue", User: "testuser", Time: time.Date(2024, 1, 15, 11, 0, 0, 0, time.UTC), }) if err != nil { t.Fatalf("tag.AddTag: %v", err) } if tagRid <= 0 { t.Fatalf("tagRid = %d, want > 0", tagRid) } // Verify tagxref has the correct entry var tagtype int var value string err = r.DB().QueryRow( `SELECT tagtype, value FROM tagxref JOIN tag ON tag.tagid = tagxref.tagid WHERE tag.tagname = ? AND tagxref.rid = ?`, "testlabel", rid, ).Scan(&tagtype, &value) if err != nil { t.Fatalf("tagxref query: %v", err) } if tagtype != tag.TagSingleton { t.Fatalf("tagtype = %d, want %d (singleton)", tagtype, tag.TagSingleton) } if value != "myvalue" { t.Fatalf("value = %q, want %q", value, "myvalue") } } func TestCancelTag(t *testing.T) { r := setupTestRepo(t) // Create a checkin (auto-gets sym-trunk tag via propagation in manifest) rid, _, err := manifest.Checkin(r, manifest.CheckinOpts{ Files: []manifest.File{{Name: "hello.txt", Content: []byte("hello")}}, Comment: "initial commit", User: "testuser", Time: time.Date(2024, 1, 15, 10, 0, 0, 0, time.UTC), }) if err != nil { t.Fatalf("Checkin: %v", err) } // Cancel the sym-trunk tag cancelRid, err := tag.AddTag(r, tag.TagOpts{ TargetRID: rid, TagName: "sym-trunk", TagType: tag.TagCancel, User: "testuser", Time: time.Date(2024, 1, 15, 11, 0, 0, 0, time.UTC), }) if err != nil { t.Fatalf("tag.AddTag cancel: %v", err) } if cancelRid <= 0 { t.Fatalf("cancelRid = %d, want > 0", cancelRid) } // Verify tagxref has tagtype=0 (cancel) var tagtype int err = r.DB().QueryRow( `SELECT tagtype FROM tagxref JOIN tag ON tag.tagid = tagxref.tagid WHERE tag.tagname = ? AND tagxref.rid = ?`, "sym-trunk", rid, ).Scan(&tagtype) if err != nil { t.Fatalf("tagxref query: %v", err) } if tagtype != tag.TagCancel { t.Fatalf("tagtype = %d, want %d (cancel)", tagtype, tag.TagCancel) } } // makeCheckin is a test helper that creates a checkin with one file. func makeCheckin(t *testing.T, r *repo.Repo, parent int64, name, content, comment string) int64 { t.Helper() rid, _, err := manifest.Checkin(r, manifest.CheckinOpts{ Files: []manifest.File{{Name: name, Content: []byte(content)}}, Comment: comment, User: "testuser", Parent: libfossil.FslID(parent), Time: time.Now().UTC(), }) if err != nil { t.Fatalf("Checkin: %v", err) } return int64(rid) } func TestPropagateChain(t *testing.T) { r := setupTestRepo(t) // Create chain A→B→C ridA := makeCheckin(t, r, 0, "a.txt", "content A", "commit A") ridB := makeCheckin(t, r, ridA, "b.txt", "content B", "commit B") ridC := makeCheckin(t, r, ridB, "c.txt", "content C", "commit C") // Add propagating "branch" tag to A with value "feature" _, err := tag.AddTag(r, tag.TagOpts{ TargetRID: libfossil.FslID(ridA), TagName: "branch", TagType: tag.TagPropagating, Value: "feature", User: "testuser", Time: time.Now().UTC(), }) if err != nil { t.Fatalf("tag.AddTag: %v", err) } // Verify B has the propagated tag (srcid=0, correct value) var srcidb, tagtypeB int var valueB string err = r.DB().QueryRow(` SELECT srcid, tagtype, value FROM tagxref JOIN tag ON tag.tagid = tagxref.tagid WHERE tag.tagname = 'branch' AND tagxref.rid = ? `, ridB).Scan(&srcidb, &tagtypeB, &valueB) if err != nil { t.Fatalf("tagxref query for B: %v", err) } if srcidb != 0 { t.Errorf("B srcid = %d, want 0 (propagated)", srcidb) } if tagtypeB != tag.TagPropagating { t.Errorf("B tagtype = %d, want %d", tagtypeB, tag.TagPropagating) } if valueB != "feature" { t.Errorf("B value = %q, want %q", valueB, "feature") } // Verify C has the propagated tag var srcidC, tagtypeC int var valueC string err = r.DB().QueryRow(` SELECT srcid, tagtype, value FROM tagxref JOIN tag ON tag.tagid = tagxref.tagid WHERE tag.tagname = 'branch' AND tagxref.rid = ? `, ridC).Scan(&srcidC, &tagtypeC, &valueC) if err != nil { t.Fatalf("tagxref query for C: %v", err) } if srcidC != 0 { t.Errorf("C srcid = %d, want 0 (propagated)", srcidC) } if tagtypeC != tag.TagPropagating { t.Errorf("C tagtype = %d, want %d", tagtypeC, tag.TagPropagating) } if valueC != "feature" { t.Errorf("C value = %q, want %q", valueC, "feature") } } func TestCancelPropagation(t *testing.T) { r := setupTestRepo(t) // Create chain A→B→C ridA := makeCheckin(t, r, 0, "a.txt", "content A", "commit A") ridB := makeCheckin(t, r, ridA, "b.txt", "content B", "commit B") ridC := makeCheckin(t, r, ridB, "c.txt", "content C", "commit C") // Add propagating tag to A _, err := tag.AddTag(r, tag.TagOpts{ TargetRID: libfossil.FslID(ridA), TagName: "testprop", TagType: tag.TagPropagating, Value: "propvalue", User: "testuser", Time: time.Now().UTC(), }) if err != nil { t.Fatalf("tag.AddTag propagating: %v", err) } // Cancel at B _, err = tag.AddTag(r, tag.TagOpts{ TargetRID: libfossil.FslID(ridB), TagName: "testprop", TagType: tag.TagCancel, User: "testuser", Time: time.Now().UTC(), }) if err != nil { t.Fatalf("tag.AddTag cancel: %v", err) } // Verify B has no active tags (count of tagtype>0 should be 0) var countB int err = r.DB().QueryRow(` SELECT COUNT(*) FROM tagxref JOIN tag ON tag.tagid = tagxref.tagid WHERE tag.tagname = 'testprop' AND tagxref.rid = ? AND tagxref.tagtype > 0 `, ridB).Scan(&countB) if err != nil { t.Fatalf("count query for B: %v", err) } if countB != 0 { t.Errorf("B has %d active tags, want 0", countB) } // Verify C has no tagxref row for this tag at all var countC int err = r.DB().QueryRow(` SELECT COUNT(*) FROM tagxref JOIN tag ON tag.tagid = tagxref.tagid WHERE tag.tagname = 'testprop' AND tagxref.rid = ? `, ridC).Scan(&countC) if err != nil { t.Fatalf("count query for C: %v", err) } if countC != 0 { t.Errorf("C has %d tagxref rows, want 0", countC) } } func TestPropagateBgcolor(t *testing.T) { r := setupTestRepo(t) // Create A→B ridA := makeCheckin(t, r, 0, "a.txt", "content A", "commit A") ridB := makeCheckin(t, r, ridA, "b.txt", "content B", "commit B") // Run crosslink to populate event table _, err := manifest.Crosslink(r) if err != nil { t.Fatalf("Crosslink: %v", err) } // Add propagating "bgcolor" tag to A _, err = tag.AddTag(r, tag.TagOpts{ TargetRID: libfossil.FslID(ridA), TagName: "bgcolor", TagType: tag.TagPropagating, Value: "#ff0000", User: "testuser", Time: time.Now().UTC(), }) if err != nil { t.Fatalf("tag.AddTag bgcolor: %v", err) } // Verify event.bgcolor updated at B var bgcolor string err = r.DB().QueryRow("SELECT bgcolor FROM event WHERE objid=?", ridB).Scan(&bgcolor) if err != nil { t.Fatalf("event query for B: %v", err) } if bgcolor != "#ff0000" { t.Errorf("B bgcolor = %q, want %q", bgcolor, "#ff0000") } } func TestApplyTag(t *testing.T) { r := setupTestRepo(t) ridA := makeCheckin(t, r, 0, "a.txt", "aaa", "commit A") ridB := makeCheckin(t, r, ridA, "a.txt", "bbb", "commit B") err := tag.ApplyTag(r, tag.ApplyOpts{ TargetRID: libfossil.FslID(ridA), SrcRID: 999, TagName: "sym-trunk", TagType: tag.TagPropagating, Value: "", MTime: libfossil.TimeToJulian(time.Date(2024, 1, 15, 12, 0, 0, 0, time.UTC)), }) if err != nil { t.Fatalf("tag.ApplyTag: %v", err) } // Verify tagxref at A has srcid=999. var srcid int64 r.DB().QueryRow( "SELECT srcid FROM tagxref JOIN tag USING(tagid) WHERE tagname='sym-trunk' AND rid=?", ridA, ).Scan(&srcid) if srcid != 999 { t.Errorf("A srcid=%d, want 999", srcid) } // Verify propagated to B with srcid=0. r.DB().QueryRow( "SELECT srcid FROM tagxref JOIN tag USING(tagid) WHERE tagname='sym-trunk' AND rid=?", ridB, ).Scan(&srcid) if srcid != 0 { t.Errorf("B srcid=%d, want 0 (propagated)", srcid) } } // TestPropagateAllOrderingAdjacentMillisecond exercises the `tagxref.mtime < ?` // ordering comparison in propagate() — the one confirmed ScanJulianDay consumer // that makes an ordering decision — driven through PropagateAll, where the // bound `?` value is produced by db.ScanJulianDay from the origin row's mtime. // // Goal: prove the "overwrite a child's propagated tag only if it is strictly // older" decision survives a scanned mtime that is only one millisecond away // from the child's stored mtime. Under the ncruces driver the origin mtime is // scanned from a time.Time carrying sub-millisecond conversion noise; if that // noise collapsed the one-millisecond gap (as a prior truncation bug did), the // strict `<` would flip and the overwrite would silently not happen. // // Methodology: A is the ancestor of two children, B and C. B carries an older // propagated tag (base), C a newer one (base+2ms), and A's own tag sits between // them at base+1ms. Re-propagating from A must overwrite B (base < base+1ms) and // leave C untouched (base+2ms is not < base+1ms). Adjacent-millisecond deltas // are the point: a one-millisecond truncation is exactly what flips the B // decision, and only sub-10ms intervals can express that — timestamps hours // apart pass regardless of the bug. This test runs under whichever driver the // testdriver import selects, so `-tags test_ncruces` runs it where the noise // lives. func TestPropagateAllOrderingAdjacentMillisecond(t *testing.T) { r := setupTestRepo(t) // A is the common ancestor; B and C are both primary children of A. ridA := makeCheckin(t, r, 0, "a.txt", "content A", "commit A") ridB := makeCheckin(t, r, ridA, "b.txt", "content B", "commit B") ridC := makeCheckin(t, r, ridA, "c.txt", "content C", "commit C") base := time.Date(2024, 1, 15, 10, 0, 0, 0, time.UTC) mtimeChildOld := libfossil.TimeToJulian(base) // B: older than A mtimeOrigin := libfossil.TimeToJulian(base.Add(1 * time.Millisecond)) // A: one ms newer than B mtimeChildNew := libfossil.TimeToJulian(base.Add(2 * time.Millisecond)) // C: newer than A // Apply the propagating tag at A. This creates the tag and, via propagate, // seeds srcid=0 tagxref rows on B and C that we overwrite below to set exact // mtimes for the comparison. if err := tag.ApplyTag(r, tag.ApplyOpts{ TargetRID: libfossil.FslID(ridA), SrcRID: libfossil.FslID(ridA), // non-zero: an origin, not a propagated row TagName: "branch", TagType: tag.TagPropagating, Value: "new", MTime: mtimeOrigin, }); err != nil { t.Fatalf("tag.ApplyTag: %v", err) } // Pin the children's existing propagated tags to exact mtimes a single // millisecond on either side of the origin. setChildTag := func(rid int64, value string, mtime float64) { t.Helper() if _, err := r.DB().Exec( `UPDATE tagxref SET value=?, mtime=?, srcid=0 WHERE tagid=(SELECT tagid FROM tag WHERE tagname='branch') AND rid=?`, value, mtime, rid, ); err != nil { t.Fatalf("pin child tag for rid=%d: %v", rid, err) } } setChildTag(ridB, "old", mtimeChildOld) setChildTag(ridC, "newer", mtimeChildNew) // Re-propagate from A. The `?` in `tagxref.mtime < ?` now comes from // ScanJulianDay(A.mtime). if err := tag.PropagateAll(r.DB(), libfossil.FslID(ridA)); err != nil { t.Fatalf("tag.PropagateAll: %v", err) } readChildValue := func(rid int64) string { t.Helper() var value string if err := r.DB().QueryRow( `SELECT value FROM tagxref WHERE tagid=(SELECT tagid FROM tag WHERE tagname='branch') AND rid=?`, rid, ).Scan(&value); err != nil { t.Fatalf("read child tag for rid=%d: %v", rid, err) } return value } // B was strictly older than the origin (base < base+1ms) → overwritten. if got := readChildValue(ridB); got != "new" { t.Fatalf("B tag value = %q, want %q: base < origin (one ms) did not overwrite — a lost millisecond collapsed the strict ordering", got, "new") } // C was newer than the origin (base+2ms not < base+1ms) → preserved. This // negative control confirms the comparison actually gates rather than always // overwriting. if got := readChildValue(ridC); got != "newer" { t.Fatalf("C tag value = %q, want %q: newer child was overwritten — the `<` guard did not hold", got, "newer") } } func TestPropagateAll(t *testing.T) { r := setupTestRepo(t) // Create linear chain A→B→C ridA := makeCheckin(t, r, 0, "a.txt", "content A", "commit A") ridB := makeCheckin(t, r, ridA, "b.txt", "content B", "commit B") ridC := makeCheckin(t, r, ridB, "c.txt", "content C", "commit C") // Add propagating "branch" tag to A with value "feature" _, err := tag.AddTag(r, tag.TagOpts{ TargetRID: libfossil.FslID(ridA), TagName: "branch", TagType: tag.TagPropagating, Value: "feature", User: "testuser", Time: time.Now().UTC(), }) if err != nil { t.Fatalf("tag.AddTag: %v", err) } // Verify B and C have propagated tags initially var countB, countC int r.DB().QueryRow(` SELECT COUNT(*) FROM tagxref JOIN tag ON tag.tagid = tagxref.tagid WHERE tag.tagname = 'branch' AND tagxref.rid = ? AND srcid = 0 `, ridB).Scan(&countB) r.DB().QueryRow(` SELECT COUNT(*) FROM tagxref JOIN tag ON tag.tagid = tagxref.tagid WHERE tag.tagname = 'branch' AND tagxref.rid = ? AND srcid = 0 `, ridC).Scan(&countC) if countB != 1 || countC != 1 { t.Fatalf("initial setup check: B has %d tags, C has %d tags, want both = 1", countB, countC) } // Clear propagated tags from B and C to simulate incomplete propagation if _, err := r.DB().Exec("DELETE FROM tagxref WHERE rid=? AND srcid=0", ridB); err != nil { t.Fatalf("clear B tags: %v", err) } if _, err := r.DB().Exec("DELETE FROM tagxref WHERE rid=? AND srcid=0", ridC); err != nil { t.Fatalf("clear C tags: %v", err) } // Verify cleared r.DB().QueryRow(` SELECT COUNT(*) FROM tagxref JOIN tag ON tag.tagid = tagxref.tagid WHERE tag.tagname = 'branch' AND tagxref.rid = ? AND srcid = 0 `, ridB).Scan(&countB) r.DB().QueryRow(` SELECT COUNT(*) FROM tagxref JOIN tag ON tag.tagid = tagxref.tagid WHERE tag.tagname = 'branch' AND tagxref.rid = ? AND srcid = 0 `, ridC).Scan(&countC) if countB != 0 || countC != 0 { t.Fatalf("after clear: B has %d tags, C has %d tags, want both = 0", countB, countC) } // Call PropagateAll on A to re-propagate if err := tag.PropagateAll(r.DB(), libfossil.FslID(ridA)); err != nil { t.Fatalf("tag.PropagateAll: %v", err) } // Verify B has propagated branch=feature tag (srcid=0) var srcidB, tagtypeB int var valueB string err = r.DB().QueryRow(` SELECT srcid, tagtype, value FROM tagxref JOIN tag ON tag.tagid = tagxref.tagid WHERE tag.tagname = 'branch' AND tagxref.rid = ? `, ridB).Scan(&srcidB, &tagtypeB, &valueB) if err != nil { t.Fatalf("tagxref query for B: %v", err) } if srcidB != 0 { t.Errorf("B srcid = %d, want 0 (propagated)", srcidB) } if tagtypeB != tag.TagPropagating { t.Errorf("B tagtype = %d, want %d", tagtypeB, tag.TagPropagating) } if valueB != "feature" { t.Errorf("B value = %q, want %q", valueB, "feature") } // Verify C has propagated branch=feature tag (srcid=0) var srcidC, tagtypeC int var valueC string err = r.DB().QueryRow(` SELECT srcid, tagtype, value FROM tagxref JOIN tag ON tag.tagid = tagxref.tagid WHERE tag.tagname = 'branch' AND tagxref.rid = ? `, ridC).Scan(&srcidC, &tagtypeC, &valueC) if err != nil { t.Fatalf("tagxref query for C: %v", err) } if srcidC != 0 { t.Errorf("C srcid = %d, want 0 (propagated)", srcidC) } if tagtypeC != tag.TagPropagating { t.Errorf("C tagtype = %d, want %d", tagtypeC, tag.TagPropagating) } if valueC != "feature" { t.Errorf("C value = %q, want %q", valueC, "feature") } }