Re: [PATCH 2/3] test: improve rebase -q test

From: SZEDER Gábor <szeder@ira.uka.de>
Date: Jun 10, 2013, 17:27 UTC
Message-ID: <20130610172708.GH2091@goldbirke>
In-Reply-To: <7vppvuj6wl.fsf@alter.siamese.dyndns.org>

On Mon, Jun 10, 2013 at 08:56:58AM -0700, Junio C Hamano wrote:

Show 23 quoted lines

> SZEDER Gábor <szeder@ira.uka.de> writes:
> 
> > On Sun, Jun 09, 2013 at 03:41:54PM -0500, Felipe Contreras wrote:
> >> There
> >> will not be a need for test_string_must_be_empty() just like there's
> >> no need for test_string_cmp().
> >
> > Actually, if there were a test_string_cmp(), that would be the test
> > helper function I used most often.
> 
> Hmm, there indeed are quite a many "At this point, the variable's
> value must be this" in the test scripts.  With things like this:
> 
>     t/t0002-gitfile.sh:     test "$SHA" = "$(git rev-list HEAD)"
> 
> we can go to the trash directory upon seeing a failure to run the
> command used on the RHS, but the value in $SHA is cumbersome to find
> out (either running it under sh -x or insert an extra echo before
> it), so such a helper function may be useful.
> 
> Do you really need a general comparison ("does A sort before B") or
> just equality?  If the latter, test_string_equal (or even
> string_equal) might be a better name for it.

Yeah, I need only equality.  Or at least it would be nice to have.

My main motivation is that, like in your example, in the bash prompt tests I only have to check a single line of output, but because of debuggability I always did:

  echo "(master)" >expected
  __git_ps1 >actual
  test_cmp expected actual

With such a helper function this could be reduced to a single line:

  test_string_equal "(master)" "$(__git_ps1)"

without loss of functionality or debuggability, because in case of a failure it would output something like this (bikesheddable, of course):

  Error:
    expected: "(master)"
    got: "((deadbeef...))"

And perhaps with a description as an optional third argument to help identify the failed check if multiple such checks are done in a single test, e.g. the test_rev_parse() helper in t/t1501-worktree.sh, 'setup: helper for testing rev-parse', which could be shortened as:

  test_string_equal "$1" "$(git rev-parse --is-bare-repository)" "bare"
  test_string_equal "$2" "$(git rev-parse --is-inside-git-dir)" "gitdir"
  test_string_equal "$3" "$(git rev-parse --is-inside-work-tree)" "worktree"

and if something goes wrong we'd get:

  Error: worktree
    expected: "true"
    got: "false"

Perhaps I could find some time in the days ahead to give it a go.

Gábor