Hi Chizoba On 10/10/2024 07:39, chizobajames21@xxxxxxxxx wrote:
From: Chizoba ODINAKA <chizobajames21@xxxxxxxxx> In pipes, the exit code of a chain of commands is determined by the downstream command.
I would perhaps say "final command" rather than "downstream command" as in a pipeline "cmd1 | cmd2 | cmd3" cmd2 and cmd3 are downstream of cmd1 but it is the exit code of cmd3 that will be used
In order not to loss the entire result code of tests, write output of upstreams into a file.
We're interested in checking the exit code of git, but not other commands so it would be helpful to make that clear. Usman's patch [1] has a good explanation of this.
This patch also changes instances of "grep" to "test_grep" so the commit message needs to explain the reason for that change which is that it gives a better debugging experience if the test fails.
The patch is looking pretty good, most of the conversions look correct. I've left a few comments below
[1] https://lore.kernel.org/git/bfff7937cd20737bb5a8791dc7492700b1d7881f.1728315124.git.gitgitgadget@xxxxxxxxx
test_expect_success 'replace the author' ' - git cat-file commit $HASH2 | grep "author A U Thor" && - R=$(git cat-file commit $HASH2 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) && - git cat-file commit $R | grep "author O Thor" && + git cat-file commit $HASH2 >actual && + test_grep "author A U Thor" actual && + git cat-file commit $HASH2 >actual &&
You don't need to repeat this command now that we are saving the output of "git cat-file commit $HASH2"
+ R=$(sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) && + git cat-file commit $R >actual && + test_grep "author O Thor" actual &&
test_expect_success 'push branch with replacement' ' - git cat-file commit $PARA3 | grep "author A U Thor" && - S=$(git cat-file commit $PARA3 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) && - git cat-file commit $S | grep "author O Thor" && + git cat-file commit $PARA3 >actual && + test_grep "author A U Thor" actual && + git cat-file commit $PARA3 >actual &&
We can drop this line for the same reason as above
+ S=$(sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) && + git cat-file commit $S >actual && + test_grep "author O Thor" actual &&
@@ -260,14 +291,14 @@ test_expect_success 'fetch branch with replacement' ' cd clone_dir && git fetch origin refs/heads/tofetch:refs/heads/parallel3 && git log --pretty=oneline parallel3 >output.txt && - ! grep $PARA3 output.txt && + ! test_grep $PARA3 output.txt &&
test_grep will print an error message the pattern does not match. In this case we expect it not to match so we want to print an error if it does match. We can do that with
test_grep ! $PARA3 output.txt &&
test_expect_success 'index-pack and replacements' ' - git --no-replace-objects rev-list --objects HEAD | + git --no-replace-objects rev-list --objects HEAD >actual && git --no-replace-objects pack-objects test- &&
This command has lost its input - you need to use '<actual' to get it to read output from "git rev-list". This test itself could probably do a better job of checking that index-pack does what we expect but that is outside the scope of this patch.
git index-pack test-*.pack '
Everything that I've not commented on looks correct to me. Best Wishes Phillip
