Re: [PATCH v3 1/3] t: add lib-crlf-messages.sh for messages containing CRLF
- From
Philippe Blain <levraiphilippeblain@gmail.com>
- Date
- Oct 14, 2020, 13:22 UTC
- Message-ID
- <ACDD60BE-34B2-4E3A-881A-E82F2BA39BF9@gmail.com>
- In-Reply-To
- <xmqqlfgbm6el.fsf@gitster.c.googlers.com>
Hi Junio,
Show 32 quoted lines
> Le 12 oct. 2020 à 18:22, Junio C Hamano <gitster@pobox.com> a écrit : > > "Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com> writes: > >> From: Philippe Blain <levraiphilippeblain@gmail.com> >> >> A following commit will fix a bug in the ref-filter API that causes >> commit and tag messages containing CRLF to be incorrectly parsed and >> displayed. >> >> Add a test library (t/lib-crlf-messages.sh) that creates refs with such >> commit messages, so that we can easily test that this bug does not >> appear in other commands in the future. >> ... >> The function `test_crlf_subject_body_and_contents` can be used to test >> that the `--format` option of `branch`, `tag`, `for-each-ref` and >> `log` correctly displays the subject, body and raw content of commits and >> tag messages. > > I am not sure about the wisdom of this arrangement. Surely you do > not want to write duplicated set-up for (existing) test scripts for > for-each-ref, branch and tag subcommands, assuming that these test > scripts are separated for subcommands they test. > > But you can have a single test script, that is differentiated from > all other test scripts by what it tests: having to deal with commits > that use CRLF. Then we do not have to add dot-includable test > library that lets various tests to create these same funny commits. > Instead, we can just do these as normal set-up step(s) for that > single test scripts, and then in that test scripts, verify what is > shown by various commands that share the underlying ref-filter > machinery. No?
Yes. I was thinking that it made more sense for the tests to be in existing test scripts, but if you feel a separate test script is warranted for these tests, I'll do that instead.
Thanks,
Philippe.