From: Antonio Russo <hidden> Date: 2021-01-04 02:35:32
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.
Signed-off-by: Antonio Russo <redacted>
---
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
@@ -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 AgitcheckoutA&&-gitmergeBC&&+gitmergeBC-mA4&&gittagA4&&test_commitA5bar.txt&&
@@ -38,81 +44,64 @@ test_expect_success 'set up rev-list --graph test' 'test_commitC3foo.txt&&test_commitC4bar.txt&&gitcheckoutA&&-gitmerge-soursC&&+gitmerge-soursC-mA6&&gittagA6&&-test_commitA7bar.txt&&--# Store commit names in variables for later use-A1=$(gitrev-parse--verifyA1)&&-A2=$(gitrev-parse--verifyA2)&&-A3=$(gitrev-parse--verifyA3)&&-A4=$(gitrev-parse--verifyA4)&&-A5=$(gitrev-parse--verifyA5)&&-A6=$(gitrev-parse--verifyA6)&&-A7=$(gitrev-parse--verifyA7)&&-B1=$(gitrev-parse--verifyB1)&&-B2=$(gitrev-parse--verifyB2)&&-C1=$(gitrev-parse--verifyC1)&&-C2=$(gitrev-parse--verifyC2)&&-C3=$(gitrev-parse--verifyC3)&&-C4=$(gitrev-parse--verifyC4)-'+test_commitA7bar.txt+' test_expect_success'--graph --all''-rm-fexpected&&-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&&-gitrev-list--graph--all>actual&&-test_cmpexpectedactual-'+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-fexpected&&gittag-dA4&&-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&&-gitrev-list--graph--all--simplify-by-decoration>actual&&-test_cmpexpectedactual-'+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''gittag-dB2&&
@@ -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-fexpected&&-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&&-gitrev-list--graph--simplify-by-decoration--all>actual&&-test_cmpexpectedactual-'+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-fexpected&&-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&&-gitrev-list--graph--full-history--all--bar.txt>actual&&-test_cmpexpectedactual-'+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-fexpected&&-echo"* $A7">>expected&&-echo"* $A6">>expected&&-echo"|\\ ">>expected&&-echo"| * $C4">>expected&&-echo"* | $A5">>expected&&-echo"* | $A3">>expected&&-echo"|/ ">>expected&&-echo"* $A2">>expected&&-gitrev-list--graph--full-history--simplify-merges--all\---bar.txt>actual&&-test_cmpexpectedactual-'+check_graph--full-history--simplify-merges--all--bar.txt<<-\EOF+*A7+*A6+|\+|*C4+*|A5+*|A3+|/+*A2+EOF+' test_expect_success'--graph -- bar.txt''-rm-fexpected&&-echo"* $A7">>expected&&-echo"* $A5">>expected&&-echo"* $A3">>expected&&-echo"| * $C4">>expected&&-echo"|/ ">>expected&&-echo"* $A2">>expected&&-gitrev-list--graph--all--bar.txt>actual&&-test_cmpexpectedactual-'+check_graph--all--bar.txt<<-\EOF+*A7+*A5+*A3+|*C4+|/+*A2+EOF+' test_expect_success'--graph --sparse -- bar.txt''-rm-fexpected&&-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&&-gitrev-list--graph--sparse--all--bar.txt>actual&&-test_cmpexpectedactual-'+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-fexpected&&-echo"* $A7">>expected&&-echo"* $A6">>expected&&-echo"* $A5">>expected&&-echo"* $A4">>expected&&-echo"|\\ ">>expected&&-echo"| * $B2">>expected&&-echo"| * $B1">>expected&&-echo"* $A3">>expected&&-gitrev-list--graph--all^C4>actual&&-test_cmpexpectedactual-'+check_graph--all^C4<<-\EOF+*A7+*A6+*A5+*A4+|\+|*B2+|*B1+*A3+EOF+' test_expect_success'--graph ^C3''-rm-fexpected&&-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&&-gitrev-list--graph--all^C3>actual&&-test_cmpexpectedactual-'+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-fexpected&&-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&&-gitrev-list--graph--boundary--all^C3>actual&&-test_cmpexpectedactual-'+check_graph--boundary--all^C3<<-\EOF+*A7+*A6+|\+|*C4+*|A5+||+|\+*-.\ A4+|\ \ \+|*||B2+|*||B1+*|||A3+o|||A2+|///+o//A1+//+|oC3+|/+oC2+EOF+' test_done
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.
quoted hunk
Signed-off-by: Antonio Russo <redacted>
---
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
@@ -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 AgitcheckoutA&&-gitmergeBC&&+gitmergeBC-mA4&&gittagA4&&test_commitA5bar.txt&&
@@ -38,81 +44,64 @@ test_expect_success 'set up rev-list --graph test' 'test_commitC3foo.txt&&test_commitC4bar.txt&&gitcheckoutA&&-gitmerge-soursC&&+gitmerge-soursC-mA6&&gittagA6&&-test_commitA7bar.txt&&--# Store commit names in variables for later use-A1=$(gitrev-parse--verifyA1)&&-A2=$(gitrev-parse--verifyA2)&&-A3=$(gitrev-parse--verifyA3)&&-A4=$(gitrev-parse--verifyA4)&&-A5=$(gitrev-parse--verifyA5)&&-A6=$(gitrev-parse--verifyA6)&&-A7=$(gitrev-parse--verifyA7)&&-B1=$(gitrev-parse--verifyB1)&&-B2=$(gitrev-parse--verifyB2)&&-C1=$(gitrev-parse--verifyC1)&&-C2=$(gitrev-parse--verifyC2)&&-C3=$(gitrev-parse--verifyC3)&&-C4=$(gitrev-parse--verifyC4)-'+test_commitA7bar.txt+' test_expect_success'--graph --all''-rm-fexpected&&-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&&-gitrev-list--graph--all>actual&&-test_cmpexpectedactual-'+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-fexpected&&gittag-dA4&&-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&&-gitrev-list--graph--all--simplify-by-decoration>actual&&-test_cmpexpectedactual-'+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''gittag-dB2&&
@@ -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-fexpected&&-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&&-gitrev-list--graph--simplify-by-decoration--all>actual&&-test_cmpexpectedactual-'+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-fexpected&&-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&&-gitrev-list--graph--full-history--all--bar.txt>actual&&-test_cmpexpectedactual-'+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-fexpected&&-echo"* $A7">>expected&&-echo"* $A6">>expected&&-echo"|\\ ">>expected&&-echo"| * $C4">>expected&&-echo"* | $A5">>expected&&-echo"* | $A3">>expected&&-echo"|/ ">>expected&&-echo"* $A2">>expected&&-gitrev-list--graph--full-history--simplify-merges--all\---bar.txt>actual&&-test_cmpexpectedactual-'+check_graph--full-history--simplify-merges--all--bar.txt<<-\EOF+*A7+*A6+|\+|*C4+*|A5+*|A3+|/+*A2+EOF+' test_expect_success'--graph -- bar.txt''-rm-fexpected&&-echo"* $A7">>expected&&-echo"* $A5">>expected&&-echo"* $A3">>expected&&-echo"| * $C4">>expected&&-echo"|/ ">>expected&&-echo"* $A2">>expected&&-gitrev-list--graph--all--bar.txt>actual&&-test_cmpexpectedactual-'+check_graph--all--bar.txt<<-\EOF+*A7+*A5+*A3+|*C4+|/+*A2+EOF+' test_expect_success'--graph --sparse -- bar.txt''-rm-fexpected&&-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&&-gitrev-list--graph--sparse--all--bar.txt>actual&&-test_cmpexpectedactual-'+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-fexpected&&-echo"* $A7">>expected&&-echo"* $A6">>expected&&-echo"* $A5">>expected&&-echo"* $A4">>expected&&-echo"|\\ ">>expected&&-echo"| * $B2">>expected&&-echo"| * $B1">>expected&&-echo"* $A3">>expected&&-gitrev-list--graph--all^C4>actual&&-test_cmpexpectedactual-'+check_graph--all^C4<<-\EOF+*A7+*A6+*A5+*A4+|\+|*B2+|*B1+*A3+EOF+' test_expect_success'--graph ^C3''-rm-fexpected&&-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&&-gitrev-list--graph--all^C3>actual&&-test_cmpexpectedactual-'+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-fexpected&&-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&&-gitrev-list--graph--boundary--all^C3>actual&&-test_cmpexpectedactual-'+check_graph--boundary--all^C3<<-\EOF+*A7+*A6+|\+|*C4+*|A5+||+|\+*-.\ A4+|\ \ \+|*||B2+|*||B1+*|||A3+o|||A2+|///+o//A1+//+|oC3+|/+oC2+EOF+' test_done
From: Antonio Russo <hidden> Date: 2021-01-05 01:00:15
On 1/3/21 11:26 PM, Abhishek Kumar wrote:
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.
Thanks for the feedback! I can split this patch in two if that's desired.
[snip]
From: Junio C Hamano <hidden> Date: 2021-01-06 05:06:28
Abhishek Kumar [off-list ref] writes:
On Sun, Jan 03, 2021 at 07:30:35PM -0700, Antonio Russo wrote:
quoted
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 ...
From: Antonio Russo <hidden> Date: 2021-01-06 15:22:20
On 1/5/21 10:05 PM, Junio C Hamano wrote:
Abhishek Kumar [off-list ref] writes:
quoted
On Sun, Jan 03, 2021 at 07:30:35PM -0700, Antonio Russo wrote:
quoted
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?
You are correct that the A4 and A6 tags can be removed without affecting
the output. In fact, A4 is basically immediately deleted (in the second test).
I can remove that, if we want to stop testing the tag deletion logic here.
I suppose that is sufficiently validated elsewhere in the test suite.
There's a (weak IMO) argument to keep the A6 tag, since it might in principle
affect the output at later stages, since it is not deleted and calls are made
with --simplify-by-decoration. Of course, it isn't needed in order to be
displayed, so we're only testing that that the merge commit A6 shows up when
it *has* a tag, and --simplify-by-decoration is used. That failure mode
certainly seems perverse!
My guiding principle when I made this patch was to be as minimally invasive
as possible, while allowing modifications to this file to be pleasant---which
I must admit is my ulterior motive.
I can certainly remove these "extraneous" tags if desired.
Best,
Antonio
From: Antonio Russo <hidden> Date: 2021-01-10 15:29:43
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.
All commits, including merges, continue to be tagged, even if they are
immediately deleted, in order to exercise the exact same logic, i.e. in
`--all`, `--simplify-by-decoration`, and `git tag -d`.
Signed-off-by: Antonio Russo <redacted>
---
t/t6016-rev-list-graph-simplify-history.sh | 354 ++++++++++-----------
1 file changed, 167 insertions(+), 187 deletions(-)
I did not try to factor this commit into two since I did not hear any
strong opinions in favor of that.
@@ -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 AgitcheckoutA&&-gitmergeBC&&+gitmergeBC-mA4&&gittagA4&&test_commitA5bar.txt&&
@@ -38,81 +44,64 @@ test_expect_success 'set up rev-list --graph test' 'test_commitC3foo.txt&&test_commitC4bar.txt&&gitcheckoutA&&-gitmerge-soursC&&+gitmerge-soursC-mA6&&gittagA6&&-test_commitA7bar.txt&&--# Store commit names in variables for later use-A1=$(gitrev-parse--verifyA1)&&-A2=$(gitrev-parse--verifyA2)&&-A3=$(gitrev-parse--verifyA3)&&-A4=$(gitrev-parse--verifyA4)&&-A5=$(gitrev-parse--verifyA5)&&-A6=$(gitrev-parse--verifyA6)&&-A7=$(gitrev-parse--verifyA7)&&-B1=$(gitrev-parse--verifyB1)&&-B2=$(gitrev-parse--verifyB2)&&-C1=$(gitrev-parse--verifyC1)&&-C2=$(gitrev-parse--verifyC2)&&-C3=$(gitrev-parse--verifyC3)&&-C4=$(gitrev-parse--verifyC4)-'+test_commitA7bar.txt+' test_expect_success'--graph --all''-rm-fexpected&&-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&&-gitrev-list--graph--all>actual&&-test_cmpexpectedactual-'+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-fexpected&&gittag-dA4&&-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&&-gitrev-list--graph--all--simplify-by-decoration>actual&&-test_cmpexpectedactual-'+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''gittag-dB2&&
@@ -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-fexpected&&-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&&-gitrev-list--graph--simplify-by-decoration--all>actual&&-test_cmpexpectedactual-'+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-fexpected&&-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&&-gitrev-list--graph--full-history--all--bar.txt>actual&&-test_cmpexpectedactual-'+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-fexpected&&-echo"* $A7">>expected&&-echo"* $A6">>expected&&-echo"|\\ ">>expected&&-echo"| * $C4">>expected&&-echo"* | $A5">>expected&&-echo"* | $A3">>expected&&-echo"|/ ">>expected&&-echo"* $A2">>expected&&-gitrev-list--graph--full-history--simplify-merges--all\---bar.txt>actual&&-test_cmpexpectedactual-'+check_graph--full-history--simplify-merges--all--bar.txt<<-\EOF+*A7+*A6+|\+|*C4+*|A5+*|A3+|/+*A2+EOF+' test_expect_success'--graph -- bar.txt''-rm-fexpected&&-echo"* $A7">>expected&&-echo"* $A5">>expected&&-echo"* $A3">>expected&&-echo"| * $C4">>expected&&-echo"|/ ">>expected&&-echo"* $A2">>expected&&-gitrev-list--graph--all--bar.txt>actual&&-test_cmpexpectedactual-'+check_graph--all--bar.txt<<-\EOF+*A7+*A5+*A3+|*C4+|/+*A2+EOF+' test_expect_success'--graph --sparse -- bar.txt''-rm-fexpected&&-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&&-gitrev-list--graph--sparse--all--bar.txt>actual&&-test_cmpexpectedactual-'+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-fexpected&&-echo"* $A7">>expected&&-echo"* $A6">>expected&&-echo"* $A5">>expected&&-echo"* $A4">>expected&&-echo"|\\ ">>expected&&-echo"| * $B2">>expected&&-echo"| * $B1">>expected&&-echo"* $A3">>expected&&-gitrev-list--graph--all^C4>actual&&-test_cmpexpectedactual-'+check_graph--all^C4<<-\EOF+*A7+*A6+*A5+*A4+|\+|*B2+|*B1+*A3+EOF+' test_expect_success'--graph ^C3''-rm-fexpected&&-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&&-gitrev-list--graph--all^C3>actual&&-test_cmpexpectedactual-'+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-fexpected&&-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&&-gitrev-list--graph--boundary--all^C3>actual&&-test_cmpexpectedactual-'+check_graph--boundary--all^C3<<-\EOF+*A7+*A6+|\+|*C4+*|A5+||+|\+*-.\ A4+|\ \ \+|*||B2+|*||B1+*|||A3+o|||A2+|///+o//A1+//+|oC3+|/+oC2+EOF+' test_done