* Three t4150 tests does not work as expected
@ 2024-05-17 16:45 Marcel Telka
2024-05-17 17:55 ` Junio C Hamano
0 siblings, 1 reply; 2+ messages in thread
From: Marcel Telka @ 2024-05-17 16:45 UTC (permalink / raw
To: git
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 e-mail: marcel@telka.sk |
| homepage: http://telka.sk/ |
+-------------------------------------------+
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: Three t4150 tests does not work as expected
2024-05-17 16:45 Three t4150 tests does not work as expected Marcel Telka
@ 2024-05-17 17:55 ` Junio C Hamano
0 siblings, 0 replies; 2+ messages in thread
From: Junio C Hamano @ 2024-05-17 17:55 UTC (permalink / raw
To: Marcel Telka; +Cc: git
Marcel Telka <marcel@telka.sk> writes:
> All of them does something like:
>
> git show HEAD --format="%B" >actual
> grep -f actual expected
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.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-05-17 17:55 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-17 16:45 Three t4150 tests does not work as expected Marcel Telka
2024-05-17 17:55 ` Junio C Hamano
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.