On 2020-04-19 14:30:19+0200, Martin Ågren <martin.agren@xxxxxxxxx> wrote: > On Sun, 19 Apr 2020 at 13:05, Đoàn Trần Công Danh <congdanhqx@xxxxxxxxx> wrote: > > --- a/t/t4254-am-corrupt.sh > > +++ b/t/t4254-am-corrupt.sh > > @@ -69,9 +69,11 @@ test_expect_success "NUL in commit message's body" ' > > grep "a NUL byte in commit log message not allowed" err > > ' > > > > -test_expect_failure "NUL in commit message's header" ' > > +test_expect_success "NUL in commit message's header" ' > > test_when_finished "git am --abort" && > > write_nul_patch subject >subject.patch && > > + test_must_fail git mailinfo msg patch <subject.patch 2>err && > > + grep "a NUL byte in Subject is not allowed" err && > > test_must_fail git am subject.patch 2>err && > > grep "a NUL byte in Subject is not allowed" err > > ' > > We used to fail for some reason and it's sort of clear from the size > of the test what that reason is. Now that we flip "failure" to "success" > we can add some more stuff here that works. Makes sense. Of course, > somewhere is a line for stuffing too much into the test, i.e., you'll > reach a point where you should have separate tests, but I think this is > ok. I'm not really keen to check the "git-mailinfo" failure here. Since, another developer may come and decide that we should keep the mailinfo as is, i.e. keep NUL character in Subject, Author, Email, etc... And let's git-commit complains instead. Adding a check for git-mailinfo here may limit future development. Other's feedback is welcomed. -- Danh
