git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v4 7/7] tests: order arguments to git-rev-list properly

From
Matthew DeVore <matvore@google.com>
Date
Oct 3, 2018, 16:26 UTC
Message-ID
<bc2b3ec030650c107bd07c63f48bd874bf5f1085.1538581868.git.matvore@google.com>
In-Reply-To
<cover.1538581868.git.matvore@google.com>

It is a common mistake to put positional arguments before flags when invoking git-rev-list. Order the positional arguments last.

This patch skips git-rev-list invocations which include the --not flag, since the ordering of flags and positional arguments affects the behavior. This patch also skips invocations of git-rev-list that occur in command substitution in which the exit code is discarded, since fixing those properly will require a more involved cleanup.

Signed-off-by: Matthew DeVore <matvore@google.com>
---
 t/t5616-partial-clone.sh            | 26 +++++++++--------
 t/t5702-protocol-v2.sh              |  4 +--
 t/t6112-rev-list-filters-objects.sh | 43 ++++++++++++++++++-----------
 3 files changed, 44 insertions(+), 29 deletions(-)
diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
index fc7aeb1ab..eeedd1623 100755
--- a/t/t5616-partial-clone.sh
+++ b/t/t5616-partial-clone.sh
@@ -35,7 +35,7 @@ test_expect_success 'setup bare clone for server' '
 test_expect_success 'do partial clone 1' '
 	git clone --no-checkout --filter=blob:none "file://$(pwd)/srv.bare" pc1 &&
 
-	git -C pc1 rev-list HEAD --quiet --objects --missing=print >revs &&
+	git -C pc1 rev-list --quiet --objects --missing=print >revs HEAD &&
 	awk -f print_1.awk revs |
 	sed "s/?//" |
 	sort >observed.oids &&
@@ -48,10 +48,10 @@ test_expect_success 'do partial clone 1' '
 
 # checkout master to force dynamic object fetch of blobs at HEAD.
 test_expect_success 'verify checkout with dynamic object fetch' '
-	git -C pc1 rev-list HEAD --quiet --objects --missing=print >observed &&
+	git -C pc1 rev-list --quiet --objects --missing=print HEAD >observed &&
 	test_line_count = 4 observed &&
 	git -C pc1 checkout master &&
-	git -C pc1 rev-list HEAD --quiet --objects --missing=print >observed &&
+	git -C pc1 rev-list --quiet --objects --missing=print HEAD >observed &&
 	test_line_count = 0 observed
 '
 
@@ -74,7 +74,8 @@ test_expect_success 'push new commits to server' '
 # have the new blobs.
 test_expect_success 'partial fetch inherits filter settings' '
 	git -C pc1 fetch origin &&
-	git -C pc1 rev-list master..origin/master --quiet --objects --missing=print >observed &&
+	git -C pc1 rev-list --quiet --objects --missing=print \
+		master..origin/master >observed &&
 	test_line_count = 5 observed
 '
 
@@ -82,7 +83,8 @@ test_expect_success 'partial fetch inherits filter settings' '
 # we should only get 1 new blob (for the file in origin/master).
 test_expect_success 'verify diff causes dynamic object fetch' '
 	git -C pc1 diff master..origin/master -- file.1.txt &&
-	git -C pc1 rev-list master..origin/master --quiet --objects --missing=print >observed &&
+	git -C pc1 rev-list --quiet --objects --missing=print \
+		 master..origin/master >observed &&
 	test_line_count = 4 observed
 '
 
@@ -91,7 +93,8 @@ test_expect_success 'verify diff causes dynamic object fetch' '
 test_expect_success 'verify blame causes dynamic object fetch' '
 	git -C pc1 blame origin/master -- file.1.txt >observed.blame &&
 	test_cmp expect.blame observed.blame &&
-	git -C pc1 rev-list master..origin/master --quiet --objects --missing=print >observed &&
+	git -C pc1 rev-list --quiet --objects --missing=print >observed \
+		master..origin/master &&
 	test_line_count = 0 observed
 '
 
@@ -111,7 +114,8 @@ test_expect_success 'push new commits to server for file.2.txt' '
 # Verify we have all the new blobs.
 test_expect_success 'override inherited filter-spec using --no-filter' '
 	git -C pc1 fetch --no-filter origin &&
-	git -C pc1 rev-list master..origin/master --quiet --objects --missing=print >observed &&
+	git -C pc1 rev-list --quiet --objects --missing=print \
+		master..origin/master >observed &&
 	test_line_count = 0 observed
 '
 
@@ -133,8 +137,8 @@ test_expect_success 'push new commits to server for file.3.txt' '
 test_expect_success 'manual prefetch of missing objects' '
 	git -C pc1 fetch --filter=blob:none origin &&
 
-	git -C pc1 rev-list master..origin/master --quiet --objects --missing=print \
-		>revs &&
+	git -C pc1 rev-list --quiet --objects --missing=print \
+		 master..origin/master >revs &&
 	awk -f print_1.awk revs |
 	sed "s/?//" |
 	sort >observed.oids &&
@@ -142,8 +146,8 @@ test_expect_success 'manual prefetch of missing objects' '
 	test_line_count = 6 observed.oids &&
 	git -C pc1 fetch-pack --stdin "file://$(pwd)/srv.bare" <observed.oids &&
 
-	git -C pc1 rev-list master..origin/master --quiet --objects --missing=print \
-		>revs &&
+	git -C pc1 rev-list --quiet --objects --missing=print \
+		master..origin/master >revs &&
 	awk -f print_1.awk revs |
 	sed "s/?//" |
 	sort >observed.oids &&
diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
index 54727450b..11a84efff 100755
--- a/t/t5702-protocol-v2.sh
+++ b/t/t5702-protocol-v2.sh
@@ -271,7 +271,7 @@ test_expect_success 'partial clone' '
 	grep "version 2" trace &&
 
 	# Ensure that the old version of the file is missing
-	git -C client rev-list master --quiet --objects --missing=print \
+	git -C client rev-list --quiet --objects --missing=print master \
 		>observed.oids &&
 	grep "$(git -C server rev-parse message1:a.txt)" observed.oids &&
 
@@ -297,7 +297,7 @@ test_expect_success 'partial fetch' '
 	grep "version 2" trace &&
 
 	# Ensure that the old version of the file is missing
-	git -C client rev-list other --quiet --objects --missing=print \
+	git -C client rev-list --quiet --objects --missing=print other \
 		>observed.oids &&
 	grep "$(git -C server rev-parse message1:a.txt)" observed.oids &&
 
diff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh
index b00cf6fa8..53975c572 100755
--- a/t/t6112-rev-list-filters-objects.sh
+++ b/t/t6112-rev-list-filters-objects.sh
@@ -25,7 +25,8 @@ test_expect_success 'verify blob:none omits all 5 blobs' '
 	awk -f print_2.awk ls_files_result |
 	sort >expected &&
 
-	git -C r1 rev-list HEAD --quiet --objects --filter-print-omitted --filter=blob:none >revs &&
+	git -C r1 rev-list --quiet --objects --filter-print-omitted \
+		--filter=blob:none HEAD >revs &&
 	awk -f print_1.awk revs |
 	sed "s/~//" |
 	sort >observed &&
@@ -34,12 +35,12 @@ test_expect_success 'verify blob:none omits all 5 blobs' '
 '
 
 test_expect_success 'verify emitted+omitted == all' '
-	git -C r1 rev-list HEAD --objects >revs &&
+	git -C r1 rev-list --objects HEAD >revs &&
 	awk -f print_1.awk revs |
 	sort >expected &&
 
-	git -C r1 rev-list HEAD --objects --filter-print-omitted --filter=blob:none \
-		>revs &&
+	git -C r1 rev-list --objects --filter-print-omitted --filter=blob:none \
+		HEAD >revs &&
 	awk -f print_1.awk revs |
 	sed "s/~//" |
 	sort >observed &&
@@ -68,7 +69,8 @@ test_expect_success 'verify blob:limit=500 omits all blobs' '
 	awk -f print_2.awk ls_files_result |
 	sort >expected &&
 
-	git -C r2 rev-list HEAD --quiet --objects --filter-print-omitted --filter=blob:limit=500 >revs &&
+	git -C r2 rev-list --quiet --objects --filter-print-omitted \
+		--filter=blob:limit=500 HEAD >revs &&
 	awk -f print_1.awk revs |
 	sed "s/~//" |
 	sort >observed &&
@@ -77,11 +79,12 @@ test_expect_success 'verify blob:limit=500 omits all blobs' '
 '
 
 test_expect_success 'verify emitted+omitted == all' '
-	git -C r2 rev-list HEAD --objects >revs &&
+	git -C r2 rev-list --objects HEAD >revs &&
 	awk -f print_1.awk revs |
 	sort >expected &&
 
-	git -C r2 rev-list HEAD --objects --filter-print-omitted --filter=blob:limit=500 >revs &&
+	git -C r2 rev-list --objects --filter-print-omitted \
+		--filter=blob:limit=500 HEAD >revs &&
 	awk -f print_1.awk revs |
 	sed "s/~//" |
 	sort >observed &&
@@ -94,7 +97,8 @@ test_expect_success 'verify blob:limit=1000' '
 	awk -f print_2.awk ls_files_result |
 	sort >expected &&
 
-	git -C r2 rev-list HEAD --quiet --objects --filter-print-omitted --filter=blob:limit=1000 >revs &&
+	git -C r2 rev-list --quiet --objects --filter-print-omitted \
+		--filter=blob:limit=1000 HEAD >revs &&
 	awk -f print_1.awk revs |
 	sed "s/~//" |
 	sort >observed &&
@@ -107,7 +111,8 @@ test_expect_success 'verify blob:limit=1001' '
 	awk -f print_2.awk ls_files_result |
 	sort >expected &&
 
-	git -C r2 rev-list HEAD --quiet --objects --filter-print-omitted --filter=blob:limit=1001 >revs &&
+	git -C r2 rev-list --quiet --objects --filter-print-omitted \
+		--filter=blob:limit=1001 HEAD >revs &&
 	awk -f print_1.awk revs |
 	sed "s/~//" |
 	sort >observed &&
@@ -120,7 +125,8 @@ test_expect_success 'verify blob:limit=1k' '
 	awk -f print_2.awk ls_files_result |
 	sort >expected &&
 
-	git -C r2 rev-list HEAD --quiet --objects --filter-print-omitted --filter=blob:limit=1k >revs &&
+	git -C r2 rev-list --quiet --objects --filter-print-omitted \
+		--filter=blob:limit=1k HEAD >revs &&
 	awk -f print_1.awk revs |
 	sed "s/~//" |
 	sort >observed &&
@@ -129,7 +135,8 @@ test_expect_success 'verify blob:limit=1k' '
 '
 
 test_expect_success 'verify blob:limit=1m' '
-	git -C r2 rev-list HEAD --quiet --objects --filter-print-omitted --filter=blob:limit=1m >revs &&
+	git -C r2 rev-list --quiet --objects --filter-print-omitted \
+		--filter=blob:limit=1m HEAD >revs &&
 	awk -f print_1.awk revs |
 	sed "s/~//" |
 	sort >observed &&
@@ -162,7 +169,8 @@ test_expect_success 'verify sparse:path=pattern1 omits top-level files' '
 	awk -f print_2.awk ls_files_result |
 	sort >expected &&
 
-	git -C r3 rev-list HEAD --quiet --objects --filter-print-omitted --filter=sparse:path=../pattern1 >revs &&
+	git -C r3 rev-list --quiet --objects --filter-print-omitted \
+		--filter=sparse:path=../pattern1 HEAD >revs &&
 	awk -f print_1.awk revs |
 	sed "s/~//" |
 	sort >observed &&
@@ -175,7 +183,8 @@ test_expect_success 'verify sparse:path=pattern2 omits both sparse2 files' '
 	awk -f print_2.awk ls_files_result |
 	sort >expected &&
 
-	git -C r3 rev-list HEAD --quiet --objects --filter-print-omitted --filter=sparse:path=../pattern2 >revs &&
+	git -C r3 rev-list --quiet --objects --filter-print-omitted \
+		--filter=sparse:path=../pattern2 HEAD >revs &&
 	awk -f print_1.awk revs |
 	sed "s/~//" |
 	sort >observed &&
@@ -200,7 +209,8 @@ test_expect_success 'verify sparse:oid=OID omits top-level files' '
 
 	oid=$(git -C r3 ls-files -s pattern | awk -f print_2.awk) &&
 
-	git -C r3 rev-list HEAD --quiet --objects --filter-print-omitted --filter=sparse:oid=$oid >revs &&
+	git -C r3 rev-list --quiet --objects --filter-print-omitted \
+		--filter=sparse:oid=$oid HEAD >revs &&
 	awk -f print_1.awk revs |
 	sed "s/~//" |
 	sort >observed &&
@@ -213,7 +223,8 @@ test_expect_success 'verify sparse:oid=oid-ish omits top-level files' '
 	awk -f print_2.awk ls_files_result |
 	sort >expected &&
 
-	git -C r3 rev-list HEAD --quiet --objects --filter-print-omitted --filter=sparse:oid=master:pattern >revs &&
+	git -C r3 rev-list --quiet --objects --filter-print-omitted \
+		--filter=sparse:oid=master:pattern HEAD >revs &&
 	awk -f print_1.awk revs |
 	sed "s/~//" |
 	sort >observed &&
@@ -235,7 +246,7 @@ test_expect_success 'rev-list W/ --missing=print' '
 		rm r1/.git/objects/$id
 	done &&
 
-	git -C r1 rev-list --quiet HEAD --missing=print --objects >revs &&
+	git -C r1 rev-list --quiet --missing=print --objects HEAD >revs &&
 	awk -f print_1.awk revs |
 	sed "s/?//" |
 	sort >observed &&

-- 
2.19.0.605.g01d371f741-goog
Previous: Matthew DeVoreNext: Matthew DeVore
Message 38 of 66 in “Cleanup tests for test_cmp argument ordering and "|" placement”
  1. 0/2 Cleanup tests for test_cmp argument ordering and "|" placementMatthew DeVore, Sep 15, 2018
  2. 1/2 t/*: fix pipe placement and remove \'sMatthew DeVore, Sep 15, 2018
  3. Jonathan NiederSep 17, 2018
  4. Matthew DeVoreSep 17, 2018
  5. 2/2 t/*: fix ordering of expected/observed argumentsMatthew DeVore, Sep 15, 2018
  6. Matthew DeVoreSep 17, 2018
  7. Junio C HamanoSep 15, 2018
  8. 0/6 Clean up tests for test_cmp arg ordering and pipe placementMatthew DeVore, Sep 17, 2018
  9. 4/6 tests: Add linter check for pipe placement styleMatthew DeVore, Sep 17, 2018
  10. Eric SunshineSep 18, 2018
  11. Matthew DeVoreSep 19, 2018
  12. 0/5 Clean up tests for test_cmp arg ordering and pipe placementMatthew DeVore, Sep 21, 2018
  13. 1/5 CodingGuidelines: add shell piping guidelinesMatthew DeVore, Sep 21, 2018
  14. Eric SunshineSep 21, 2018
  15. Matthew DeVoreSep 21, 2018
  16. SZEDER GáborSep 24, 2018
  17. Matthew DeVoreSep 25, 2018
  18. SZEDER GáborSep 27, 2018
  19. Matthew DeVoreOct 1, 2018
  20. 2/5 tests: standardize pipe placementMatthew DeVore, Sep 21, 2018
  21. 3/5 t/*: fix ordering of expected/observed argumentsMatthew DeVore, Sep 21, 2018
  22. 4/5 tests: don't swallow Git errors upstream of pipesMatthew DeVore, Sep 21, 2018
  23. 5/5 t9109: don't swallow Git errors upstream of pipesMatthew DeVore, Sep 21, 2018
  24. 0/7 Clean up tests for test_cmp arg ordering and pipe placementMatthew DeVore, Oct 3, 2018
  25. 1/7 t/README: reformat Do, Don't, Keep in mind listsMatthew DeVore, Oct 3, 2018
  26. Junio C HamanoOct 5, 2018
  27. Matthew DeVoreOct 5, 2018
  28. 2/7 Documentation: add shell guidelinesMatthew DeVore, Oct 3, 2018
  29. Junio C HamanoOct 5, 2018
  30. Matthew DeVoreOct 5, 2018
  31. 3/7 tests: standardize pipe placementMatthew DeVore, Oct 3, 2018
  32. 4/7 t/*: fix ordering of expected/observed argumentsMatthew DeVore, Oct 3, 2018
  33. 5/7 tests: don't swallow Git errors upstream of pipesMatthew DeVore, Oct 3, 2018
  34. Junio C HamanoOct 5, 2018
  35. Matthew DeVoreOct 5, 2018
  36. Matthew DeVoreOct 5, 2018
  37. 6/7 t9109: don't swallow Git errors upstream of pipesMatthew DeVore, Oct 3, 2018
  38. 7/7 tests: order arguments to git-rev-list properlyMatthew DeVore, Oct 3, 2018
  39. Matthew DeVoreOct 3, 2018
  40. Junio C HamanoOct 5, 2018
  41. 0/7 subject: Clean up tests for test_cmp arg ordering and pipe placementMatthew DeVore, Oct 5, 2018
  42. 1/7 t/README: reformat Do, Don't, Keep in mind listsMatthew DeVore, Oct 5, 2018
  43. 2/7 Documentation: add shell guidelinesMatthew DeVore, Oct 5, 2018
  44. 3/7 tests: standardize pipe placementMatthew DeVore, Oct 5, 2018
  45. 4/7 t/*: fix ordering of expected/observed argumentsMatthew DeVore, Oct 5, 2018
  46. 5/7 tests: don't swallow Git errors upstream of pipesMatthew DeVore, Oct 5, 2018
  47. 6/7 t9109: don't swallow Git errors upstream of pipesMatthew DeVore, Oct 5, 2018
  48. 7/7 tests: order arguments to git-rev-list properlyMatthew DeVore, Oct 5, 2018
  49. Junio C HamanoOct 6, 2018
  50. 1/6 CodingGuidelines: add shell piping guidelinesMatthew DeVore, Sep 17, 2018
  51. Eric SunshineSep 18, 2018
  52. Matthew DeVoreSep 19, 2018
  53. Eric SunshineSep 19, 2018
  54. Junio C HamanoSep 19, 2018
  55. Matthew DeVoreSep 19, 2018
  56. 2/6 tests: standardize pipe placementMatthew DeVore, Sep 17, 2018
  57. 3/6 t/*: fix ordering of expected/observed argumentsMatthew DeVore, Sep 17, 2018
  58. 4/6 tests: add linter check for pipe placement styleMatthew DeVore, Sep 17, 2018
  59. 5/6 tests: split up pipesMatthew DeVore, Sep 17, 2018
  60. Eric SunshineSep 18, 2018
  61. Matthew DeVoreSep 19, 2018
  62. 6/6 t9109-git-svn-props.sh: split up several pipesMatthew DeVore, Sep 17, 2018
  63. Eric SunshineSep 18, 2018
  64. Matthew DeVoreSep 19, 2018
  65. Eric SunshineSep 19, 2018
  66. Matthew DeVoreSep 19, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.