Jeff King wrote:
> 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.

At the risk of showing my complete lack of knowledge about
these tests, I was wondering if the order mattered for the
other tests in t0011 and t0016.  I had assumed it didn't and
had something like this for testing (and a similar change to
test_oidmap() in t0016):

 diff --git i/t/t0011-hashmap.sh w/t/t0011-hashmap.sh
 index 9c96b3e3b1..9ed1c4f14d 100755
 --- i/t/t0011-hashmap.sh
 +++ w/t/t0011-hashmap.sh
 @@ -4,8 +4,8 @@ test_description='test hashmap and string hash functions'
  . ./test-lib.sh
  
  test_hashmap() {
 -      echo "$1" | test-tool hashmap $3 > actual &&
 -      echo "$2" > expect &&
 +      echo "$1" | test-tool hashmap $3 | sort >actual &&
 +      echo "$2" | sort >expect &&
        test_cmp expect actual
  }

You've got a more comprehensive patch and a proper commit
message, so this is really just a matter of curiosity.

-- 
Todd

Reply via email to