On Fri, Aug 05 2022, Đoàn Trần Công Danh wrote: > On 2022-08-05 16:15:33+0200, Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote: > [...] >> +test_expect_success 'git hook run a hook with a bad shebang' ' >> + test_when_finished "rm -rf bad-hooks" && >> + mkdir bad-hooks && >> + write_script bad-hooks/test-hook "/bad/path/no/spaces" </dev/null && >> + >> + # TODO: We should emit the same (or at least a more similar) >> + # error on Windows and !Windows. See the OS-specific code in >> + # start_command() >> + if test_have_prereq !WINDOWS >> + then >> + cat >expect <<-\EOF >> + fatal: cannot run bad-hooks/test-hook: ... >> + EOF >> + else >> + cat >expect <<-\EOF >> + error: cannot spawn bad-hooks/test-hook: ... >> + EOF >> + fi && >> + test_expect_code 1 git \ >> + -c core.hooksPath=bad-hooks \ >> + hook run test-hook >out 2>err && >> + test_must_be_empty out && >> + sed -e "s/test-hook: .*/test-hook: .../" <err >actual && > > If we're using "sed" here, can we also s/cannot run/cannot spawn/ > in order to have the same expectation? > > Otherwise, the fix looks sane to me (obviously, since I also suggest > removing the line entirely). We could, but then we'd miss some weird regression where we changed the non-Windows message to "fatal: cannot spawn", which neither emit now. We could of course do the "sed"-ing in an identical "test_have_prereq" block, but I think we'd just be back to square one then, and it would be more readable to just "cat" what we expect to happen there. So I think I'd prefer to keep it as-is, it also makes a subsequent change where we unify these error messages more obvious, i.e. we'd keep the !WINDOWS branch of that if/else in thath case. > Reviewed-by: Đoàn Trần Công Danh <congdanhqx@xxxxxxxxx> Thanks!
