Hi Charvi
On 17/01/2021 03:39, Charvi Mendiratta wrote:
Hi Phillip and Taylor,
[...]
@@ -224,7 +223,7 @@ test_expect_success 'auto squash that matches longer sha1' '
git cat-file blob HEAD^:file1 >actual &&
test_cmp expect actual &&
git cat-file commit HEAD^ >commit &&
- grep squash commit >actual &&
+ grep "extra para" commit >actual &&
test_line_count = 1 actual
'
Worth checking that "squash" doesn't appear in an uncommented part of
actual? Or better yet, checking that "# squash ..." _does_ appear.
I.e., that we would leave this as:
- grep squash commit >actual &&
+ grep "^# squash" commit >actual &&
+ grep "extra para" commit >actual &&
This test is checking the message that gets committed, not the contents
of the file passed to the editor. I like the idea of checking that the
squash! line is indeed commented out, but we'd need to test it with
grep -v squash
It seems to me that you suggest to use 'grep -v squash' instead of
grep "^#squash".
That's correct
So I added to check the test as here:
- grep squash commit >actual &&
+ grep -v "squash" commit >actual &&
There is no need to redirect the output of this one - we don't expect
any output and the test will fail if there is so we want to see what the
output is.
+ grep "extra para" commit >actual &&
Looking at the changes to the tests in this commit it highlights the
fact that I don't think we ever check exactly what the user sees in
their editor. We do add such a test for the new `fixup -C` functionality
in a later patch but perhaps we should improve the test coverage of the
squash message presented to the user before then.
I agree and in this test it's now just checking if the commit message body of
"squash!" i.e line "extra para", is added in commit message or not. So, I am
doubtful if the above is the right way to test whether squash! line is commented
out or not , as "grep "extra para" commit >actual &&" will rewrite the
'actual' file.
The test above gives us reassurance that "squash! ..." does not make it
into the commit message which is important. We'd want a separate test to
check the message that is presented to the user however looking at patch
7 the test 'sequence of fixup, fixup -C & squash --signoff works' checks
that a "squash! ..." subject line gets commented out so I wouldn't worry
about an additional test here
Best Wishes
Phillip
Thanks and Regards,
Charvi
