{"thread":{"id":"51291","subject":"[PATCH 0/2] Improve test code coverage on jt/partial-clone-missing-ref-delta-base","startedAt":"2019-06-11T21:06:52Z","lastAt":"2019-06-12T02:39:31Z","messageCount":4,"participants":["Jonathan Tan","Derrick Stolee"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"377030","messageId":"cover.1560286956.git.jonathantanmy@google.com","threadId":"51291","inReplyTo":null,"subject":"[PATCH 0/2] Improve test code coverage on jt/partial-clone-missing-ref-delta-base","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2019-06-11T21:06:45Z","receivedAt":"2019-06-11T21:06:52Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"This series is on jt/partial-clone-missing-ref-delta-base.\n\nPatch 1 is a cleanup, and patch 2 improves test code coverage. Thanks to\nStolee for the code coverage report.\n\nI have checked this by running `make coverage` then `make cover_db_html`\non both \"next\" and this series, and comparing the output - the\nrelevant line indeed shows up as uncovered on \"next\", and covered on\nthis.\n\nJonathan Tan (2):\n  t5616: use correct flag to check object is missing\n  t5616: cover case of client having delta base\n\n t/t5616-partial-clone.sh | 41 ++++++++++++++++++++++++++++------------\n 1 file changed, 29 insertions(+), 12 deletions(-)\n\n-- \n2.22.0.rc2.383.gf4fbbf30c2-goog\n\n"},{"id":"377031","messageId":"c09d6119e51d70390d130cd9c47ef8d363244f80.1560286956.git.jonathantanmy@google.com","threadId":"51291","inReplyTo":"cover.1560286956.git.jonathantanmy@google.com","subject":"[PATCH 1/2] t5616: use correct flag to check object is missing","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2019-06-11T21:06:46Z","receivedAt":"2019-06-11T21:06:54Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"If we want to check whether an object is missing, the correct flag to\npass to rev-list is --ignore-missing; --exclude-promisor-objects will\nexclude any object that came from the promisor remote, whether it is\npresent or missing. Use the correct flag.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n t/t5616-partial-clone.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh\nindex f1baf83502..8b9b471a62 100755\n--- a/t/t5616-partial-clone.sh\n+++ b/t/t5616-partial-clone.sh\n@@ -359,7 +359,7 @@ test_expect_success 'tolerate server sending REF_DELTA against missing promisor\n \t\t--filter=blob:none $HTTPD_URL/one_time_sed/server repo &&\n \n \t# Sanity check to ensure that the client does not have that blob.\n-\tgit -C repo rev-list --objects --exclude-promisor-objects \\\n+\tgit -C repo rev-list --objects --ignore-missing \\\n \t\t-- $(cat deltabase) >objlist &&\n \ttest_line_count = 0 objlist &&\n \n-- \n2.22.0.rc2.383.gf4fbbf30c2-goog\n\n"},{"id":"377032","messageId":"baa48722faf7ff748341fb07fe5ba46877684db0.1560286956.git.jonathantanmy@google.com","threadId":"51291","inReplyTo":"cover.1560286956.git.jonathantanmy@google.com","subject":"[PATCH 2/2] t5616: cover case of client having delta base","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2019-06-11T21:06:47Z","receivedAt":"2019-06-11T21:06:57Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"When fetching into a partial clone, Git first prefetches missing\nREF_DELTA bases from the promisor remote. (This feature was introduced\nin [1].) But as can be seen in a recent test coverage report [2], the\ncase in which a REF_DELTA base is already present is not covered by\ntests.\n\nExtend the tests slightly to cover this case.\n\n[1] 8a30a1efd1 (\"index-pack: prefetch missing REF_DELTA bases\",\n2019-05-15).\n[2] https://public-inbox.org/git/396091fc-5572-19a5-4f18-61c258590dd5@gmail.com/\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n t/t5616-partial-clone.sh | 39 ++++++++++++++++++++++++++++-----------\n 1 file changed, 28 insertions(+), 11 deletions(-)\n\ndiff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh\nindex 8b9b471a62..b91ef548f8 100755\n--- a/t/t5616-partial-clone.sh\n+++ b/t/t5616-partial-clone.sh\n@@ -346,30 +346,37 @@ test_expect_success 'tolerate server sending REF_DELTA against missing promisor\n \ttest_config -C \"$SERVER\" uploadpack.allowfilter 1 &&\n \ttest_config -C \"$SERVER\" uploadpack.allowanysha1inwant 1 &&\n \n-\t# Create a commit with a blob to be used as a delta base.\n+\t# Create a commit with 2 blobs to be used as delta bases.\n \tfor i in $(test_seq 10)\n \tdo\n-\t\techo \"this is a line\" >>\"$SERVER/foo.txt\"\n+\t\techo \"this is a line\" >>\"$SERVER/foo.txt\" &&\n+\t\techo \"this is another line\" >>\"$SERVER/have.txt\"\n \tdone &&\n-\tgit -C \"$SERVER\" add foo.txt &&\n+\tgit -C \"$SERVER\" add foo.txt have.txt &&\n \tgit -C \"$SERVER\" commit -m bar &&\n-\tgit -C \"$SERVER\" rev-parse HEAD:foo.txt >deltabase &&\n+\tgit -C \"$SERVER\" rev-parse HEAD:foo.txt >deltabase_missing &&\n+\tgit -C \"$SERVER\" rev-parse HEAD:have.txt >deltabase_have &&\n \n+\t# Clone. The client has deltabase_have but not deltabase_missing.\n \tgit -c protocol.version=2 clone --no-checkout \\\n \t\t--filter=blob:none $HTTPD_URL/one_time_sed/server repo &&\n+\tgit -C repo hash-object -w -- \"$SERVER/have.txt\" &&\n \n-\t# Sanity check to ensure that the client does not have that blob.\n+\t# Sanity check to ensure that the client does not have\n+\t# deltabase_missing.\n \tgit -C repo rev-list --objects --ignore-missing \\\n-\t\t-- $(cat deltabase) >objlist &&\n+\t\t-- $(cat deltabase_missing) >objlist &&\n \ttest_line_count = 0 objlist &&\n \n \t# Another commit. This commit will be fetched by the client.\n \techo \"abcdefghijklmnopqrstuvwxyz\" >>\"$SERVER/foo.txt\" &&\n-\tgit -C \"$SERVER\" add foo.txt &&\n+\techo \"abcdefghijklmnopqrstuvwxyz\" >>\"$SERVER/have.txt\" &&\n+\tgit -C \"$SERVER\" add foo.txt have.txt &&\n \tgit -C \"$SERVER\" commit -m baz &&\n \n \t# Pack a thin pack containing, among other things, HEAD:foo.txt\n-\t# delta-ed against HEAD^:foo.txt.\n+\t# delta-ed against HEAD^:foo.txt and HEAD:have.txt delta-ed against\n+\t# HEAD^:have.txt.\n \tprintf \"%s\\n--not\\n%s\\n\" \\\n \t\t$(git -C \"$SERVER\" rev-parse HEAD) \\\n \t\t$(git -C \"$SERVER\" rev-parse HEAD^) |\n@@ -381,8 +388,13 @@ test_expect_success 'tolerate server sending REF_DELTA against missing promisor\n \t# most significant nybble of the first byte is 0b1111 (0b1 to indicate\n \t# that the header continues, and 0b111 to indicate REF_DELTA), followed\n \t# by any 3 nybbles, then the OID of the delta base.\n-\tgit -C \"$SERVER\" rev-parse HEAD^:foo.txt >deltabase &&\n-\tprintf \"f.,..%s\" $(intersperse \",\" <deltabase) >want &&\n+\tprintf \"f.,..%s\" $(intersperse \",\" <deltabase_missing) >want &&\n+\thex_unpack <thin.pack | intersperse \",\" >have &&\n+\tgrep $(cat want) have &&\n+\n+\t# Ensure that the pack contains one delta against HEAD^:have.txt,\n+\t# similar to the above.\n+\tprintf \"f.,..%s\" $(intersperse \",\" <deltabase_have) >want &&\n \thex_unpack <thin.pack | intersperse \",\" >have &&\n \tgrep $(cat want) have &&\n \n@@ -394,7 +406,12 @@ test_expect_success 'tolerate server sending REF_DELTA against missing promisor\n \n \t# Fetch the thin pack and ensure that index-pack is able to handle the\n \t# REF_DELTA object with a missing promisor delta base.\n-\tgit -C repo -c protocol.version=2 fetch &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/trace\" git -C repo -c protocol.version=2 fetch &&\n+\n+\t# Ensure that the missing delta base was directly fetched, but not the\n+\t# one that the client has.\n+\tgrep \"want $(cat deltabase_missing)\" trace &&\n+\t! grep \"want $(cat deltabase_have)\" trace &&\n \n \t# Ensure that the one-time-sed script was used.\n \t! test -e \"$HTTPD_ROOT_PATH/one-time-sed\"\n-- \n2.22.0.rc2.383.gf4fbbf30c2-goog\n\n"},{"id":"377049","messageId":"464f2cb0-68a5-ec2b-e526-98874c27a8b4@gmail.com","threadId":"51291","inReplyTo":"cover.1560286956.git.jonathantanmy@google.com","subject":"Re: [PATCH 0/2] Improve test code coverage on jt/partial-clone-missing-ref-delta-base","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2019-06-12T02:39:26Z","receivedAt":"2019-06-12T02:39:31Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 6/11/2019 5:06 PM, Jonathan Tan wrote:\n> This series is on jt/partial-clone-missing-ref-delta-base.\n> \n> Patch 1 is a cleanup, and patch 2 improves test code coverage. Thanks to\n> Stolee for the code coverage report.\n> \n> I have checked this by running `make coverage` then `make cover_db_html`\n> on both \"next\" and this series, and comparing the output - the\n> relevant line indeed shows up as uncovered on \"next\", and covered on\n> this.\n\nThanks for the additional tests, Jonathan! The update in PATCH 2 looks\nlike it took a bit of work, so thanks for your efforts there!\n\nLGTM.\n\n-Stolee\n\n"}]}