Re: [PATCH 1/2] t6300: test sort with multiple keys

From: Kacper Kornet <draenog@pld-linux.org>
Date: Aug 20, 2012, 05:24 UTC
Message-ID: <20120820052429.GF1076@camk.edu.pl>
In-Reply-To: <7vk3wuo0sa.fsf@alter.siamese.dyndns.org>

On Sun, Aug 19, 2012 at 05:38:29PM -0700, Junio C Hamano wrote:

> Kacper Kornet <draenog@pld-linux.org> writes:

> > Documentation of git-for-each-ref says that --sort=<key> option can be
> > used multiple times, in which case the last key becomes the primary key.
> > However this functionality was never checked in test suite and is
> > currently broken. This commit adds appropriate test in preparation for fix.

> > Signed-off-by: Kacper Kornet <draenog@pld-linux.org>
> > ---

> Thanks.

Show 11 quoted lines

> > +test_expect_success 'Create branches to test sort with multiple keys' '
> > +	git checkout -b Branch1 &&
> > +	echo foo >> one &&
> > +	git commit -a -m "Branch1 commit" &&
> > +	git checkout -b Branch2 &&
> > +	echo foo >> one &&
> > +	git commit -a -m "Branch2 commit"
> > +'
> > +
> > +test_atom refs/heads/Branch1 objectname 32fca05e9f638021a123a84226acf17756acc18b
> > +test_atom refs/heads/Branch2 objectname 194a5b89ac661a114566ba4374bc06c2797539f3

Show 5 quoted lines

> Do these need to be "Branch[12]", not "branch[12]" for the code to
> exhibit the bug?  If not, please don't be creative in names like
> these.  On case corrupting filesystems you may write Branch1 and
> they may come back as branch1, but that is not what we are testing
> here.

Branches names can be lowercased. Only the commit messages should be preserved as they produce the test depends on the lexicographical order of created SHA1s.

> > @@ -296,6 +325,8 @@ test_expect_success 'Check short refname format' '
> >  '

> >  cat >expected <<EOF
> > +
> > +
> >  origin/master

> What are these blank line outputs?

The upstreams of Branch1 and Branch2.

> >  EOF

> > @@ -309,7 +340,7 @@ cat >expected <<EOF
> >  EOF

Show 5 quoted lines

> >  test_expect_success 'Check short objectname format' '
> > -	git for-each-ref --format="%(objectname:short)" refs/heads >actual &&
> > +	git for-each-ref --format="%(objectname:short)" refs/heads/master >actual &&
> >  	test_cmp expected actual
> >  '

Show 5 quoted lines

> All in all, I have to wonder if you can limit the updates to other
> unrelated tests if you added a new test near the end.  Also doesn't
> the existing test already create enough refs to let you sort with
> multiple keys and demonstrate the breakage already, without adding new
> refs and objects?

My intention was to group all tests to sort in one place. But if the preferred place for a new one is at the end, then it is possible to find the adequate refs among existing ones.

-- 
  Kacper Kornet