{"thread":{"id":"51245","subject":"[PATCH] t5551: test usage of chunked encoding explicitly","startedAt":"2019-06-05T19:26:31Z","lastAt":"2019-06-05T19:41:47Z","messageCount":3,"participants":["Jonathan Tan","Derrick Stolee","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"376724","messageId":"20190605192624.129677-1-jonathantanmy@google.com","threadId":"51245","inReplyTo":null,"subject":"[PATCH] t5551: test usage of chunked encoding explicitly","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2019-06-05T19:26:24Z","receivedAt":"2019-06-05T19:26:31Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"When run using GIT_TEST_PROTOCOL_VERSION=2, a test in t5551 fails\nbecause 4 POSTs (probe, ls-refs, probe, fetch) are sent instead of 2\n(probe, fetch).\n\nOne way to resolve this would be to relax the condition (from \"= 2\" to\ngreater than 1, say), but upon further inspection, the test probably\nshouldn't be counting the number of POSTs. This test states that large\nrequests are split across POSTs, but this is not correct; the main\nchange is that chunked transfer encoding is used, but the request is\nstill contained within one POST. (The test coincidentally works because\nGit indeed sends 2 POSTs in the case of a large request, but that is\nbecause, as stated above, the first POST is a probing RPC - see\npost_rpc() in remote-curl.c for more information.)\n\nTherefore, instead of counting POSTs, check that chunked transfer\nencoding is used. This also has the desirable side effect of passing\nwith GIT_TEST_PROTOCOL_VERSION=2.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\nWith this change, all tests pass at master with\nGIT_TEST_PROTOCOL_VERSION=2.\n---\n t/t5551-http-fetch-smart.sh | 5 ++---\n 1 file changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh\nindex ac74626a7b..aed2d9bb05 100755\n--- a/t/t5551-http-fetch-smart.sh\n+++ b/t/t5551-http-fetch-smart.sh\n@@ -301,11 +301,10 @@ test_expect_success CMDLINE_LIMIT \\\n \t)\n '\n \n-test_expect_success 'large fetch-pack requests can be split across POSTs' '\n+test_expect_success 'large fetch-pack requests can be sent using chunked encoding' '\n \tGIT_TRACE_CURL=true git -c http.postbuffer=65536 \\\n \t\tclone --bare \"$HTTPD_URL/smart/repo.git\" split.git 2>err &&\n-\tgrep \"^=> Send header: POST\" err >posts &&\n-\ttest_line_count = 2 posts\n+\tgrep \"^=> Send header: Transfer-Encoding: chunked\" err\n '\n \n test_expect_success 'test allowreachablesha1inwant' '\n-- \n2.22.0.rc1.311.g5d7573a151-goog\n\n"},{"id":"376726","messageId":"38419c39-cf69-099e-7ab1-2d4c4d9fbb74@gmail.com","threadId":"51245","inReplyTo":"20190605192624.129677-1-jonathantanmy@google.com","subject":"Re: [PATCH] t5551: test usage of chunked encoding explicitly","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2019-06-05T19:40:44Z","receivedAt":"2019-06-05T19:40:53Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 6/5/2019 3:26 PM, Jonathan Tan wrote:\n> When run using GIT_TEST_PROTOCOL_VERSION=2, a test in t5551 fails\n> because 4 POSTs (probe, ls-refs, probe, fetch) are sent instead of 2\n> (probe, fetch).\n> \n> One way to resolve this would be to relax the condition (from \"= 2\" to\n> greater than 1, say), but upon further inspection, the test probably\n> shouldn't be counting the number of POSTs. This test states that large\n> requests are split across POSTs, but this is not correct; the main\n> change is that chunked transfer encoding is used, but the request is\n> still contained within one POST. (The test coincidentally works because\n> Git indeed sends 2 POSTs in the case of a large request, but that is\n> because, as stated above, the first POST is a probing RPC - see\n> post_rpc() in remote-curl.c for more information.)\n> \n> Therefore, instead of counting POSTs, check that chunked transfer\n> encoding is used. This also has the desirable side effect of passing\n> with GIT_TEST_PROTOCOL_VERSION=2.\n\nI'm all for testing the _right_ thing.\n\n> -test_expect_success 'large fetch-pack requests can be split across POSTs' '\n> +test_expect_success 'large fetch-pack requests can be sent using chunked encoding' '\n>  \tGIT_TRACE_CURL=true git -c http.postbuffer=65536 \\\n>  \t\tclone --bare \"$HTTPD_URL/smart/repo.git\" split.git 2>err &&\n> -\tgrep \"^=> Send header: POST\" err >posts &&\n> -\ttest_line_count = 2 posts\n> +\tgrep \"^=> Send header: Transfer-Encoding: chunked\" err\n>  '\n\nAnd this does seem to be testing what you now claim it tests.\n\nLGTM.\n-Stolee\n\n"},{"id":"376727","messageId":"20190605194143.GA12686@sigill.intra.peff.net","threadId":"51245","inReplyTo":"20190605192624.129677-1-jonathantanmy@google.com","subject":"Re: [PATCH] t5551: test usage of chunked encoding explicitly","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-06-05T19:41:43Z","receivedAt":"2019-06-05T19:41:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 05, 2019 at 12:26:24PM -0700, Jonathan Tan wrote:\n\n> When run using GIT_TEST_PROTOCOL_VERSION=2, a test in t5551 fails\n> because 4 POSTs (probe, ls-refs, probe, fetch) are sent instead of 2\n> (probe, fetch).\n> \n> One way to resolve this would be to relax the condition (from \"= 2\" to\n> greater than 1, say), but upon further inspection, the test probably\n> shouldn't be counting the number of POSTs. This test states that large\n> requests are split across POSTs, but this is not correct; the main\n> change is that chunked transfer encoding is used, but the request is\n> still contained within one POST. (The test coincidentally works because\n> Git indeed sends 2 POSTs in the case of a large request, but that is\n> because, as stated above, the first POST is a probing RPC - see\n> post_rpc() in remote-curl.c for more information.)\n\nThat seems reasonable. What I was really going for with this test was\njust ensuring that we were actually hitting the \"large request\" code\npath at all. And checking for chunked encoding is another way of doing\nthat. (And I certainly agree that \"split across POSTs\" is an inaccurate\nway of describing what it was trying for).\n\n> With this change, all tests pass at master with\n> GIT_TEST_PROTOCOL_VERSION=2.\n\nYay. Thanks for your persistence with this.\n\n-Peff\n"}]}