refactor: share one atomic write path for generated files

README.md and site/data.json were written with os.Create/os.WriteFile,
so a write that failed midway left the repo front page truncated, while
history.jsonl already used a temp-file-plus-rename. Extract that pattern
into atomicWriteFile and route all three writers through it.

Also sort snapshots by date when reading history.jsonl: delta windows
pick the newest snapshot inside the window by scan order, which silently
produces wrong deltas if a hand edit or a merge of two concurrent runs
interleaves lines.

Cover the README renderer, which had no test beyond sanitizeCell, and
name the generated paths as constants instead of repeating literals.
Coverage 65.1% -> 75.7%.
This commit is contained in:
tiennm99 committed 2026-09-11 14:29:49 +07:00
1 parent 1262e3435b
commit fde3ae7efd
8 files changed
+280 -62

No files matched your search

+38
View File
@@ -0,0 +1,38 @@
package main
import (
"io"
"os"
)
// atomicWriteFile writes through a temp file in the same directory and renames
// it over path, so a crash or a failed write never leaves a generated file
// truncated or half-written. The callback receives the temp file as an
// io.Writer, which suits streaming producers (templates, JSON encoders)
// without buffering the whole payload in memory.
//
// Errors from Sync and Close are reported too: on a write path they are the
// difference between "data reached the disk" and silent loss.
func atomicWriteFile(path string, write func(io.Writer) error) error {
tmp := path + ".tmp"
f, err := os.Create(tmp)
if err != nil {
return err
}
writeErr := write(f)
if syncErr := f.Sync(); syncErr != nil && writeErr == nil {
writeErr = syncErr
}
if closeErr := f.Close(); closeErr != nil && writeErr == nil {
writeErr = closeErr
}
if writeErr != nil {
_ = os.Remove(tmp) // best-effort cleanup; the write error is what matters
return writeErr
}
return os.Rename(tmp, path)
}
+67
View File
@@ -0,0 +1,67 @@
package main
import (
"errors"
"io"
"os"
"testing"
)
func TestAtomicWriteFile_WritesAndLeavesNoTemp(t *testing.T) {
path := t.TempDir() + "/out.txt"
if err := atomicWriteFile(path, func(w io.Writer) error {
_, err := io.WriteString(w, "hello")
return err
}); err != nil {
t.Fatalf("atomicWriteFile: %v", err)
}
got, err := os.ReadFile(path)
if err != nil {
t.Fatalf("ReadFile: %v", err)
}
if string(got) != "hello" {
t.Errorf("content = %q, want %q", got, "hello")
}
if _, err := os.Stat(path + ".tmp"); !os.IsNotExist(err) {
t.Errorf("temp file still present after success")
}
}
func TestAtomicWriteFile_FailedWriteKeepsPreviousContent(t *testing.T) {
// The point of the temp-file dance: a generated file (README.md,
// history.jsonl) must never be truncated by a write that fails midway.
path := t.TempDir() + "/out.txt"
if err := os.WriteFile(path, []byte("previous"), 0o644); err != nil {
t.Fatalf("seed file: %v", err)
}
boom := errors.New("boom")
err := atomicWriteFile(path, func(w io.Writer) error {
_, _ = io.WriteString(w, "partial")
return boom
})
if !errors.Is(err, boom) {
t.Fatalf("err = %v, want %v", err, boom)
}
got, err := os.ReadFile(path)
if err != nil {
t.Fatalf("ReadFile: %v", err)
}
if string(got) != "previous" {
t.Errorf("content = %q, want the file left untouched", got)
}
if _, err := os.Stat(path + ".tmp"); !os.IsNotExist(err) {
t.Errorf("temp file left behind after a failed write")
}
}
func TestAtomicWriteFile_UncreatableTempReportsError(t *testing.T) {
if err := atomicWriteFile(t.TempDir()+"/missing-dir/out.txt", func(w io.Writer) error {
return nil
}); err == nil {
t.Fatal("expected an error when the temp file cannot be created")
}
}
+21 -31
View File
@@ -2,8 +2,10 @@ package main
import (
"bufio"
"cmp"
"encoding/json"
"fmt"
"io"
"os"
"slices"
"time"
@@ -98,7 +100,16 @@ func readSnapshots(path string) ([]Snapshot, error) {
s.Stars = applyMigrations(s.Stars)
out = append(out, s)
}
return out, scanner.Err()
if err := scanner.Err(); err != nil {
return nil, err
}
// Delta windows pick the newest snapshot inside the window by scan order,
// so the slice must be date-ascending. Sort rather than trust the file:
// a hand edit or a merge of two concurrent runs can interleave lines.
slices.SortStableFunc(out, func(a, b Snapshot) int { return cmp.Compare(a.Date, b.Date) })
return out, nil
}
// applyMigrations rewrites any deprecated history keys to their current
@@ -156,38 +167,17 @@ func resolveCanonicalKey(key string) string {
return key
}
// writeSnapshots writes to a temp file then renames atomically so a crash
// mid-write never leaves history.jsonl truncated or partially written.
// writeSnapshots persists every snapshot as one JSON object per line.
func writeSnapshots(path string, snapshots []Snapshot) error {
tmp := path + ".tmp"
f, err := os.Create(tmp)
if err != nil {
return err
}
enc := json.NewEncoder(f)
writeErr := error(nil)
for _, s := range snapshots {
if err := enc.Encode(s); err != nil {
writeErr = err
break
return atomicWriteFile(path, func(w io.Writer) error {
enc := json.NewEncoder(w)
for _, s := range snapshots {
if err := enc.Encode(s); err != nil {
return err
}
}
}
if syncErr := f.Sync(); syncErr != nil && writeErr == nil {
writeErr = syncErr
}
// A failed close on a write path can hide lost data — surface it.
if closeErr := f.Close(); closeErr != nil && writeErr == nil {
writeErr = closeErr
}
if writeErr != nil {
_ = os.Remove(tmp) // best-effort cleanup; the write error is what matters
return writeErr
}
return os.Rename(tmp, path)
return nil
})
}
// computeDeltas returns stars-now minus stars-at-or-before-cutoff for each
+40
View File
@@ -393,3 +393,43 @@ func TestWriteSnapshots_AtomicWrite(t *testing.T) {
t.Errorf("temp file should not exist after successful write")
}
}
func TestReadSnapshots_SortsOutOfOrderDates(t *testing.T) {
// A hand edit or a merge of two concurrent updater runs can leave lines
// out of date order; delta windows depend on ascending order.
path := t.TempDir() + "/history.jsonl"
lines := `{"date":"2026-09-03","stars":{"org/repo":300}}
{"date":"2026-08-27","stars":{"org/repo":100}}
{"date":"2026-09-10","stars":{"org/repo":400}}
{"date":"2026-09-01","stars":{"org/repo":200}}
`
if err := os.WriteFile(path, []byte(lines), 0o644); err != nil {
t.Fatalf("seed file: %v", err)
}
got, err := readSnapshots(path)
if err != nil {
t.Fatalf("readSnapshots: %v", err)
}
want := []string{"2026-08-27", "2026-09-01", "2026-09-03", "2026-09-10"}
if len(got) != len(want) {
t.Fatalf("got %d snapshots, want %d", len(got), len(want))
}
for i, d := range want {
if got[i].Date != d {
t.Errorf("snapshot %d date = %s, want %s", i, got[i].Date, d)
}
}
// With the dates ordered, the 7-day base is 2026-09-03 (not the
// last line in the file), so the delta is 400-300.
fixed := time.Date(2026, 9, 10, 0, 0, 0, 0, time.UTC)
orig := timeNow
timeNow = func() time.Time { return fixed }
defer func() { timeNow = orig }()
deltas := computeDeltas(got, Snapshot{Date: "2026-09-10", Stars: map[string]int{"org/repo": 400}})
if deltas["org/repo"] != 100 {
t.Errorf("delta = %d, want 100", deltas["org/repo"])
}
}
+15 -6
View File
@@ -7,12 +7,21 @@ import (
"os"
)
// Paths the updater reads and writes, relative to the repository root.
const (
agentsPath = "data/agents.yml"
historyPath = "data/history.jsonl"
readmeTmpl = "templates/readme.tmpl"
readmePath = "README.md"
siteDataPath = "site/data.json"
)
func main() {
check := flag.Bool("check", false, "validate data/agents.yml offline (no network, no token) and exit")
flag.Parse()
if *check {
if err := runCheck("data/agents.yml"); err != nil {
if err := runCheck(agentsPath); err != nil {
log.Printf("check failed: %v", err)
os.Exit(1)
}
@@ -25,12 +34,12 @@ func main() {
}
func run() error {
agents, err := loadAgents("data/agents.yml")
agents, err := loadAgents(agentsPath)
if err != nil {
return err
}
if len(agents) == 0 {
return fmt.Errorf("no agents in data/agents.yml")
return fmt.Errorf("no agents in %s", agentsPath)
}
token := os.Getenv("GITHUB_TOKEN")
@@ -43,16 +52,16 @@ func run() error {
return err
}
snapshots, deltas7, deltas30, err := appendHistory("data/history.jsonl", stats)
snapshots, deltas7, deltas30, err := appendHistory(historyPath, stats)
if err != nil {
return err
}
if err := renderReadme("templates/readme.tmpl", "README.md", stats, deltas7); err != nil {
if err := renderReadme(readmeTmpl, readmePath, stats, deltas7); err != nil {
return err
}
if err := writeSiteData("site/data.json", stats, deltas7, deltas30, snapshots); err != nil {
if err := writeSiteData(siteDataPath, stats, deltas7, deltas30, snapshots); err != nil {
return err
}
+8 -16
View File
@@ -2,7 +2,7 @@ package main
import (
"fmt"
"os"
"io"
"path/filepath"
"strings"
"text/template"
@@ -84,22 +84,14 @@ func renderReadme(tmplPath, outPath string, stats []Stat, deltas map[string]int)
return err
}
f, err := os.Create(outPath)
if err != nil {
return err
}
execErr := tmpl.ExecuteTemplate(f, filepath.Base(tmplPath), map[string]any{
"Rows": rows,
"UpdatedAt": timeNow().UTC().Format("2006-01-02 15:04 UTC"),
"Total": len(rows),
"TopMover": topMover,
return atomicWriteFile(outPath, func(w io.Writer) error {
return tmpl.ExecuteTemplate(w, filepath.Base(tmplPath), map[string]any{
"Rows": rows,
"UpdatedAt": timeNow().UTC().Format("2006-01-02 15:04 UTC"),
"Total": len(rows),
"TopMover": topMover,
})
})
// A failed close on a write path can hide lost data — surface it.
if closeErr := f.Close(); closeErr != nil && execErr == nil {
execErr = closeErr
}
return execErr
}
// sanitizeCell makes a third-party repo description safe to embed in a
+84
View File
@@ -1,7 +1,10 @@
package main
import (
"os"
"strings"
"testing"
"time"
)
func TestSanitizeCell(t *testing.T) {
@@ -118,3 +121,84 @@ func TestSanitizeCell(t *testing.T) {
})
}
}
func TestRenderReadme_TableRowsAndCallout(t *testing.T) {
fixed := time.Date(2026, 9, 11, 3, 39, 0, 0, time.UTC)
orig := timeNow
timeNow = func() time.Time { return fixed }
defer func() { timeNow = orig }()
stats := []Stat{
{CanonicalKey: "org/big", NameWithOwner: "org/big", URL: "https://github.com/org/big",
Stars: 1_500_000, Language: "Go", PushedAt: fixed, Description: "huge | agent", Category: "cli"},
{CanonicalKey: "org/mid", NameWithOwner: "org/mid", URL: "https://github.com/org/mid",
Stars: 1234, Language: "Rust", PushedAt: fixed, Description: "mid agent", Category: "cli"},
{CanonicalKey: "org/small", NameWithOwner: "org/small", URL: "https://github.com/org/small",
Stars: 999, Language: "", PushedAt: fixed, Description: "small agent", Category: "cli"},
}
// org/small has no delta: its row must show the em dash, and it must not
// win the top-mover callout.
deltas := map[string]int{"org/big": 12, "org/mid": -3}
out := t.TempDir() + "/README.md"
if err := renderReadme("templates/readme.tmpl", out, stats, deltas); err != nil {
t.Fatalf("renderReadme: %v", err)
}
raw, err := os.ReadFile(out)
if err != nil {
t.Fatalf("ReadFile: %v", err)
}
got := string(raw)
want := []string{
"**Last updated:** 2026-09-11 03:39 UTC · **Tracked:** 3 repos",
"**Top 7-day mover:** [org/big](https://github.com/org/big) (+12 stars)",
"| 1 | [org/big](https://github.com/org/big) | 1.5M | +12 | Go | 2026-09-11 | huge \\| agent |",
"| 2 | [org/mid](https://github.com/org/mid) | 1.2k | -3 | Rust | 2026-09-11 | mid agent |",
"| 3 | [org/small](https://github.com/org/small) | 999 | — | | 2026-09-11 | small agent |",
}
for _, w := range want {
if !strings.Contains(got, w) {
t.Errorf("README missing line:\n%s\n--- got ---\n%s", w, got)
}
}
}
func TestRenderReadme_NoDeltasOmitsTopMover(t *testing.T) {
fixed := time.Date(2026, 9, 11, 3, 39, 0, 0, time.UTC)
orig := timeNow
timeNow = func() time.Time { return fixed }
defer func() { timeNow = orig }()
stats := []Stat{{CanonicalKey: "org/repo", NameWithOwner: "org/repo",
URL: "https://github.com/org/repo", Stars: 10, PushedAt: fixed, Category: "cli"}}
out := t.TempDir() + "/README.md"
if err := renderReadme("templates/readme.tmpl", out, stats, map[string]int{}); err != nil {
t.Fatalf("renderReadme: %v", err)
}
raw, err := os.ReadFile(out)
if err != nil {
t.Fatalf("ReadFile: %v", err)
}
if strings.Contains(string(raw), "Top 7-day mover") {
t.Error("top-mover callout rendered with no deltas available")
}
}
func TestRenderReadme_MissingTemplateDoesNotTouchOutput(t *testing.T) {
out := t.TempDir() + "/README.md"
if err := os.WriteFile(out, []byte("previous"), 0o644); err != nil {
t.Fatalf("seed file: %v", err)
}
if err := renderReadme("templates/does-not-exist.tmpl", out, nil, nil); err == nil {
t.Fatal("expected an error for a missing template")
}
raw, err := os.ReadFile(out)
if err != nil {
t.Fatalf("ReadFile: %v", err)
}
if string(raw) != "previous" {
t.Errorf("output = %q, want the existing README left intact", raw)
}
}
+7 -9
View File
@@ -2,7 +2,7 @@ package main
import (
"encoding/json"
"os"
"io"
)
// siteRow is one ranked repo in site/data.json, consumed by site/index.html.
@@ -55,13 +55,11 @@ func writeSiteData(path string, stats []Stat, deltas7, deltas30 map[string]int,
}
}
out, err := json.Marshal(siteData{
UpdatedAt: timeNow().UTC().Format("2006-01-02 15:04 UTC"),
Rows: rows,
History: history,
return atomicWriteFile(path, func(w io.Writer) error {
return json.NewEncoder(w).Encode(siteData{
UpdatedAt: timeNow().UTC().Format("2006-01-02 15:04 UTC"),
Rows: rows,
History: history,
})
})
if err != nil {
return err
}
return os.WriteFile(path, out, 0o644)
}