Repository navigation
test(git): reproduce upstream 810d424 and ae9721e on macOS - #56
Conversation
74e5c22 to
2b38802
Compare
axisrow
left a comment
There was a problem hiding this comment.
Верdict: approve — тесты апстрим-фиксов + Stryker-тулинг
Одобряю. PR тестовый и тулинговый: три новых набора тестов, воспроизводящих апстримные security-фиксы (810d424 git hardening, ae9721e линейные парсеры, 9246095 semver-guard), плюс конфигурация Stryker. Изменений исполняемого кода нет.
Проверено сверкой ожиданий с исходниками (без запуска):
- Ожидания
parseFilterConfig/getFilterOverrideArgs/withGitHardeningEnvсходятся сsrc/utils/git-hardening.ts: грани-кейсы (пустой scope,filter..x,user.alice.name, имя драйвера с=, git-crypt с относительным путём или путём внутри repo, mismatch ключа) корректны. - Ожидания
parseRemoteUrl(ssh/url формы, порт только для http(s), nested owners, набор reject-ов) сходятся сgit-remote.ts; линейныйsplit/filterдействительно заменяет бэктрекинг-regex, так что тест на 100k слэшей различает до/после фикса. - Ожидания
checkForUpdates/runGlobalPackageInstallсходятся с guard-омRELEASE_VERSIONвupdate-checker.ts; PATH-stub npm корректно сохраняется и восстанавливается. - fsmonitor-тест построен верно: hook прописывается в local config уже после commit, так что setup-команды его не триггерят, а
runGitидёт черезexecGitсGIT_HARDENING_ARGS. bun.lock— чисто аддитивный (только stryker-зависимости), удалений нет.describe.skipIf— устоявшийся в репо паттерн (см. git-hardening-exec.test.ts).
Наблюдения (не блокирующие, только по желанию):
upstream-git-linear.test.ts:115— мёртвый тернарникprotocol === :https: || protocol === :http: ? :github.com: : :github.com:(обе ветки одинаковые) — можно упростить до константы.- Временной ассерт
toBeLessThan(1000)у теста линейности: quadratic-сценарий даёт минуты, линейный — миллисекунды, так что порог можно поднять (например до 5000) без потери различающей силы — меньше риск флака на загруженной машине. - gitSync / BunRuntime / probe-boilerplate (~50 строк) продублированы в обоих upstream-git-* файлах — при случае можно вынести в общий test-helper.
- Реальные git-сьюты, вероятно, скиппятся в полном
bun test(CI) из-за утечки vi.mock child_process — это осознанный и задокументированный в комментариях компромисс, но стоит помнить: флагманский fsmonitor-тест в CI может не выполняться; реально он гоняется в изолированном прогоне файла и под vitest/stryker-конфигом. - upstream-update-checker.test.ts, в отличие от git-сьютов, не защищён skipIf-пробой от той же утечки mock-ов — но там падение было бы громким, а не тихим, так что допустимо.
- Новые real-git сьюты не гейтятся на win32 (в отличие от git-hardening-exec.test.ts):
/dev/nullи#!/bin/shне работают на Windows; для CI (ubuntu) и macOS разработки некритично. .gitignore: паттернreportsматчит одноимённые папки на любом уровне; корневойreports/был бы точнее.
Review follow-up on PR #56 (AO reviewer, non-blocking observations): - move gitSync / COMMIT_ENV / the runGit availability probe into the existing git-test-helpers.ts instead of duplicating them in both upstream-git-* suites - drop a dead conditional in the url-form parseRemoteUrl expectations - anchor the Stryker reports ignore to the repository root Co-Authored-By: Claude Code <noreply@anthropic.com>
Review follow-up on PR #56 (AO reviewer, non-blocking observations): - move gitSync / COMMIT_ENV / the runGit availability probe into the existing git-test-helpers.ts instead of duplicating them in both upstream-git-* suites - drop a dead conditional in the url-form parseRemoteUrl expectations - anchor the Stryker reports ignore to the repository root Co-Authored-By: Claude Code <noreply@anthropic.com>
43f5d33 to
42e9de3
Compare
|
Статус после ребейза на main (1cccb73): воспроизводящие коммиты этой ветки ( Разбор наблюдений ревьюера (все были помечены как необязательные): Исправлено
Отклонено
Проверка: 🤖 Generated with Claude Code |
axisrow
left a comment
There was a problem hiding this comment.
Verdict: approve — обновление PR: sanitize-тесты + дедуп boilerplate
Одобряю. С прошлого ревью (head 2b38802) в ветке два коммита: 1cccb73 — влитый squash из #57 с новым upstream-terminal-sanitize.test.ts (апстрим 3d94211, GHSA-8j6h-phgx-ww84), и 42e9de3 — вынос gitSync / COMMIT_ENV / probe в существующий git-test-helpers.ts. Исполняемого кода продуктовой логики PR не меняет — только тесты и тест-инфраструктура.
Проверено сверкой с исходниками (без запуска):
- Все 18 кейсов sanitize-теста трассированы по terminal-sanitize.ts: OSC/DCS/SOS/PM/APC до BEL / C1_ST / ESC-backslash, CSI с параметрами, intermediates и final byte (включая boundary: пробел-посредник, ? как приватный маркер, @ и ~ как final), незакрытые CSI/DCS до конца строки, C1-controls, charset-escape ESC ( B — ожидания совпадают с реализацией, включая хитрые случаи
a\x1b[2\x07Jb→ aJb иa\x1b/\x07bc→ abc. - renderOsc8Link: URL с control-символом возвращает plain label, безопасный — оборачивается в OSC 8 ✓ (isSafeHyperlinkUrl / SAFE_URL).
- Тест идёт через preRenderAllWidgets, то есть через реальную fix-строку renderer.ts:943, где sanitize применяется к выводу виджета; custom-text рендерит customText как есть, DEFAULT_SETTINGS лейблов не добавляет — путь чистый.
- Дедуп: чистое добавление трёх экспортов в существующий src/utils/tests/git-test-helpers.ts; isolateGitWorkingDirectory / expectGitExecOptions / gitCommandOf не тронуты, обе upstream-git-сьюты переключены на хелпер, локальные копии удалены.
- Новые импорты хелпера (runGit / clearGitCache из ../git) сдвигают момент первой загрузки git.ts в общем bun-процессе, но skip решает probeRealRunGitAvailable в рантайме — поведение не хуже прежнего. Хелпер не *.test.ts, в stryker-профиль не попадает.
Из наблюдений прошлого ревью учтено: мёртвый тернарник в upstream-git-linear.test.ts упрощён до константы, .gitignore-паттерн reports стал корневым /reports/, boilerplate вынесен в общий хелпер. Осталось как есть (и осталось необязательным): порог timing-ассерта 1000ms; upstream-git-сьюты без win32-гейта — для ubuntu CI и macOS-разработки некритично.
Примечание по процессу, не блокирующее: 1cccb73 несёт в сообщении метку (#57) — если PR #57 остаётся открытым с тем же содержимым, один из двух дублей стоит закрыть после мержа.
Batch 2: reproduce upstream git fixes on macOS (810d424, ae9721e)
TDD verification per the /fix pipeline: red on the pre-fix tree → green on main under both runners (bun test + vitest/Node) → mutation check (StrykerJS) → lint. Follows #55 (tooling + pilot). Depends on its Stryker config; branch is stacked on
verify/toolingwith--base main.Upstream CI only runs Linux; both fixes are security-relevant (GHSA-rf4f-gc2h-c9vp; quadratic parser input from repository config), so the wave proves them on macOS.
core.fsmonitorhook from repo config really executed during a status call (marker file created) @31983de4git-hardening.ts96.24% (178 killed + 1 timeout / 186)parseRemoteUrltook 5931–7073ms (limit 1000ms) on a 100k-slash remote URL @e8ef3cbgit-remote.ts93.20% (137 / 147)Survivors triage
All 17 surviving mutants across the two files were triaged as equivalent, each with a concrete reason:
git-hardening.ts (7)
relative === '' ||(2 mutants): implied by the right-hand side —''never starts with..and is never absolute.match?.[3]optional chaining: covered by the short-circuit!program(match is null only when program is undefined).<→<=,+→-) at the parse loop: out-of-range iterations produceundefinedscope/keyValue and are eaten by the existing guard.parseInt(env.GIT_CONFIG_COUNT ?? '0')→?? '': both yieldNaN→ start 0.existing < 0→existing <= 0: identical atexisting === 0(both start at 0).git-remote.ts (10)
new URL('')throws).filter(Boolean)(2 mutants): the second filter already drops what the first would.slice()beforesort():split()already returns a fresh array.sortcomparator mutants (2): sorting cannot change thefind()outcome — overlapping remote prefixes cannot both match a tracking ref (a prefix match requires the slash position to differ).upstreamRef === remote ||branch: unreachable through real git —@{upstream}always resolves toremote/branch, never a bare remote name.Exclusions / notes
.git filehalf of ae9721e is covered by upstream's own mock test and is not reproducible mock-free on this machine: modern engines no longer backtrack on the oldgitdir:\s*(.+)$pattern (2ms under bun and Node), and real git refuses to consume a whitespace-run.gitfile.Bun.spawnSyncunder bun because sibling suites mocknode:child_processand that mock leaks across files in the single-process bun runner; vitest/Node keeps file isolation and uses the realspawnSync.runGititself is broken (every call resolves to null), so both real-git suites proberunGitend to end once (fresh repo, dummy remote, non-empty output) and skip when that lookup is broken; vitest always runs them, as does an isolatedbun test <file>. The first CI run caught exactly this:runGit-dependent tests failed on Linux while the null-expectation ones passed. Pure-unit describes (parseFilterConfig, getFilterOverrideArgs, withGitHardeningEnv, parseRemoteUrl) run under every runner unconditionally.git-hardening-execLFS case (pre-existing, see verify(tooling): Stryker mutation tooling + upstream 9246095 pilot (batch 1) #55 notes). An occasional ItemsEditor flake in full runs is unrelated (green in isolation).Commits
🤖 Generated with Claude Code
Follow-up (42e9de3)
Rebased onto main: the reproduction commits and the Stryker/update-checker tooling already landed in main via #57, so this PR now carries only the review follow-up — dedupe of the real-git test boilerplate into
git-test-helpers.ts, a dead conditional removed,.gitignoreanchor fix. See the resolution comment for the addressed/declined reviewer observations.