On Tue, Jul 30, 2019 at 09:59:17PM -0400, Todd Zullinger wrote: > 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. I think the order does matter for those ones. E.g., the ones that run "get" want to make sure they're seeing the values in the same order in which they were requested. One thing that makes it all a bit funky is that the "put" lines also output the old value (which is what all those NULLs) are. And I think that solves my "value3" puzzlement from earlier. It is not part of the iteration at all, but rather the result of the duplicate "put". That would perhaps be clearer if the "hashmap" tool actually did the sorting itself (so we'd sort _just_ the iteration, not the whole output). Something like this, though I'm on the fence about whether it is worth it: diff --git a/t/helper/test-hashmap.c b/t/helper/test-hashmap.c index aaf17b0ddf..9f6901666e 100644 --- a/t/helper/test-hashmap.c +++ b/t/helper/test-hashmap.c @@ -2,6 +2,7 @@ #include "git-compat-util.h" #include "hashmap.h" #include "strbuf.h" +#include "string-list.h" struct test_entry { @@ -221,10 +222,18 @@ int cmd__hashmap(int argc, const char **argv) } else if (!strcmp("iterate", cmd)) { + struct string_list sorted = STRING_LIST_INIT_NODUP; + struct string_list_item *item; struct hashmap_iter iter; hashmap_iter_init(&map, &iter); - while ((entry = hashmap_iter_next(&iter))) - printf("%s %s\n", entry->key, get_value(entry)); + while ((entry = hashmap_iter_next(&iter))) { + item = string_list_append(&sorted, entry->key); + item->util = (void *)get_value(entry); + } + string_list_sort(&sorted); + for_each_string_list_item(item, &sorted) + printf("%s %s\n", item->string, (const char *)item->util); + string_list_clear(&sorted, 0); } else if (!strcmp("size", cmd)) {