{"thread":{"id":"24698","subject":"[PATCH] git-notes: Run partial expensive test everywhere","startedAt":"2010-08-10T19:56:04Z","lastAt":"2010-08-29T19:20:15Z","messageCount":9,"participants":["Ævar Arnfjörð Bjarmason","Sverre Rabbelier","Jonathan Nieder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"147680","messageId":"1281470164-27008-1-git-send-email-avarab@gmail.com","threadId":"24698","inReplyTo":null,"subject":"[PATCH] git-notes: Run partial expensive test everywhere","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-08-10T19:56:04Z","receivedAt":"2010-08-10T19:56:04Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The git-notes expensive timing test is only expensive because it\neither did 1k iterations or nothing. Change it to do 10 by default,\nwith an option to run the expensive version with the old\nGIT_NOTES_TIMING_TESTS=ZomgYesPlease variable.\n\nSince nobody was ostensibly running this test under TAP the code had\nbitrotted so that it emitted invalid TAP. This change fixes that.\n\nThe old version would also mysteriously fail on systems without\n/usr/bin/time, there's now a check for that using the test\nprerequisite facility.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t3302-notes-index-expensive.sh | 39 ++++++++++++++++++++++++++-----------\n 1 files changed, 27 insertions(+), 12 deletions(-)\n\ndiff --git a/t/t3302-notes-index-expensive.sh b/t/t3302-notes-index-expensive.sh\nindex 361a10a..3b8313c 100755\n--- a/t/t3302-notes-index-expensive.sh\n+++ b/t/t3302-notes-index-expensive.sh\n@@ -7,11 +7,16 @@ test_description='Test commit notes index (expensive!)'\n \n . ./test-lib.sh\n \n-test -z \"$GIT_NOTES_TIMING_TESTS\" && {\n-\tskip_all=\"Skipping timing tests\"\n-\ttest_done\n-\texit\n-}\n+test_set_prereq NOT_EXPENSIVE\n+test -n \"$GIT_NOTES_TIMING_TESTS\" && test_set_prereq EXPENSIVE\n+\n+if test -x /usr/bin/time\n+then\n+\t# Hack around multiple test prerequisites not supporting AND-ing\n+\t# of terms\n+\ttest_set_prereq USR_BIN_TIME+NOT_EXPENSIVE\n+\ttest_have_prereq EXPENSIVE && test_set_prereq USR_BIN_TIME+EXPENSIVE\n+fi\n \n create_repo () {\n \tnumber_of_commits=$1\n@@ -102,17 +107,27 @@ time_notes () {\n \tdone\n }\n \n-for count in 10 100 1000 10000; do\n+do_tests () {\n+\tpr=$1\n+\tcount=$2\n+\n+\ttest_expect_success $pr 'setup / mkdir' '\n+\t\tmkdir $count &&\n+\t\tcd $count\n+\t'\n \n-\tmkdir $count\n-\t(cd $count;\n+\ttest_expect_success $pr \"setup $count\" \"create_repo $count\"\n \n-\ttest_expect_success \"setup $count\" \"create_repo $count\"\n+\ttest_expect_success $pr 'notes work' \"test_notes $count\"\n \n-\ttest_expect_success 'notes work' \"test_notes $count\"\n+\ttest_expect_success USR_BIN_TIME+$pr 'notes timing with /usr/bin/time' \"time_notes 100\"\n+\n+\ttest_expect_success $pr 'teardown / cd ..' 'cd ..'\n+}\n \n-\ttest_expect_success 'notes timing' \"time_notes 100\"\n-\t)\n+do_tests NOT_EXPENSIVE 10\n+for count in 100 1000 10000; do\n+\tdo_tests EXPENSIVE $count\n done\n \n test_done\n-- \n1.7.2.1.295.gd03d\n"},{"id":"147683","messageId":"AANLkTim2tozO31HKLwxi6kWyZKYMd9egDhS5U_gb5OvZ@mail.gmail.com","threadId":"24698","inReplyTo":"1281470164-27008-1-git-send-email-avarab@gmail.com","subject":"Re: [PATCH] git-notes: Run partial expensive test everywhere","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2010-08-10T20:29:59Z","receivedAt":"2010-08-10T20:29:59Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Tue, Aug 10, 2010 at 14:56, Ævar Arnfjörð Bjarmason wrote:\n> The git-notes expensive timing test is only expensive because it\n> either did 1k iterations or nothing. Change it to do 10 by default,\n> with an option to run the expensive version with the old\n> GIT_NOTES_TIMING_TESTS=ZomgYesPlease variable.\n\nNice, why 10 though? Any motivation for that particular value?\n\n> Since nobody was ostensibly running this test under TAP the code had\n> bitrotted so that it emitted invalid TAP. This change fixes that.\n\nNice catch.\n\n> The old version would also mysteriously fail on systems without\n> /usr/bin/time, there's now a check for that using the test\n> prerequisite facility.\n\nShould this patch be split up?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"147688","messageId":"AANLkTikO6_cBBywSdvSdpWNO8OgSzmp9EB-xQ7Y_s4qf@mail.gmail.com","threadId":"24698","inReplyTo":"AANLkTim2tozO31HKLwxi6kWyZKYMd9egDhS5U_gb5OvZ@mail.gmail.com","subject":"Re: [PATCH] git-notes: Run partial expensive test everywhere","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-08-10T21:00:11Z","receivedAt":"2010-08-10T21:00:11Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, Aug 10, 2010 at 20:29, Sverre Rabbelier wrote:\n> Heya,\n>\n> On Tue, Aug 10, 2010 at 14:56, Ævar Arnfjörð Bjarmason wrote:\n>> The git-notes expensive timing test is only expensive because it\n>> either did 1k iterations or nothing. Change it to do 10 by default,\n>> with an option to run the expensive version with the old\n>> GIT_NOTES_TIMING_TESTS=ZomgYesPlease variable.\n>\n> Nice, why 10 though? Any motivation for that particular value?\n\nThe old version had \"for count in 10 100 1000 10000; do\". Mine has 10\nas non-expensive, and \"for count in 100 1000 10000; do\" as expensive.\n\nI.e. I'm running the first test batch from the old tests.\n\nI have no idea whether it actually needs to run 10..10k times, I\ndidn't try to grok the actual test code.\n\n>> The old version would also mysteriously fail on systems without\n>> /usr/bin/time, there's now a check for that using the test\n>> prerequisite facility.\n>\n> Should this patch be split up?\n\nIt all touched the same bits, it'd be nastier to split it up IMO.\n"},{"id":"147690","messageId":"AANLkTimQE6q=3jOXZcWrFfRcpA6-Ew79z9+r-RwyaYHZ@mail.gmail.com","threadId":"24698","inReplyTo":"1281470164-27008-1-git-send-email-avarab@gmail.com","subject":"Re: [PATCH] git-notes: Run partial expensive test everywhere","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-08-10T21:56:24Z","receivedAt":"2010-08-10T21:56:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, Aug 10, 2010 at 19:56, Ævar Arnfjörð Bjarmason wrote:\n> +       # Hack around multiple test prerequisites not supporting AND-ing\n> +       # of terms\n> +       test_set_prereq USR_BIN_TIME+NOT_EXPENSIVE\n> +       test_have_prereq EXPENSIVE && test_set_prereq USR_BIN_TIME+EXPENSIVE\n> +fi\n\nIn retrospect this may have been some brainfried code, I'll check it\nout tomorrow.\n"},{"id":"147703","messageId":"1281483468-24388-1-git-send-email-avarab@gmail.com","threadId":"24698","inReplyTo":"1281470164-27008-1-git-send-email-avarab@gmail.com","subject":"[PATCH v2] git-notes: Run partial expensive test everywhere","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-08-10T23:37:48Z","receivedAt":"2010-08-10T23:37:48Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The git-notes expensive timing test is only expensive because it\neither did 10,100,1k and 10k iterations or nothing.\n\nChange it to do 10 by default, with an option to run the expensive\nversion with the old GIT_NOTES_TIMING_TESTS=ZomgYesPlease variable.\n\nSince nobody was ostensibly running this test under TAP the code had\nbitrotted so that it emitted invalid TAP. This change fixes that.\n\nThe old version would also mysteriously fail on systems without\n/usr/bin/time, there's now a check for that using the multiple test\nprerequisite facility.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n\nOn Tue, Aug 10, 2010 at 21:56, Ævar Arnfjörð Bjarmason wrote:\n> On Tue, Aug 10, 2010 at 19:56, Ævar Arnfjörð Bjarmason wrote:\n>> +       # Hack around multiple test prerequisites not supporting AND-ing\n>> +       # of terms\n>> +       test_set_prereq USR_BIN_TIME+NOT_EXPENSIVE\n>> +       test_have_prereq EXPENSIVE && test_set_prereq USR_BIN_TIME+EXPENSIVE\n>> +fi\n>\n> In retrospect this may have been some brainfried code, I'll check it\n> out tomorrow.\n\nHere's a patch that's not crazy. In v1 I was hacking around not having\na facility I already added to the test-lib (tired).\n\nThis patch goes on top of my \"test-lib: Multi-prereq support only\nchecked the last prereq\" patch, which fixes up the test prereq\nfacility so that it actually works.\n\n t/t3302-notes-index-expensive.sh | 32 ++++++++++++++++++++------------\n 1 files changed, 20 insertions(+), 12 deletions(-)\n\ndiff --git a/t/t3302-notes-index-expensive.sh b/t/t3302-notes-index-expensive.sh\nindex 361a10a..7c08e99 100755\n--- a/t/t3302-notes-index-expensive.sh\n+++ b/t/t3302-notes-index-expensive.sh\n@@ -7,11 +7,9 @@ test_description='Test commit notes index (expensive!)'\n \n . ./test-lib.sh\n \n-test -z \"$GIT_NOTES_TIMING_TESTS\" && {\n-\tskip_all=\"Skipping timing tests\"\n-\ttest_done\n-\texit\n-}\n+test_set_prereq NOT_EXPENSIVE\n+test -n \"$GIT_NOTES_TIMING_TESTS\" && test_set_prereq EXPENSIVE\n+test -x /usr/bin/time && test_set_prereq USR_BIN_TIME\n \n create_repo () {\n \tnumber_of_commits=$1\n@@ -102,17 +100,27 @@ time_notes () {\n \tdone\n }\n \n-for count in 10 100 1000 10000; do\n+do_tests () {\n+\tpr=$1\n+\tcount=$2\n+\n+\ttest_expect_success $pr 'setup / mkdir' '\n+\t\tmkdir $count &&\n+\t\tcd $count\n+\t'\n \n-\tmkdir $count\n-\t(cd $count;\n+\ttest_expect_success $pr \"setup $count\" \"create_repo $count\"\n \n-\ttest_expect_success \"setup $count\" \"create_repo $count\"\n+\ttest_expect_success $pr 'notes work' \"test_notes $count\"\n \n-\ttest_expect_success 'notes work' \"test_notes $count\"\n+\ttest_expect_success USR_BIN_TIME,$pr 'notes timing with /usr/bin/time' \"time_notes 100\"\n+\n+\ttest_expect_success $pr 'teardown / cd ..' 'cd ..'\n+}\n \n-\ttest_expect_success 'notes timing' \"time_notes 100\"\n-\t)\n+do_tests NOT_EXPENSIVE 10\n+for count in 100 1000 10000; do\n+\tdo_tests EXPENSIVE $count\n done\n \n test_done\n-- \n1.7.2.1.295.gd03d\n"},{"id":"148455","messageId":"AANLkTikfwbtKt2WrxzRyKHnP3ySGtBF33=z_rJwjPx5Y@mail.gmail.com","threadId":"24698","inReplyTo":"1281483468-24388-1-git-send-email-avarab@gmail.com","subject":"Re: [PATCH v2] git-notes: Run partial expensive test everywhere","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-08-19T15:50:09Z","receivedAt":"2010-08-19T15:50:09Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, Aug 10, 2010 at 23:37, Ævar Arnfjörð Bjarmason wrote:\n\n> +test -x /usr/bin/time && test_set_prereq USR_BIN_TIME\n\nIt turns out that this fails on Solaris because its /usr/bin/time is different.\n\nMaybe this should just check if /usr/bin/time --version is ^GNU ?\n"},{"id":"148827","messageId":"20100824071453.GA3064@burratino","threadId":"24698","inReplyTo":"AANLkTikfwbtKt2WrxzRyKHnP3ySGtBF33=z_rJwjPx5Y@mail.gmail.com","subject":"Re: [PATCH v2] git-notes: Run partial expensive test everywhere","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-24T07:14:53Z","receivedAt":"2010-08-24T07:14:53Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n\n> It turns out that this fails on Solaris because its /usr/bin/time is different.\n\nOdd. Different how? As far as I can tell, all that test asks\nof time is to execv() its arguments and pass on a 0 exit status.\n\nAh, maybe this is it: perhaps /usr/bin/time sh runs /bin/sh. Does the\nfollowing help?\n\nPatch is against next. Untested except on Linux where it wouldn't\nmake a difference.\n-- 8< --\nSubject: t3302 (notes): Port to Solaris\n\nThe time_notes script, which uses POSIX shell features, is\ncurrently sometimes run with a non-POSIX /bin/sh.\n\nReported-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Jonathan Nieder \n---\ndiff --git a/t/t3302-notes-index-expensive.sh b/t/t3302-notes-index-expensive.sh\nindex 7c08e99..e35d781 100755\n--- a/t/t3302-notes-index-expensive.sh\n+++ b/t/t3302-notes-index-expensive.sh\n@@ -96,7 +96,7 @@ time_notes () {\n \tfor mode in no-notes notes\n \tdo\n \t\techo $mode\n-\t\t/usr/bin/time sh ../time_notes $mode $1\n+\t\t/usr/bin/time \"$SHELL_PATH\" ../time_notes $mode $1\n \tdone\n }\n \n-- \n"},{"id":"148847","messageId":"7vd3t8x8kk.fsf@alter.siamese.dyndns.org","threadId":"24698","inReplyTo":"20100824071453.GA3064@burratino","subject":"Re: [PATCH v2] git-notes: Run partial expensive test everywhere","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-24T16:46:19Z","receivedAt":"2010-08-24T16:46:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Odd. Different how? As far as I can tell, all that test asks\n> of time is to execv() its arguments and pass on a 0 exit status.\n>\n> Ah, maybe this is it: perhaps /usr/bin/time sh runs /bin/sh. Does the\n> following help?\n>\n> Patch is against next. Untested except on Linux where it wouldn't\n> make a difference.\n\nMakes sense, although I don't have access to Solaris boxes right now...\n\n> -- 8< --\n> Subject: t3302 (notes): Port to Solaris\n>\n> The time_notes script, which uses POSIX shell features, is\n> currently sometimes run with a non-POSIX /bin/sh.\n>\n> Reported-by: Ævar Arnfjörð Bjarmason \n> Signed-off-by: Jonathan Nieder \n> ---\n> diff --git a/t/t3302-notes-index-expensive.sh b/t/t3302-notes-index-expensive.sh\n> index 7c08e99..e35d781 100755\n> --- a/t/t3302-notes-index-expensive.sh\n> +++ b/t/t3302-notes-index-expensive.sh\n> @@ -96,7 +96,7 @@ time_notes () {\n> \tfor mode in no-notes notes\n> \tdo\n> \t\techo $mode\n> -\t\t/usr/bin/time sh ../time_notes $mode $1\n> +\t\t/usr/bin/time \"$SHELL_PATH\" ../time_notes $mode $1\n> \tdone\n> }\n> \n> -- \n"},{"id":"149223","messageId":"AANLkTi=PdF4VodZDHimq3XpxaYJBuZ0aXb+VU6hUsx3+@mail.gmail.com","threadId":"24698","inReplyTo":"7vd3t8x8kk.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] git-notes: Run partial expensive test everywhere","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-08-29T19:20:15Z","receivedAt":"2010-08-29T19:20:15Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, Aug 24, 2010 at 16:46, Junio C Hamano wrote:\n> Jonathan Nieder writes:\n>\n>> Odd.  Different how?  As far as I can tell, all that test asks\n>> of time is to execv() its arguments and pass on a 0 exit status.\n>>\n>> Ah, maybe this is it: perhaps /usr/bin/time sh runs /bin/sh.  Does the\n>> following help?\n>>\n>> Patch is against next.  Untested except on Linux where it wouldn't\n>> make a difference.\n>\n> Makes sense, although I don't have access to Solaris boxes right now...\n\nI tested it FWIW. It works, thanks both.\n"}]}