All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"GIT Mailing-list" <git@vger.kernel.org>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: Re: [RFC/PATCH v2] t3300-*.sh: Fix a TAP parse error
Date: Wed, 25 Jul 2012 20:46:53 +0100	[thread overview]
Message-ID: <50104D2D.70109@ramsay1.demon.co.uk> (raw)
In-Reply-To: <20120724191707.GA5210@burratino>

Jonathan Nieder wrote:
>> This version of the patch only moves code to determine the test
>> prerequisite to the outer level, while leaving the 'setup' aspects
>> of the first test in place.
> 
> I guess I don't see the point.

You don't see the point of fixing the TAP Parse error? :-D

>                                The current convention of "don't do
> anything complicated outside test assertions" is easy to explain.
> What new convention are you suggesting to replace it?

Hmm, well I guess I'm not going to suggest anything!

However, what is "anything complicated"?

At the end of test-lib.sh we find:

    # test whether the filesystem supports symbolic links
    ln -s x y 2>/dev/null && test -h y 2>/dev/null && test_set_prereq SYMLINKS
    rm -f y

    # When the tests are run as root, permission tests will report that
    # things are writable when they shouldn't be.
    test -w / || test_set_prereq SANITY

Is this too complicated? If not, why not? If yes, should it be added to
a test assertion?

Would it be acceptable for me to add some code, here at the end of
test-lib.sh, to set the TABS_IN_FILENAME test prerequisite and use it
in tests t3300-funny-names.sh, t3902-quoted.sh, t3600-rm.sh,
t4016-diff-quote.sh and t4135-apply-weird-filenames.sh?

How about some of the test "library" files:

diff-lib.sh              lib-git-p4.sh            lib-read-tree.sh
gitweb-lib.sh            lib-git-svn.sh           lib-rebase.sh
lib-bash.sh              lib-gpg.sh*              lib-t6000.sh
lib-credential.sh*       lib-httpd.sh             lib-terminal.sh
lib-cvs.sh               lib-pager.sh             test-lib-functions.sh
lib-diff-alternative.sh  lib-patch-mode.sh        test-lib.sh
lib-gettext.sh           lib-prereq-FILEMODE.sh
lib-git-daemon.sh        lib-read-tree-m-3way.sh

Several of these files contain executable code (rather than just a
library of functions). For example, look at lib-cvs.sh, lib-httpd.sh,
and lib-prereq-FILEMODE.sh. Is this code too complicated?

Would it be acceptable for me to create an lib-prereq-TABSINFILE.sh
file so that I could source it only in the test files that require
the TABS_IN_FILENAME prerequisite?

ATB,
Ramsay Jones

      reply	other threads:[~2012-07-25 19:49 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-24 18:48 [RFC/PATCH v2] t3300-*.sh: Fix a TAP parse error Ramsay Jones
2012-07-24 19:17 ` Jonathan Nieder
2012-07-25 19:46   ` Ramsay Jones [this message]

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

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

  git send-email \
    --in-reply-to=50104D2D.70109@ramsay1.demon.co.uk \
    --to=ramsay@ramsay1.demon.co.uk \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.