Fix abi key for local golden file testing

[Piinks]

Fixes #148022

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
• I signed the CLA.
• I listed at least one issue that this PR fixes in the description above.
• I updated/added relevant documentation (doc comments with ///).
• I added new tests to check the change I am making, or this PR is test-exempt.
• I followed the breaking change policy and added Data Driven Fixes where supported.
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[goderbauer]

LGTM

Thank you for digging into this!

[goderbauer]

Is this also relevant for _getKeysJSON?

[Piinks]

It is not, since those keys are sorted by Gold when they are ingested. It matters here because we are using the keys to generate an md5 sum to look up the image, and being out of order means we get the wrong result.

[Hixie]

"alphabetically, after grouping by letter case" is just "sort by Unicode scalar value" which is just the default string sort in Dart, so one way to do this would be to create the map first, then get the keys, sort them, and add the key/value pairs to a new map in the sorted order. That would avoid future bugs when someone adds a new key manually and gets the order wrong. It would cost a few CPU cycles and some memory though.

[Hixie]

ah, just saw the comment below. Yeah, the order here is just the default order for Dart sorting.

void main() {
  Map<String, int> test = {'a': 3, 'B': 2, 'A': 1, 'b': 4};
  print(test);
  print(test.keys.toList()..sort());
  Map<String, int> sorted = {};
  for (String key in test.keys.toList()..sort()) {
    sorted[key] = test[key]!;
  }
  print(sorted);
}

->

{a: 3, B: 2, A: 1, b: 4}
[A, B, a, b]
{A: 1, B: 2, a: 3, b: 4}

[Piinks]

This week I learned Dart does that by default! Awesome! Yeah I'll update this to sort keys so we don't have to worry about it recurring. :)

[goderbauer]

Could we add an assert here (or a test elsewhere) that enforces the sorting rules and breaks if this is messed up in the future?

[Piinks]

I can, it would be pretty complicated though (and probably not today) to account for all the cases where we do or don't include a given key. We don't add keys very often, I was kind of hoping the comment would suffice. WDYT?

[kjlubick]

It does seem a touch fragile. Would it be worth adding some code that somehow alphabetizes the map keys first? If so, then I would imagine it would be easier to add a few tests for that logic: input(map) output(md5 hash).

[goderbauer]

Isn't this just:

  assert(() {
    List sorted = keys.keys.toList()..sort();
    return listEquals(sorted, keys.keys.toList());
  }());

Although, maybe doing this on every golden check is kinda slowing things down? I'd be okay with leaving this out as well.

Edit: Thinking about this some more, I do think we should guard against this error a little stronger than just a comment...

[goderbauer]

Another option would be to just throw these key-values into a self-ordering data structure, e.g. https://main-api.flutter.dev/flutter/dart-collection/SplayTreeMap-class.html

[Piinks]

Oh! Will dart sort by case before alphabetical order? I didn't think it would.. let me check

[goderbauer]

LGTM