git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: "SZEDER Gábor" <szeder.dev@gmail.com>
Cc: "Todd Zullinger" <tmz@pobox.com>, "René Scharfe" <l.s.r@web.de>,
	"Randall S. Becker" <rsbecker@nexbridge.com>,
	"'Junio C Hamano'" <gitster@pobox.com>,
	"Christian Couder" <chriscool@tuxfamily.org>,
	git@vger.kernel.org, git-packagers@googlegroups.com
Subject: Re: [ANNOUNCE] Git v2.23.0-rc0 - Initial test failures on NonStop
Date: Tue, 30 Jul 2019 21:23:37 -0400	[thread overview]
Message-ID: <20190731012336.GA13880@sigill.intra.peff.net> (raw)
In-Reply-To: <20190731005933.GA9610@sigill.intra.peff.net>

On Tue, Jul 30, 2019 at 08:59:33PM -0400, Jeff King wrote:

> > OTOH, this is not just any hashmap, but an oidmap, and I could imagine
> > that there might be use cases where it would be beneficial if the
> > iteration order were to match the oid order (but don't know whether we
> > actually have such a use case).
> 
> I don't think we can promise anything about iteration order. This test
> is relying on the order at least being deterministic between runs, but
> if we added a new entry and had to grow the table, all bets are off.
> 
> So regardless of the endian thing above, it probably does make sense for
> any hashmap iteration output to be sorted before comparing. That goes
> for t0011, too; it doesn't have this endian thing, but it looks to be
> relying on hash order that could change if we swapped out hash
> functions.

So here's an actual patch.

-- >8 --
Subject: [PATCH] t: sort output of hashmap iteration

The iteration order of a hashmap is undefined, and may depend on things
like the exact set of items added, or the table has been grown or
shrunk. In the case of an oidmap, it even depends on endianness, because
we take the oid hash by casting sha1 bytes directly into an unsigned
int.

Let's sort the test-tool output from any hash iterators. In the case of
t0011, this is just future-proofing. But for t0016, it actually fixes a
reported failure on the big-endian s390 and nonstop ports.

I didn't bother to teach the helper functions to optionally sort output.
They are short enough that it's simpler to just repeat them inline for
the iteration tests than it is to add a --sort option.

Reported-by: Randall S. Becker <rsbecker@nexbridge.com>
Signed-off-by: Jeff King <peff@peff.net>
---
 t/t0011-hashmap.sh | 58 ++++++++++++++++++++++++++++------------------
 t/t0016-oidmap.sh  | 30 +++++++++++++++---------
 2 files changed, 55 insertions(+), 33 deletions(-)

diff --git a/t/t0011-hashmap.sh b/t/t0011-hashmap.sh
index 9c96b3e3b1..5343ffd3f9 100755
--- a/t/t0011-hashmap.sh
+++ b/t/t0011-hashmap.sh
@@ -170,31 +170,45 @@ NULL
 '
 
 test_expect_success 'iterate' '
-
-test_hashmap "put key1 value1
-put key2 value2
-put fooBarFrotz value3
-iterate" "NULL
-NULL
-NULL
-key2 value2
-key1 value1
-fooBarFrotz value3"
-
+	test-tool hashmap >actual.raw <<-\EOF &&
+	put key1 value1
+	put key2 value2
+	put fooBarFrotz value3
+	iterate
+	EOF
+
+	cat >expect <<-\EOF &&
+	NULL
+	NULL
+	NULL
+	fooBarFrotz value3
+	key1 value1
+	key2 value2
+	EOF
+
+	sort <actual.raw >actual &&
+	test_cmp expect actual
 '
 
 test_expect_success 'iterate (case insensitive)' '
-
-test_hashmap "put key1 value1
-put key2 value2
-put fooBarFrotz value3
-iterate" "NULL
-NULL
-NULL
-fooBarFrotz value3
-key2 value2
-key1 value1" ignorecase
-
+	test-tool hashmap ignorecase >actual.raw <<-\EOF &&
+	put key1 value1
+	put key2 value2
+	put fooBarFrotz value3
+	iterate
+	EOF
+
+	cat >expect <<-\EOF &&
+	NULL
+	NULL
+	NULL
+	fooBarFrotz value3
+	key1 value1
+	key2 value2
+	EOF
+
+	sort <actual.raw >actual &&
+	test_cmp expect actual
 '
 
 test_expect_success 'grow / shrink' '
diff --git a/t/t0016-oidmap.sh b/t/t0016-oidmap.sh
index bbe719e950..31f8276ba8 100755
--- a/t/t0016-oidmap.sh
+++ b/t/t0016-oidmap.sh
@@ -86,17 +86,25 @@ NULL"
 '
 
 test_expect_success 'iterate' '
-
-test_oidmap "put one 1
-put two 2
-put three 3
-iterate" "NULL
-NULL
-NULL
-$(git rev-parse two) 2
-$(git rev-parse one) 1
-$(git rev-parse three) 3"
-
+	test-tool oidmap >actual.raw <<-\EOF &&
+	put one 1
+	put two 2
+	put three 3
+	iterate
+	EOF
+
+	# sort "expect" too so we do not rely on the order of particular oids
+	sort >expect <<-EOF &&
+	NULL
+	NULL
+	NULL
+	$(git rev-parse one) 1
+	$(git rev-parse two) 2
+	$(git rev-parse three) 3
+	EOF
+
+	sort <actual.raw >actual &&
+	test_cmp expect actual
 '
 
 test_done
-- 
2.23.0.rc0.426.gbdee707ba7


  reply	other threads:[~2019-07-31  1:23 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-30 17:08 [ANNOUNCE] Git v2.23.0-rc0 - Initial test failures on NonStop Randall S. Becker
2019-07-30 17:31 ` Junio C Hamano
2019-07-30 18:09   ` Matheus Tavares Bernardino
2019-07-30 18:10   ` Randall S. Becker
2019-07-30 18:35     ` Junio C Hamano
2019-07-30 19:45 ` Jeff King
2019-07-30 20:25   ` Randall S. Becker
2019-07-30 19:49 ` Todd Zullinger
2019-07-30 20:02   ` Jeff King
2019-07-30 20:39     ` Junio C Hamano
2019-07-30 20:56     ` SZEDER Gábor
2019-07-31  0:59       ` Jeff King
2019-07-31  1:23         ` Jeff King [this message]
2019-07-31  1:27           ` Jeff King
2019-07-31  1:59           ` Todd Zullinger
2019-07-31  3:27             ` Jeff King
2019-07-31  3:53               ` Jeff King
2019-07-31 17:17                 ` Junio C Hamano
2019-07-31 21:22                   ` non-cryptographic hash algorithms in git Jeff King
2019-07-31  4:06               ` [ANNOUNCE] Git v2.23.0-rc0 - Initial test failures on NonStop René Scharfe
2019-07-31  4:30                 ` Jeff King
2019-07-31  6:04               ` Todd Zullinger
2019-07-31 16:57         ` Junio C Hamano
2019-07-30 20:27   ` Randall S. Becker

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: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20190731012336.GA13880@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=chriscool@tuxfamily.org \
    --cc=git-packagers@googlegroups.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=l.s.r@web.de \
    --cc=rsbecker@nexbridge.com \
    --cc=szeder.dev@gmail.com \
    --cc=tmz@pobox.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.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).