unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
From: David Bremner <david@tethera.net>
To: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>, notmuch@notmuchmail.org
Subject: Re: [PATCH] test: reset known_broken status in test_expect_equal and test_expect_equal_file
Date: Sun, 11 Sep 2011 20:51:47 -0300	[thread overview]
Message-ID: <877h5ed4do.fsf@zancas.localnet> (raw)
In-Reply-To: <87r53mveq9.fsf@gmail.com>

On Mon, 12 Sep 2011 03:30:54 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> Hi David.
> IMHO this is not a good idea, because:
> 
> 1. It introduces multiple places where the flag is reset.  If new
>    test_expect_* functions are added in the future, there would be more
>    of these.  So it brings us more complex code, code duplication, more
>    chances for bugs, etc.

Well, I'm not sure how to solve this without code duplication, since
there is no test_end_subtest. We could make one, but that seems pretty
intrusive.

> 2. No support for tests with multiple test_expect_* calls.  I do not
>    know if it is used or works now, but the patch certainly does not

Looking at the code for test_expect_equal_* (note that these two are
very different than test_expect_success and test_expect_failure), 
several things are reset already, so I don't think it will work even
before my patch to call those routines twice.

> 3. I thought that every test must start with a test_begin_subtest call.
>    So it is the right place to put all subtest initialization code to.
>    Is this not correct?  If it is correct, then I do not understand why
>    we should support buggy tests by hiding (some of) their bugs.  Why do
>    we need it?

I could not find anything in the docs (or test-lib.sh) that says
test_begin_subtest must be called before test_expect_success or
test_expect_failure. Indeed if test_begin_subtest is called first, the
extra message argument to those two does not really make sense.

What brought this to my attention is the atomicity test introduced by
amdragon, which does exactly

test_begin_subtest
test_expect_equal_failure
test_expect_success

d

  reply	other threads:[~2011-09-11 23:51 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-04  1:59 [PATCH 1/3] test: update documentation for test_emacs in test/README Dmitry Kurochkin
2011-07-04  1:59 ` [PATCH 2/3] test: improve known broken tests support Dmitry Kurochkin
2011-07-04  3:42   ` Austin Clements
2011-07-04  3:46     ` Dmitry Kurochkin
2011-07-04  1:59 ` [PATCH 3/3] test: add emacs test for hiding a message following an HTML part Dmitry Kurochkin
2011-09-26 11:01   ` David Bremner
2011-09-26 17:25     ` Dmitry Kurochkin
2011-10-01 11:51       ` David Bremner
2011-10-02  1:45         ` Dmitry Kurochkin
2011-10-03 12:39           ` Thomas Jost
2011-10-03 13:38             ` Dmitry Kurochkin
2011-10-03 14:27             ` David Bremner
2011-10-03 16:47               ` [PATCH 00/13] Test prereqs and screen-based Emacs tests Thomas Jost
2011-11-01 19:54                 ` Pieter Praet
2011-11-13 17:46                   ` David Bremner
2011-11-16 13:03                     ` Pieter Praet
2011-10-03 16:47               ` [PATCH 01/13] test: define a helper function for defining prereqs on executables Thomas Jost
2011-10-03 16:47               ` [PATCH 02/13] test: add 'GnuPG' prereq to dependent 'crypto' tests Thomas Jost
2011-10-03 16:47               ` [PATCH 03/13] test: add 'Emacs' " Thomas Jost
2011-11-01 19:56                 ` Pieter Praet
2011-11-01 20:15                   ` Pieter Praet
2011-10-03 16:47               ` [PATCH 04/13] test: add 'Emacs' prereq to dependent 'emacs' tests Thomas Jost
2011-11-01 19:57                 ` Pieter Praet
2011-11-01 20:08                   ` Pieter Praet
2011-10-03 16:47               ` [PATCH 05/13] test: add 'Emacs' prereq to dependent 'emacs-large-search-buffer' tests Thomas Jost
2011-10-03 16:47               ` [PATCH 06/13] test: run emacs inside screen Thomas Jost
2011-11-10  7:36                 ` Jameson Graef Rollins
2011-11-10  8:10                   ` Thomas Jost
2011-10-03 16:47               ` [PATCH 07/13] test: avoid using screen(1) configuration files Thomas Jost
2011-10-03 16:47               ` [PATCH 08/13] test: do not set frame width in emacs Thomas Jost
2011-10-03 16:47               ` [PATCH 09/13] test: `notmuch-show-advance-and-archive' with invisible signature Thomas Jost
2011-10-03 16:47               ` [PATCH 10/13] emacs: improve hidden signatures handling in notmuch-show-advance-and-archive Thomas Jost
2011-10-03 16:47               ` [PATCH 11/13] emacs: remove no longer used functions from notmuch-show.el Thomas Jost
2011-10-03 16:47               ` [PATCH 12/13] emacs: remove unused `point-invisible-p' function Thomas Jost
2011-10-03 16:47               ` [PATCH 13/13] test: make smtp-dummy work with Emacs 24 Thomas Jost
2011-11-13 19:09                 ` David Bremner
2011-12-15 12:14   ` [PATCH 3/3] test: add emacs test for hiding a message following an HTML part David Bremner
2011-12-15 17:27     ` Tom Prince
2011-12-16 18:51       ` Dmitry Kurochkin
2011-07-04  4:07 ` [PATCH v2 0/3] improved broken tests support and test for a bug Dmitry Kurochkin
2011-07-04  4:07   ` [PATCH v2 1/3] test: update documentation for test_emacs in test/README Dmitry Kurochkin
2011-07-04  4:07   ` [PATCH v2 2/3] test: improve known broken tests support Dmitry Kurochkin
2011-09-11 23:11     ` [PATCH] test: reset known_broken status in test_expect_equal and test_expect_equal_file david
2011-09-11 23:30       ` Dmitry Kurochkin
2011-09-11 23:51         ` David Bremner [this message]
2011-09-12  0:26           ` Dmitry Kurochkin
2011-09-13  2:41             ` [PATCH] test: reset test_subtest_known_broken_ after each success/failure david
2011-09-13 10:19               ` Dmitry Kurochkin
2011-09-13 11:39                 ` David Bremner
2011-07-04  4:07   ` [PATCH v2 3/3] test: add emacs test for hiding a message following an HTML part Dmitry Kurochkin
2011-09-10 18:08   ` [PATCH v2 0/3] improved broken tests support and test for a bug David Bremner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://notmuchmail.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=877h5ed4do.fsf@zancas.localnet \
    --to=david@tethera.net \
    --cc=dmitry.kurochkin@gmail.com \
    --cc=notmuch@notmuchmail.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).