80/14ee7c20dea1164756c13108c3e9302bb0ea36

   1 Return-Path: <amdragon@gmail.com>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id 2845B431FD0\r
   6         for <notmuch@notmuchmail.org>; Sun,  3 Jul 2011 20:42:16 -0700 (PDT)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: -0.699\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-0.699 tagged_above=-999 required=5\r
  12         tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, FREEMAIL_FROM=0.001,\r
  13         RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
  14 Received: from olra.theworths.org ([127.0.0.1])\r
  15         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  16         with ESMTP id WZsA8IDmIQsm for <notmuch@notmuchmail.org>;\r
  17         Sun,  3 Jul 2011 20:42:15 -0700 (PDT)\r
  18 Received: from mail-qy0-f181.google.com (mail-qy0-f181.google.com\r
  19         [209.85.216.181]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
  20         (No client certificate requested)\r
  21         by olra.theworths.org (Postfix) with ESMTPS id ECE64431FB6\r
  22         for <notmuch@notmuchmail.org>; Sun,  3 Jul 2011 20:42:14 -0700 (PDT)\r
  23 Received: by qyk9 with SMTP id 9so3224310qyk.5\r
  24         for <notmuch@notmuchmail.org>; Sun, 03 Jul 2011 20:42:14 -0700 (PDT)\r
  25 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma;\r
  26         h=mime-version:sender:in-reply-to:references:date\r
  27         :x-google-sender-auth:message-id:subject:from:to:cc:content-type\r
  28         :content-transfer-encoding;\r
  29         bh=W6ge2dTNoDe1XlDUS23AIq8QGQnEUaX2+hdKL8Zl4iQ=;\r
  30         b=UhhtF7WPli4Bn9jRt1byNTSh5auQYpcyUAS25Y4rF6vvp21EAzTXRk9i/L4xoQ5de0\r
  31         f964nXGd8k+xrOLVC3XbtuzcSvsW1zZhLtzsDhaZOKJTprX0CjbzZulT4ovffidEcgme\r
  32         Udxcc8YRmynYahb2Nk+5xHCm3HM8vLwoA55S8=\r
  33 MIME-Version: 1.0\r
  34 Received: by 10.224.188.146 with SMTP id da18mr4523195qab.124.1309750934019;\r
  35         Sun, 03 Jul 2011 20:42:14 -0700 (PDT)\r
  36 Sender: amdragon@gmail.com\r
  37 Received: by 10.229.229.211 with HTTP; Sun, 3 Jul 2011 20:42:13 -0700 (PDT)\r
  38 In-Reply-To: <1309744743-8556-2-git-send-email-dmitry.kurochkin@gmail.com>\r
  39 References: <1309744743-8556-1-git-send-email-dmitry.kurochkin@gmail.com>\r
  40         <1309744743-8556-2-git-send-email-dmitry.kurochkin@gmail.com>\r
  41 Date: Sun, 3 Jul 2011 23:42:13 -0400\r
  42 X-Google-Sender-Auth: 7_W2JV2V_d4bwMTBKzqUavpscWQ\r
  43 Message-ID:\r
  44  <CAH-f9WuKrBpEjBaBet0d95oHXo1kWzGv6oa7zz7Zqe4JK=4N+Q@mail.gmail.com>\r
  45 Subject: Re: [PATCH 2/3] test: improve known broken tests support\r
  46 From: Austin Clements <amdragon@mit.edu>\r
  47 To: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>\r
  48 Content-Type: text/plain; charset=ISO-8859-1\r
  49 Content-Transfer-Encoding: quoted-printable\r
  50 Cc: notmuch@notmuchmail.org\r
  51 X-BeenThere: notmuch@notmuchmail.org\r
  52 X-Mailman-Version: 2.1.13\r
  53 Precedence: list\r
  54 List-Id: "Use and development of the notmuch mail system."\r
  55         <notmuch.notmuchmail.org>\r
  56 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  57         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  58 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  59 List-Post: <mailto:notmuch@notmuchmail.org>\r
  60 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  61 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  62         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  63 X-List-Received-Date: Mon, 04 Jul 2011 03:42:16 -0000\r
  64 \r
  65 Great idea!\r
  66 \r
  67 Just a heads-up, this will conflict with the first two patches in the\r
  68 atomicity series (though git may not detect the second conflict).\r
  69 Both are trivial patches, though, so the merge should be easy.\r
  70 \r
  71 Three minor comments below.\r
  72 \r
  73 On Sun, Jul 3, 2011 at 9:59 PM, Dmitry Kurochkin\r
  74 <dmitry.kurochkin@gmail.com> wrote:\r
  75 > There is existing support for broken tests. =A0But it is not convenient\r
  76 > to use. =A0The primary issue is that we have to maintain a set of\r
  77 > test_expect_*_failure functions which are equivalent to the normal\r
  78 > test_expect_* counterparts except for what functions are called for\r
  79 > result reporting. =A0The patch adds test_subtest_known_broken function\r
  80 > which marks a subset as broken, making the normal test_expect_*\r
  81 > functions behave as test_expect_*_failure. =A0All test_expect_*_failure\r
  82 > functions are removed. =A0Test_known_broken_failure_ is changed to\r
  83 > format details the same way as test_failure_ does.\r
  84 >\r
  85 > Another benefit of this change is that the diff when a broken test is\r
  86 > fixed would be small and nice.\r
  87 >\r
  88 > Documentation is updated accordingly.\r
  89 > ---\r
  90 > =A0test/README =A0 =A0 =A0| =A0 17 ++++++++---------\r
  91 > =A0test/test-lib.sh | =A0 53 +++++++++++++++-----------------------------=\r
  92 ---------\r
  93 > =A02 files changed, 23 insertions(+), 47 deletions(-)\r
  94 >\r
  95 > diff --git a/test/README b/test/README\r
  96 > index a245bf1..f926b9f 100644\r
  97 > --- a/test/README\r
  98 > +++ b/test/README\r
  99 > @@ -132,20 +132,19 @@ library for your script to use.\r
 100 > =A0 =A0<script>. =A0If it yields success, test is considered\r
 101 > =A0 =A0successful. =A0<message> should state what it is testing.\r
 102 >\r
 103 > - test_expect_failure <message> <script>\r
 104 > -\r
 105 > - =A0 This is NOT the opposite of test_expect_success, but is used\r
 106 > - =A0 to mark a test that demonstrates a known breakage. =A0Unlike\r
 107 > - =A0 the usual test_expect_success tests, which say "ok" on\r
 108 > - =A0 success and "FAIL" on failure, this will say "FIXED" on\r
 109 > - =A0 success and "still broken" on failure. =A0Failures from these\r
 110 > - =A0 tests won't cause -i (immediate) to stop.\r
 111 > -\r
 112 > =A0test_begin_subtest <message>\r
 113 >\r
 114 > =A0 =A0Set the test description message for a subsequent test_expect_equa=\r
 115 l\r
 116 > =A0 =A0invocation (see below).\r
 117 >\r
 118 > + test_subtest_known_broken\r
 119 > +\r
 120 > + =A0 Mark the current test as broken. =A0Such tests are expected to fail=\r
 121 .\r
 122 > + =A0 Unlike the normal tests, which say "PASS" on success and "FAIL" on\r
 123 > + =A0 failure, these will say "FIXED" on success and "BROKEN" on failure.\r
 124 > + =A0 Failures from these tests won't cause -i (immediate) to stop. =A0Th=\r
 125 is\r
 126 > + =A0 must be called before any test_expect_* function.\r
 127 \r
 128 Perhaps "A test must call this before any test_expect_* function"?\r
 129 There can be earlier test_expect_* calls from other tests.\r
 130 \r
 131 > +\r
 132 > =A0test_expect_equal <output> <expected>\r
 133 >\r
 134 > =A0 =A0This is an often-used convenience function built on top of\r
 135 > diff --git a/test/test-lib.sh b/test/test-lib.sh\r
 136 > index 22e387e..0cd4170 100755\r
 137 > --- a/test/test-lib.sh\r
 138 > +++ b/test/test-lib.sh\r
 139 > @@ -424,6 +424,7 @@ test_begin_subtest ()\r
 140 > =A0 =A0 =A0 =A0error "bug in test script: Missing test_expect_equal in ${=\r
 141 BASH_SOURCE[1]}:${BASH_LINENO[0]}"\r
 142 > =A0 =A0 fi\r
 143 > =A0 =A0 test_subtest_name=3D"$1"\r
 144 > + =A0 =A0test_subtest_known_broken_=3D\r
 145 > =A0 =A0 # Remember stdout and stderr file descriptors and redirect test\r
 146 > =A0 =A0 # output to the previously prepared file descriptors 3 and 4 (see\r
 147 > =A0 =A0 # below)\r
 148 > @@ -484,29 +485,6 @@ test_expect_equal_file ()\r
 149 > =A0 =A0 fi\r
 150 > =A0}\r
 151 >\r
 152 > -test_expect_equal_failure ()\r
 153 > -{\r
 154 > - =A0 =A0 =A0 exec 1>&6 2>&7 =A0 =A0 =A0 =A0 =A0# Restore stdout and stde=\r
 155 rr\r
 156 > - =A0 =A0 =A0 inside_subtest=3D\r
 157 > - =A0 =A0 =A0 test "$#" =3D 3 && { prereq=3D$1; shift; } || prereq=3D\r
 158 > - =A0 =A0 =A0 test "$#" =3D 2 ||\r
 159 > - =A0 =A0 =A0 error "bug in the test script: not 2 or 3 parameters to tes=\r
 160 t_expect_equal"\r
 161 > -\r
 162 > - =A0 =A0 =A0 output=3D"$1"\r
 163 > - =A0 =A0 =A0 expected=3D"$2"\r
 164 > - =A0 =A0 =A0 if ! test_skip "$@"\r
 165 > - =A0 =A0 =A0 then\r
 166 > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 if [ "$output" =3D "$expected" ]; then\r
 167 > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 test_known_broken_ok_ "$tes=\r
 168 t_subtest_name"\r
 169 > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 else\r
 170 > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 test_known_broken_failure_ =\r
 171 "$test_subtest_name"\r
 172 > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 testname=3D$this_test.$test=\r
 173 _count\r
 174 > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 echo "$expected" > $testnam=\r
 175 e.expected\r
 176 > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 echo "$output" > $testname.=\r
 177 output\r
 178 > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 fi\r
 179 > - =A0 =A0fi\r
 180 > -}\r
 181 > -\r
 182 > =A0NOTMUCH_NEW ()\r
 183 > =A0{\r
 184 > =A0 =A0 notmuch new | grep -v -E -e '^Processed [0-9]*( total)? file|Foun=\r
 185 d [0-9]* total file'\r
 186 > @@ -568,12 +546,20 @@ test_have_prereq () {\r
 187 > =A0# the text_expect_* functions instead.\r
 188 >\r
 189 > =A0test_ok_ () {\r
 190 > + =A0 =A0 =A0 if [ "$test_subtest_known_broken_" =3D 1 ]; then\r
 191 > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 test_known_broken_ok_ "$@"\r
 192 > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return\r
 193 > + =A0 =A0 =A0 fi\r
 194 > =A0 =A0 =A0 =A0test_success=3D$(($test_success + 1))\r
 195 > =A0 =A0 =A0 =A0say_color pass "%-6s" "PASS"\r
 196 > =A0 =A0 =A0 =A0echo " $@"\r
 197 > =A0}\r
 198 >\r
 199 > =A0test_failure_ () {\r
 200 > + =A0 =A0 =A0 if [ "$test_subtest_known_broken_" =3D 1 ]; then\r
 201 > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 test_known_broken_failure_ "$@"\r
 202 > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return\r
 203 > + =A0 =A0 =A0 fi\r
 204 > =A0 =A0 =A0 =A0test_failure=3D$(($test_failure + 1))\r
 205 > =A0 =A0 =A0 =A0say_color error "%-6s" "FAIL"\r
 206 > =A0 =A0 =A0 =A0echo " $1"\r
 207 > @@ -592,7 +578,10 @@ test_known_broken_ok_ () {\r
 208 > =A0test_known_broken_failure_ () {\r
 209 > =A0 =A0 =A0 =A0test_broken=3D$(($test_broken+1))\r
 210 > =A0 =A0 =A0 =A0say_color pass "%-6s" "BROKEN"\r
 211 > - =A0 =A0 =A0 echo " $@"\r
 212 > + =A0 =A0 =A0 echo " $1"\r
 213 > + =A0 =A0 =A0 shift\r
 214 > + =A0 =A0 =A0 echo "$@" | sed -e 's/^/ =A0 =A0 =A0 =A0/'\r
 215 > + =A0 =A0 =A0 if test "$verbose" !=3D "t"; then cat test.output; fi\r
 216 \r
 217 Would it be possible to keep just the one copy of the failure printing\r
 218 code in test_failure_?\r
 219 \r
 220 > =A0}\r
 221 >\r
 222 > =A0test_debug () {\r
 223 > @@ -636,20 +625,8 @@ test_skip () {\r
 224 > =A0 =A0 =A0 =A0esac\r
 225 > =A0}\r
 226 >\r
 227 > -test_expect_failure () {\r
 228 > - =A0 =A0 =A0 test "$#" =3D 3 && { prereq=3D$1; shift; } || prereq=3D\r
 229 > - =A0 =A0 =A0 test "$#" =3D 2 ||\r
 230 > - =A0 =A0 =A0 error "bug in the test script: not 2 or 3 parameters to tes=\r
 231 t-expect-failure"\r
 232 > - =A0 =A0 =A0 if ! test_skip "$@"\r
 233 > - =A0 =A0 =A0 then\r
 234 > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 test_run_ "$2"\r
 235 > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 if [ "$?" =3D 0 -a "$eval_ret" =3D 0 ]\r
 236 > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 then\r
 237 > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 test_known_broken_ok_ "$1"\r
 238 > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 else\r
 239 > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 test_known_broken_failure_ =\r
 240 "$1"\r
 241 > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 fi\r
 242 > - =A0 =A0 =A0 fi\r
 243 > +test_subtest_known_broken () {\r
 244 > + =A0 =A0 =A0 test_subtest_known_broken_=3D1\r
 245 \r
 246 The convention in test-lib appears to be "t" for true variables.\r
 247 \r
 248 > =A0}\r
 249 >\r
 250 > =A0test_expect_success () {\r
 251 > --\r
 252 > 1.7.5.4\r