|
| 1 | +import { describe, test, expect } from "bun:test" |
| 2 | +import { parseDiffForMarkerWarnings } from "./analyze" |
| 3 | + |
| 4 | +// Helper to create a unified diff string from lines |
| 5 | +function makeDiff(hunks: string): string { |
| 6 | + return `diff --git a/file.ts b/file.ts |
| 7 | +index abc1234..def5678 100644 |
| 8 | +--- a/file.ts |
| 9 | ++++ b/file.ts |
| 10 | +${hunks}` |
| 11 | +} |
| 12 | + |
| 13 | +describe("parseDiffForMarkerWarnings", () => { |
| 14 | + test("returns no warnings for empty diff", () => { |
| 15 | + expect(parseDiffForMarkerWarnings("file.ts", "")).toEqual([]) |
| 16 | + expect(parseDiffForMarkerWarnings("file.ts", " \n ")).toEqual([]) |
| 17 | + }) |
| 18 | + |
| 19 | + test("added code inside added markers — no warning", () => { |
| 20 | + const diff = makeDiff( |
| 21 | + `@@ -10,3 +10,5 @@ |
| 22 | + const existing = true |
| 23 | ++// altimate_change start — new feature |
| 24 | ++const custom = true |
| 25 | ++// altimate_change end |
| 26 | + const more = true`, |
| 27 | + ) |
| 28 | + expect(parseDiffForMarkerWarnings("file.ts", diff)).toEqual([]) |
| 29 | + }) |
| 30 | + |
| 31 | + test("added code without markers — warning", () => { |
| 32 | + const diff = makeDiff( |
| 33 | + `@@ -10,3 +10,4 @@ |
| 34 | + const existing = true |
| 35 | ++const unmarked = true |
| 36 | + const more = true`, |
| 37 | + ) |
| 38 | + const warnings = parseDiffForMarkerWarnings("file.ts", diff) |
| 39 | + expect(warnings).toHaveLength(1) |
| 40 | + expect(warnings[0].file).toBe("file.ts") |
| 41 | + expect(warnings[0].context).toContain("unmarked") |
| 42 | + }) |
| 43 | + |
| 44 | + test("REGRESSION: added code inside existing (context) markers — no warning", () => { |
| 45 | + // This is the exact bug that caused the upgrade indicator leak. |
| 46 | + // Markers are context lines (already committed), new code is added inside. |
| 47 | + const diff = makeDiff( |
| 48 | + `@@ -10,4 +10,5 @@ |
| 49 | + const existing = true |
| 50 | + // altimate_change start — existing feature |
| 51 | ++const newCodeInsideExistingBlock = true |
| 52 | + // altimate_change end |
| 53 | + const more = true`, |
| 54 | + ) |
| 55 | + expect(parseDiffForMarkerWarnings("file.ts", diff)).toEqual([]) |
| 56 | + }) |
| 57 | + |
| 58 | + test("REGRESSION: context marker end followed by added code — warning", () => { |
| 59 | + // New code added AFTER an existing marker block should be flagged. |
| 60 | + const diff = makeDiff( |
| 61 | + `@@ -10,4 +10,5 @@ |
| 62 | + // altimate_change start — block A |
| 63 | + const blockA = true |
| 64 | + // altimate_change end |
| 65 | ++const outsideBlock = true |
| 66 | + const existing = true`, |
| 67 | + ) |
| 68 | + const warnings = parseDiffForMarkerWarnings("file.ts", diff) |
| 69 | + expect(warnings).toHaveLength(1) |
| 70 | + expect(warnings[0].context).toContain("outsideBlock") |
| 71 | + }) |
| 72 | + |
| 73 | + test("REGRESSION: context marker start, added code, context marker end — no warning", () => { |
| 74 | + // Entire marker block is pre-existing (context), only the inner code is new. |
| 75 | + const diff = makeDiff( |
| 76 | + `@@ -8,4 +8,5 @@ |
| 77 | + // altimate_change start - yolo mode visual indicator |
| 78 | + import { Flag } from "@/flag/flag" |
| 79 | + // altimate_change end |
| 80 | ++import { UpgradeIndicator } from "../../component/upgrade-indicator" |
| 81 | + const next = true`, |
| 82 | + ) |
| 83 | + // The import line is skipped by the "import " heuristic, so no warning |
| 84 | + expect(parseDiffForMarkerWarnings("file.ts", diff)).toEqual([]) |
| 85 | + }) |
| 86 | + |
| 87 | + test("REGRESSION: non-import code inside existing context markers — no warning", () => { |
| 88 | + // Verifies the fix works independently of the import heuristic |
| 89 | + const diff = makeDiff( |
| 90 | + `@@ -10,4 +10,5 @@ |
| 91 | + const existing = true |
| 92 | + // altimate_change start — custom feature |
| 93 | ++const customCode = doSomething() |
| 94 | + // altimate_change end |
| 95 | + const more = true`, |
| 96 | + ) |
| 97 | + expect(parseDiffForMarkerWarnings("file.ts", diff)).toEqual([]) |
| 98 | + }) |
| 99 | + |
| 100 | + test("REGRESSION: JSX comment markers on context lines — no warning", () => { |
| 101 | + // JSX uses {/* altimate_change start ... */} syntax |
| 102 | + const diff = makeDiff( |
| 103 | + `@@ -95,4 +95,5 @@ |
| 104 | + {/* altimate_change start — upgrade indicator */} |
| 105 | ++<UpgradeIndicator /> |
| 106 | + {/* altimate_change end */} |
| 107 | + </box>`, |
| 108 | + ) |
| 109 | + expect(parseDiffForMarkerWarnings("file.ts", diff)).toEqual([]) |
| 110 | + }) |
| 111 | + |
| 112 | + test("multiple hunks — marker state resets at hunk boundary", () => { |
| 113 | + // Each hunk starts fresh, marker state should NOT carry across hunks |
| 114 | + // since different parts of the file may have different marker context. |
| 115 | + const diff = makeDiff( |
| 116 | + `@@ -5,3 +5,4 @@ |
| 117 | + // altimate_change start — block 1 |
| 118 | ++const inBlock = true |
| 119 | + // altimate_change end |
| 120 | +@@ -50,3 +51,4 @@ |
| 121 | + const existing = true |
| 122 | ++const unmarkedInSecondHunk = true |
| 123 | + const more = true`, |
| 124 | + ) |
| 125 | + const warnings = parseDiffForMarkerWarnings("file.ts", diff) |
| 126 | + expect(warnings).toHaveLength(1) |
| 127 | + expect(warnings[0].context).toContain("unmarkedInSecondHunk") |
| 128 | + }) |
| 129 | + |
| 130 | + test("marker state from hunk 1 does not leak into hunk 2", () => { |
| 131 | + // If hunk 1 ends inside a marker block (start without end in context), |
| 132 | + // hunk 2 should NOT inherit that state. |
| 133 | + const diff = makeDiff( |
| 134 | + `@@ -5,3 +5,4 @@ |
| 135 | + // altimate_change start — block 1 |
| 136 | ++const inBlock = true |
| 137 | + const moreInBlock = true |
| 138 | +@@ -80,3 +81,4 @@ |
| 139 | + const unrelated = true |
| 140 | ++const shouldBeWarned = true |
| 141 | + const end = true`, |
| 142 | + ) |
| 143 | + const warnings = parseDiffForMarkerWarnings("file.ts", diff) |
| 144 | + expect(warnings).toHaveLength(1) |
| 145 | + expect(warnings[0].context).toContain("shouldBeWarned") |
| 146 | + }) |
| 147 | + |
| 148 | + test("import lines are skipped even without markers", () => { |
| 149 | + const diff = makeDiff( |
| 150 | + `@@ -1,3 +1,4 @@ |
| 151 | + import { existing } from "./existing" |
| 152 | ++import { NewThing } from "./new-thing" |
| 153 | + const x = 1`, |
| 154 | + ) |
| 155 | + expect(parseDiffForMarkerWarnings("file.ts", diff)).toEqual([]) |
| 156 | + }) |
| 157 | + |
| 158 | + test("export lines are skipped even without markers", () => { |
| 159 | + const diff = makeDiff( |
| 160 | + `@@ -1,3 +1,4 @@ |
| 161 | + const x = 1 |
| 162 | ++export { x } |
| 163 | + const y = 2`, |
| 164 | + ) |
| 165 | + expect(parseDiffForMarkerWarnings("file.ts", diff)).toEqual([]) |
| 166 | + }) |
| 167 | + |
| 168 | + test("comment-only lines are skipped (not TODOs)", () => { |
| 169 | + const diff = makeDiff( |
| 170 | + `@@ -1,3 +1,4 @@ |
| 171 | + const x = 1 |
| 172 | ++// this is a harmless comment |
| 173 | + const y = 2`, |
| 174 | + ) |
| 175 | + expect(parseDiffForMarkerWarnings("file.ts", diff)).toEqual([]) |
| 176 | + }) |
| 177 | + |
| 178 | + test("TODO comments are NOT skipped", () => { |
| 179 | + const diff = makeDiff( |
| 180 | + `@@ -1,3 +1,4 @@ |
| 181 | + const x = 1 |
| 182 | ++// TODO: implement custom feature |
| 183 | + const y = 2`, |
| 184 | + ) |
| 185 | + const warnings = parseDiffForMarkerWarnings("file.ts", diff) |
| 186 | + expect(warnings).toHaveLength(1) |
| 187 | + }) |
| 188 | + |
| 189 | + test("empty added lines are skipped", () => { |
| 190 | + const diff = makeDiff( |
| 191 | + `@@ -1,3 +1,4 @@ |
| 192 | + const x = 1 |
| 193 | ++ |
| 194 | + const y = 2`, |
| 195 | + ) |
| 196 | + expect(parseDiffForMarkerWarnings("file.ts", diff)).toEqual([]) |
| 197 | + }) |
| 198 | + |
| 199 | + test("deleted lines don't affect marker state or line numbers", () => { |
| 200 | + const diff = makeDiff( |
| 201 | + `@@ -5,5 +5,5 @@ |
| 202 | + // altimate_change start — feature |
| 203 | +-const oldCode = true |
| 204 | ++const newCode = true |
| 205 | + // altimate_change end |
| 206 | + const next = true`, |
| 207 | + ) |
| 208 | + expect(parseDiffForMarkerWarnings("file.ts", diff)).toEqual([]) |
| 209 | + }) |
| 210 | + |
| 211 | + test("line number in warning matches diff hunk position", () => { |
| 212 | + const diff = makeDiff( |
| 213 | + `@@ -42,3 +42,4 @@ |
| 214 | + const existing = true |
| 215 | ++const unmarked = true |
| 216 | + const more = true`, |
| 217 | + ) |
| 218 | + const warnings = parseDiffForMarkerWarnings("file.ts", diff) |
| 219 | + expect(warnings).toHaveLength(1) |
| 220 | + expect(warnings[0].line).toBe(43) |
| 221 | + }) |
| 222 | + |
| 223 | + test("context truncated to 80 chars in warning", () => { |
| 224 | + const longLine = "x".repeat(120) |
| 225 | + const diff = makeDiff( |
| 226 | + `@@ -1,3 +1,4 @@ |
| 227 | + const a = 1 |
| 228 | ++const ${longLine} = true |
| 229 | + const b = 2`, |
| 230 | + ) |
| 231 | + const warnings = parseDiffForMarkerWarnings("file.ts", diff) |
| 232 | + expect(warnings).toHaveLength(1) |
| 233 | + expect(warnings[0].context.length).toBeLessThanOrEqual(80) |
| 234 | + }) |
| 235 | + |
| 236 | + test("only first unmarked line is reported per file", () => { |
| 237 | + const diff = makeDiff( |
| 238 | + `@@ -1,3 +1,5 @@ |
| 239 | + const a = 1 |
| 240 | ++const first = true |
| 241 | ++const second = true |
| 242 | + const b = 2`, |
| 243 | + ) |
| 244 | + const warnings = parseDiffForMarkerWarnings("file.ts", diff) |
| 245 | + expect(warnings).toHaveLength(1) |
| 246 | + expect(warnings[0].context).toContain("first") |
| 247 | + }) |
| 248 | + |
| 249 | + test("real-world scenario: upgrade indicator in footer.tsx", () => { |
| 250 | + // Simulates the exact diff that leaked: UpgradeIndicator added to |
| 251 | + // session footer without markers, adjacent to existing yolo marker block. |
| 252 | + const diff = makeDiff( |
| 253 | + `@@ -8,4 +8,6 @@ |
| 254 | + // altimate_change start - yolo mode visual indicator |
| 255 | + import { Flag } from "@/flag/flag" |
| 256 | + // altimate_change end |
| 257 | ++// altimate_change start — upgrade indicator import |
| 258 | ++import { UpgradeIndicator } from "../../component/upgrade-indicator" |
| 259 | ++// altimate_change end |
| 260 | +
|
| 261 | +@@ -96,4 +98,6 @@ |
| 262 | + </Switch> |
| 263 | ++ {/* altimate_change start — upgrade indicator in session footer */} |
| 264 | ++ <UpgradeIndicator /> |
| 265 | ++ {/* altimate_change end */} |
| 266 | + </box>`, |
| 267 | + ) |
| 268 | + expect(parseDiffForMarkerWarnings("footer.tsx", diff)).toEqual([]) |
| 269 | + }) |
| 270 | + |
| 271 | + test("real-world scenario: unmarked upgrade indicator would be caught", () => { |
| 272 | + // Same scenario but WITHOUT markers — should flag |
| 273 | + const diff = makeDiff( |
| 274 | + `@@ -8,4 +8,5 @@ |
| 275 | + // altimate_change start - yolo mode visual indicator |
| 276 | + import { Flag } from "@/flag/flag" |
| 277 | + // altimate_change end |
| 278 | ++import { UpgradeIndicator } from "../../component/upgrade-indicator" |
| 279 | +
|
| 280 | +@@ -96,4 +97,5 @@ |
| 281 | + </Switch> |
| 282 | ++ <UpgradeIndicator /> |
| 283 | + </box>`, |
| 284 | + ) |
| 285 | + const warnings = parseDiffForMarkerWarnings("footer.tsx", diff) |
| 286 | + // import is skipped by heuristic, but <UpgradeIndicator /> is flagged |
| 287 | + expect(warnings).toHaveLength(1) |
| 288 | + expect(warnings[0].context).toContain("UpgradeIndicator") |
| 289 | + }) |
| 290 | +}) |
0 commit comments