b563e86e
test(rewriter): deterministic concurrent-modification path
a73x 2026-04-29 05:51
Commit message
internal/rewriter/rewriter.go
| Old | New | ||
|---|---|---|---|
| @@ -8,6 +8,7 @@ import ( | |||
| 8 | "path/filepath" | 8 | "path/filepath" |
| 9 | "regexp" | 9 | "regexp" |
| 10 | "strings" | 10 | "strings" |
| 11 | "time" | ||
| 11 | ) | 12 | ) |
| 12 | 13 | ||
| 13 | var ErrConcurrentModification = errors.New("file modified concurrently") | 14 | var ErrConcurrentModification = errors.New("file modified concurrently") |
| @@ -84,21 +85,29 @@ func StampUUIDs(path string, stamps map[int]string) error { | |||
| 84 | return err | 85 | return err |
| 85 | } | 86 | } |
| 86 | 87 | ||
| 87 | // mtime check before atomic rename (deterministic test for this comes in Task 7) | 88 | return atomicReplace(path, tmpPath, info.Mode().Perm(), mtimeBefore) |
| 88 | info2, err := os.Stat(path) | 89 | } |
| 90 | |||
| 91 | // atomicReplace stat-checks path against mtimeBefore. If the file was modified | ||
| 92 | // concurrently, returns ErrConcurrentModification and removes tmpPath. Otherwise | ||
| 93 | // renames tmpPath over path. The mode argument is currently unused (the caller | ||
| 94 | // already chmods tmpPath before close), but is included to keep this helper's | ||
| 95 | // signature self-contained for direct testing. | ||
| 96 | func atomicReplace(path, tmpPath string, mode os.FileMode, mtimeBefore time.Time) error { | ||
| 97 | info, err := os.Stat(path) | ||
| 89 | if err != nil { | 98 | if err != nil { |
| 90 | os.Remove(tmpPath) | 99 | os.Remove(tmpPath) |
| 91 | return err | 100 | return err |
| 92 | } | 101 | } |
| 93 | if !info2.ModTime().Equal(mtimeBefore) { | 102 | if !info.ModTime().Equal(mtimeBefore) { |
| 94 | os.Remove(tmpPath) | 103 | os.Remove(tmpPath) |
| 95 | return ErrConcurrentModification | 104 | return ErrConcurrentModification |
| 96 | } | 105 | } |
| 97 | |||
| 98 | if err := os.Rename(tmpPath, path); err != nil { | 106 | if err := os.Rename(tmpPath, path); err != nil { |
| 99 | os.Remove(tmpPath) | 107 | os.Remove(tmpPath) |
| 100 | return fmt.Errorf("rename: %w", err) | 108 | return fmt.Errorf("rename: %w", err) |
| 101 | } | 109 | } |
| 110 | _ = mode // currently unused; reserved for future modes that need post-rename Chmod | ||
| 102 | return nil | 111 | return nil |
| 103 | } | 112 | } |
| 104 | 113 | ||
internal/rewriter/rewriter_test.go
| Old | New | ||
|---|---|---|---|
| @@ -1,10 +1,12 @@ | |||
| 1 | package rewriter | 1 | package rewriter |
| 2 | 2 | ||
| 3 | import ( | 3 | import ( |
| 4 | "errors" | ||
| 4 | "os" | 5 | "os" |
| 5 | "path/filepath" | 6 | "path/filepath" |
| 6 | "strings" | 7 | "strings" |
| 7 | "testing" | 8 | "testing" |
| 9 | "time" | ||
| 8 | ) | 10 | ) |
| 9 | 11 | ||
| 10 | func TestStampUUIDsInsertsTagAfterClaude(t *testing.T) { | 12 | func TestStampUUIDsInsertsTagAfterClaude(t *testing.T) { |
| @@ -63,3 +65,20 @@ func TestStampUUIDsBlockComment(t *testing.T) { | |||
| 63 | t.Errorf("got:\n%s\nwant:\n%s", got, want) | 65 | t.Errorf("got:\n%s\nwant:\n%s", got, want) |
| 64 | } | 66 | } |
| 65 | } | 67 | } |
| 68 | |||
| 69 | func TestAtomicReplaceAbortsOnStaleMtime(t *testing.T) { | ||
| 70 | dir := t.TempDir() | ||
| 71 | path := filepath.Join(dir, "foo.go") | ||
| 72 | if err := os.WriteFile(path, []byte("hi\n"), 0o644); err != nil { | ||
| 73 | t.Fatal(err) | ||
| 74 | } | ||
| 75 | tmpPath := filepath.Join(dir, "tmp") | ||
| 76 | if err := os.WriteFile(tmpPath, []byte("bye\n"), 0o644); err != nil { | ||
| 77 | t.Fatal(err) | ||
| 78 | } | ||
| 79 | stale := time.Now().Add(-time.Hour) | ||
| 80 | err := atomicReplace(path, tmpPath, 0o644, stale) | ||
| 81 | if !errors.Is(err, ErrConcurrentModification) { | ||
| 82 | t.Fatalf("got %v, want ErrConcurrentModification", err) | ||
| 83 | } | ||
| 84 | } | ||