Abhishek Kumar <abhishekkumar8222@xxxxxxxxx> writes:
> On Sun, Jan 03, 2021 at 07:30:35PM -0700, Antonio Russo wrote:
>> t6016 manually reconstructs git log --graph output by using the reported
>> commit hashes from `git rev-parse`. Each tag is converted into an
>> environment variable manually, and then `echo`-ed to an expected output
>> file, which is in turn compared to the actual output.
>>
>> The expected output is difficult to read and write, because, e.g.,
>> each line of output must be prefaced with echo, quoted, and properly
>> escaped. Additionally, the test is sensitive to trailing whitespace,
>> which may potentially be removed from graph log output in the future.
>>
>> In order to reduce duplication, ease troubleshooting of failed tests by
>> improving readability, and ease the addition of more tests to this file,
>> port the operations to `lib-log-graph.sh`, which is already used in
>> several other tests, e.g., t4215. Give all merges a simple commit
>> message, and use a common `check_graph` macro taking a heredoc of the
>> expected output which does not required extensive escaping.
>>
>
> Glad to see others using `lib-log-graph.sh` as well!
>
> The changes look alright to me but maybe you could have split the two
> changes into two different commits: Using tags directly instead of
> environment variables and using `check_graph` instead of manually
> `echo`-ing to an expected output and comparing with the actual output.
Perhaps. Also I am wondering if the tests still need to create tags
after this change---isn't the output all matched by the commit title
now?
That is ...
>> . ./test-lib.sh
>> +. "$TEST_DIRECTORY"/lib-log-graph.sh
>> +
>> +check_graph () {
>> + cat >expect &&
>> + lib_test_cmp_graph --format=%s "$@"
>> +}
... this only shows the title, and ...
>> - git merge B C &&
>> + git merge B C -m A4 &&
... that is why we need to have the title here.
>> git tag A4 &&
Now, does this A4 used anywhere?
>> - # Store commit names in variables for later use
>> - A1=$(git rev-parse --verify A1) &&
>> - A2=$(git rev-parse --verify A2) &&
>> - A3=$(git rev-parse --verify A3) &&
>> - A4=$(git rev-parse --verify A4) &&
It certainly used to be needed here, but ...
>> + check_graph --all <<-\EOF
>> + * A7
>> + * A6
>> + |\
>> + | * C4
>> + | * C3
>> + * | A5
>> + | |
>> + | \
>> + *-. | A4
... not anymore in the new version.
>> + |\ \|
>> + | | * C2
>> + | | * C1
>> + | * | B2
>> + | * | B1
>> + * | | A3
>> + | |/
>> + |/|
>> + * | A2
>> + |/
>> + * A1
>> + EOF
>> +'
>>
