Three t4150 tests does not work as expected

Hi,

I noticed that the following three t4150 tests probably does not work as expected:

- record as an empty commit when meeting e-mail message that lacks a patch
- record an empty patch as an empty commit in the middle of an am session
- create an non-empty commit when the index IS changed though "--allow-empty" is given

All of them does something like:

	git show HEAD --format="%B" >actual
	grep -f actual expected

While the 'actual' file usually contains something like (four lines between the 'cut' markers):

-------------------------- cut -------------------------- empty commit

-- 2.45.1

-------------------------- cut --------------------------

IOW, there are two empty lines there. Because of that the grep gets an empty RE from the 'actual' file and so it matches everything that comes in the 'expected' file.

Is this the expected behavior?

Thank you.

Marcel Telka <marcel@telka.sk> writes:

Wow, these commands look nonsensical in as many counts as there are lines in there X-< [*].

It seems that they came from two patches, namely

 7c096b8d (am: support --empty=<option> to handle empty patches, 2021-12-09)
 9e7e41bf (am: support --allow-empty to record specific empty patches, 2021-12-09)

Most likely the author of these lines were confused and meant to use test_cmp (which by the way should always compare actual against expect, i.e. "test_cmp expect actual", because when the actual output differs, that is the order of the diff we want to see in the output to diagnose the breakage). I especially suspect that the author did not mean "read patterns from this file", but was confused to think that 'f' implies "match literally" somehow (the correct spelling of that fixed-strings option is -F but still it does the wrong thing reading from the file---it does not even ensure that all patterns listed in the file matches).

I am not saying (as I did not check) that doing just "grep -f A B" -> "test_cmp B A" woudl make things work. I do not think I trust that these tests added by the same patch have a sane expectations X-<. The outcome these tests try to verify may also need to be corrected.

[Footnote] 

 * To start with, any dashed option like --format=%B should come
   before a revision argument HEAD.  There are many other
   antipatterns in this test script, which should be cleaned up.