From: Abhishek Kumar <abhishekkumar8222@gmail.com>
To: Antonio Russo <aerusso@aerusso.net>
Cc: git@vger.kernel.org, gitster@pobox.com
Subject: Re: [PATCH] t6016: move to lib-log-graph.sh framework
Date: Mon, 4 Jan 2021 11:56:14 +0530 [thread overview]
Message-ID: <X/K1BgP8tpsgNe2x@Abhishek-Arch> (raw)
In-Reply-To: <6dc37f6b-1afd-544d-126e-2be9422571c6@aerusso.net>
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.
Other contributors would have a better idea whether it's truly required
or not.
> Signed-off-by: Antonio Russo <aerusso@aerusso.net>
> ---
> t/t6016-rev-list-graph-simplify-history.sh | 354 ++++++++++-----------
> 1 file changed, 167 insertions(+), 187 deletions(-)
>
> This patch builds, and passes the test suite on travis-ci:
>
> https://travis-ci.org/github/aerusso/git/builds/752694578
>
> diff --git a/t/t6016-rev-list-graph-simplify-history.sh b/t/t6016-rev-list-graph-simplify-history.sh
> index f5e6e92f5b..f79df8b6d1 100755
> --- a/t/t6016-rev-list-graph-simplify-history.sh
> +++ b/t/t6016-rev-list-graph-simplify-history.sh
> @@ -8,6 +8,12 @@
> test_description='--graph and simplified history'
>
> . ./test-lib.sh
> +. "$TEST_DIRECTORY"/lib-log-graph.sh
> +
> +check_graph () {
> + cat >expect &&
> + lib_test_cmp_graph --format=%s "$@"
> +}
>
> test_expect_success 'set up rev-list --graph test' '
> # 3 commits on branch A
> @@ -28,7 +34,7 @@ test_expect_success 'set up rev-list --graph test' '
>
> # Octopus merge B and C into branch A
> git checkout A &&
> - git merge B C &&
> + git merge B C -m A4 &&
> git tag A4 &&
>
> test_commit A5 bar.txt &&
> @@ -38,81 +44,64 @@ test_expect_success 'set up rev-list --graph test' '
> test_commit C3 foo.txt &&
> test_commit C4 bar.txt &&
> git checkout A &&
> - git merge -s ours C &&
> + git merge -s ours C -m A6 &&
> git tag A6 &&
>
> - test_commit A7 bar.txt &&
> -
> - # 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) &&
> - A5=$(git rev-parse --verify A5) &&
> - A6=$(git rev-parse --verify A6) &&
> - A7=$(git rev-parse --verify A7) &&
> - B1=$(git rev-parse --verify B1) &&
> - B2=$(git rev-parse --verify B2) &&
> - C1=$(git rev-parse --verify C1) &&
> - C2=$(git rev-parse --verify C2) &&
> - C3=$(git rev-parse --verify C3) &&
> - C4=$(git rev-parse --verify C4)
> - '
> + test_commit A7 bar.txt
> +'
>
> test_expect_success '--graph --all' '
> - rm -f expected &&
> - echo "* $A7" >> expected &&
> - echo "* $A6" >> expected &&
> - echo "|\\ " >> expected &&
> - echo "| * $C4" >> expected &&
> - echo "| * $C3" >> expected &&
> - echo "* | $A5" >> expected &&
> - echo "| | " >> expected &&
> - echo "| \\ " >> expected &&
> - echo "*-. | $A4" >> expected &&
> - echo "|\\ \\| " >> expected &&
> - echo "| | * $C2" >> expected &&
> - echo "| | * $C1" >> expected &&
> - echo "| * | $B2" >> expected &&
> - echo "| * | $B1" >> expected &&
> - echo "* | | $A3" >> expected &&
> - echo "| |/ " >> expected &&
> - echo "|/| " >> expected &&
> - echo "* | $A2" >> expected &&
> - echo "|/ " >> expected &&
> - echo "* $A1" >> expected &&
> - git rev-list --graph --all > actual &&
> - test_cmp expected actual
> - '
> + check_graph --all <<-\EOF
> + * A7
> + * A6
> + |\
> + | * C4
> + | * C3
> + * | A5
> + | |
> + | \
> + *-. | A4
> + |\ \|
> + | | * C2
> + | | * C1
> + | * | B2
> + | * | B1
> + * | | A3
> + | |/
> + |/|
> + * | A2
> + |/
> + * A1
> + EOF
> +'
>
> # Make sure the graph_is_interesting() code still realizes
> # that undecorated merges are interesting, even with --simplify-by-decoration
> test_expect_success '--graph --simplify-by-decoration' '
> - rm -f expected &&
> git tag -d A4 &&
> - echo "* $A7" >> expected &&
> - echo "* $A6" >> expected &&
> - echo "|\\ " >> expected &&
> - echo "| * $C4" >> expected &&
> - echo "| * $C3" >> expected &&
> - echo "* | $A5" >> expected &&
> - echo "| | " >> expected &&
> - echo "| \\ " >> expected &&
> - echo "*-. | $A4" >> expected &&
> - echo "|\\ \\| " >> expected &&
> - echo "| | * $C2" >> expected &&
> - echo "| | * $C1" >> expected &&
> - echo "| * | $B2" >> expected &&
> - echo "| * | $B1" >> expected &&
> - echo "* | | $A3" >> expected &&
> - echo "| |/ " >> expected &&
> - echo "|/| " >> expected &&
> - echo "* | $A2" >> expected &&
> - echo "|/ " >> expected &&
> - echo "* $A1" >> expected &&
> - git rev-list --graph --all --simplify-by-decoration > actual &&
> - test_cmp expected actual
> - '
> + check_graph --all --simplify-by-decoration <<-\EOF
> + * A7
> + * A6
> + |\
> + | * C4
> + | * C3
> + * | A5
> + | |
> + | \
> + *-. | A4
> + |\ \|
> + | | * C2
> + | | * C1
> + | * | B2
> + | * | B1
> + * | | A3
> + | |/
> + |/|
> + * | A2
> + |/
> + * A1
> + EOF
> +'
>
> test_expect_success 'setup: get rid of decorations on B' '
> git tag -d B2 &&
> @@ -122,142 +111,133 @@ test_expect_success 'setup: get rid of decorations on B' '
>
> # Graph with branch B simplified away
> test_expect_success '--graph --simplify-by-decoration prune branch B' '
> - rm -f expected &&
> - echo "* $A7" >> expected &&
> - echo "* $A6" >> expected &&
> - echo "|\\ " >> expected &&
> - echo "| * $C4" >> expected &&
> - echo "| * $C3" >> expected &&
> - echo "* | $A5" >> expected &&
> - echo "* | $A4" >> expected &&
> - echo "|\\| " >> expected &&
> - echo "| * $C2" >> expected &&
> - echo "| * $C1" >> expected &&
> - echo "* | $A3" >> expected &&
> - echo "|/ " >> expected &&
> - echo "* $A2" >> expected &&
> - echo "* $A1" >> expected &&
> - git rev-list --graph --simplify-by-decoration --all > actual &&
> - test_cmp expected actual
> - '
> + check_graph --simplify-by-decoration --all <<-\EOF
> + * A7
> + * A6
> + |\
> + | * C4
> + | * C3
> + * | A5
> + * | A4
> + |\|
> + | * C2
> + | * C1
> + * | A3
> + |/
> + * A2
> + * A1
> + EOF
> +'
>
> test_expect_success '--graph --full-history -- bar.txt' '
> - rm -f expected &&
> - echo "* $A7" >> expected &&
> - echo "* $A6" >> expected &&
> - echo "|\\ " >> expected &&
> - echo "| * $C4" >> expected &&
> - echo "* | $A5" >> expected &&
> - echo "* | $A4" >> expected &&
> - echo "|\\| " >> expected &&
> - echo "* | $A3" >> expected &&
> - echo "|/ " >> expected &&
> - echo "* $A2" >> expected &&
> - git rev-list --graph --full-history --all -- bar.txt > actual &&
> - test_cmp expected actual
> - '
> + check_graph --full-history --all -- bar.txt <<-\EOF
> + * A7
> + * A6
> + |\
> + | * C4
> + * | A5
> + * | A4
> + |\|
> + * | A3
> + |/
> + * A2
> + EOF
> +'
>
> test_expect_success '--graph --full-history --simplify-merges -- bar.txt' '
> - rm -f expected &&
> - echo "* $A7" >> expected &&
> - echo "* $A6" >> expected &&
> - echo "|\\ " >> expected &&
> - echo "| * $C4" >> expected &&
> - echo "* | $A5" >> expected &&
> - echo "* | $A3" >> expected &&
> - echo "|/ " >> expected &&
> - echo "* $A2" >> expected &&
> - git rev-list --graph --full-history --simplify-merges --all \
> - -- bar.txt > actual &&
> - test_cmp expected actual
> - '
> + check_graph --full-history --simplify-merges --all -- bar.txt <<-\EOF
> + * A7
> + * A6
> + |\
> + | * C4
> + * | A5
> + * | A3
> + |/
> + * A2
> + EOF
> +'
>
> test_expect_success '--graph -- bar.txt' '
> - rm -f expected &&
> - echo "* $A7" >> expected &&
> - echo "* $A5" >> expected &&
> - echo "* $A3" >> expected &&
> - echo "| * $C4" >> expected &&
> - echo "|/ " >> expected &&
> - echo "* $A2" >> expected &&
> - git rev-list --graph --all -- bar.txt > actual &&
> - test_cmp expected actual
> - '
> + check_graph --all -- bar.txt <<-\EOF
> + * A7
> + * A5
> + * A3
> + | * C4
> + |/
> + * A2
> + EOF
> +'
>
> test_expect_success '--graph --sparse -- bar.txt' '
> - rm -f expected &&
> - echo "* $A7" >> expected &&
> - echo "* $A6" >> expected &&
> - echo "* $A5" >> expected &&
> - echo "* $A4" >> expected &&
> - echo "* $A3" >> expected &&
> - echo "| * $C4" >> expected &&
> - echo "| * $C3" >> expected &&
> - echo "| * $C2" >> expected &&
> - echo "| * $C1" >> expected &&
> - echo "|/ " >> expected &&
> - echo "* $A2" >> expected &&
> - echo "* $A1" >> expected &&
> - git rev-list --graph --sparse --all -- bar.txt > actual &&
> - test_cmp expected actual
> - '
> + check_graph --sparse --all -- bar.txt <<-\EOF
> + * A7
> + * A6
> + * A5
> + * A4
> + * A3
> + | * C4
> + | * C3
> + | * C2
> + | * C1
> + |/
> + * A2
> + * A1
> + EOF
> +'
>
> test_expect_success '--graph ^C4' '
> - rm -f expected &&
> - echo "* $A7" >> expected &&
> - echo "* $A6" >> expected &&
> - echo "* $A5" >> expected &&
> - echo "* $A4" >> expected &&
> - echo "|\\ " >> expected &&
> - echo "| * $B2" >> expected &&
> - echo "| * $B1" >> expected &&
> - echo "* $A3" >> expected &&
> - git rev-list --graph --all ^C4 > actual &&
> - test_cmp expected actual
> - '
> + check_graph --all ^C4 <<-\EOF
> + * A7
> + * A6
> + * A5
> + * A4
> + |\
> + | * B2
> + | * B1
> + * A3
> + EOF
> +'
>
> test_expect_success '--graph ^C3' '
> - rm -f expected &&
> - echo "* $A7" >> expected &&
> - echo "* $A6" >> expected &&
> - echo "|\\ " >> expected &&
> - echo "| * $C4" >> expected &&
> - echo "* $A5" >> expected &&
> - echo "* $A4" >> expected &&
> - echo "|\\ " >> expected &&
> - echo "| * $B2" >> expected &&
> - echo "| * $B1" >> expected &&
> - echo "* $A3" >> expected &&
> - git rev-list --graph --all ^C3 > actual &&
> - test_cmp expected actual
> - '
> + check_graph --all ^C3 <<-\EOF
> + * A7
> + * A6
> + |\
> + | * C4
> + * A5
> + * A4
> + |\
> + | * B2
> + | * B1
> + * A3
> + EOF
> +'
>
> # I don't think the ordering of the boundary commits is really
> # that important, but this test depends on it. If the ordering ever changes
> # in the code, we'll need to update this test.
> test_expect_success '--graph --boundary ^C3' '
> - rm -f expected &&
> - echo "* $A7" >> expected &&
> - echo "* $A6" >> expected &&
> - echo "|\\ " >> expected &&
> - echo "| * $C4" >> expected &&
> - echo "* | $A5" >> expected &&
> - echo "| | " >> expected &&
> - echo "| \\ " >> expected &&
> - echo "*-. \\ $A4" >> expected &&
> - echo "|\\ \\ \\ " >> expected &&
> - echo "| * | | $B2" >> expected &&
> - echo "| * | | $B1" >> expected &&
> - echo "* | | | $A3" >> expected &&
> - echo "o | | | $A2" >> expected &&
> - echo "|/ / / " >> expected &&
> - echo "o / / $A1" >> expected &&
> - echo " / / " >> expected &&
> - echo "| o $C3" >> expected &&
> - echo "|/ " >> expected &&
> - echo "o $C2" >> expected &&
> - git rev-list --graph --boundary --all ^C3 > actual &&
> - test_cmp expected actual
> - '
> + check_graph --boundary --all ^C3 <<-\EOF
> + * A7
> + * A6
> + |\
> + | * C4
> + * | A5
> + | |
> + | \
> + *-. \ A4
> + |\ \ \
> + | * | | B2
> + | * | | B1
> + * | | | A3
> + o | | | A2
> + |/ / /
> + o / / A1
> + / /
> + | o C3
> + |/
> + o C2
> + EOF
> +'
>
> test_done
> --
> 2.30.0
Thanks
- Abhishek
next prev parent reply other threads:[~2021-01-04 6:26 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-04 2:30 [PATCH] t6016: move to lib-log-graph.sh framework Antonio Russo
2021-01-04 6:26 ` Abhishek Kumar [this message]
2021-01-05 0:59 ` Antonio Russo
2021-01-06 5:05 ` Junio C Hamano
2021-01-06 15:21 ` Antonio Russo
2021-01-06 20:55 ` Junio C Hamano
2021-01-10 15:28 ` [PATCH v2] " Antonio Russo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=X/K1BgP8tpsgNe2x@Abhishek-Arch \
--to=abhishekkumar8222@gmail.com \
--cc=6dc37f6b-1afd-544d-126e-2be9422571c6@aerusso.net \
--cc=aerusso@aerusso.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).