Fix AssetsEntry::equals
#143355
Fix AssetsEntry::equals
#143355
Conversation
andrewkolos
added
the
tool
| } | ||
| return null; | ||
| } | ||
|
|
There was a problem hiding this comment.
Does ordering matter when you're checking the list of flavors?
For example, do [flavor1, flavor2] and [flavor2, flavor1] mean 2 different things?
There was a problem hiding this comment.
addressed in commit express flavors as an unordered set, rather than a list
Logically, they mean the same thing. To capture this idea, I've changed flavors to be a set rather than a list.
I am not 100% sure on the hashing function I wrote, but the chance of unexpected collision should be low. We also don't use it anywhere (only the equals implementation is used in tests).
| case 'licenses': | ||
| if (yamlValue is! YamlList) { | ||
| errors.add('Expected "$yamlKey" to be a list of files, but got $yamlValue (${yamlValue.runtimeType})'); | ||
| } else if (yamlValue.isEmpty) { |
There was a problem hiding this comment.
Do we still need to check within this case if the yamlValue.isEmpty and break?
There was a problem hiding this comment.
Empty lists should pass the _validateList check, so I don't think so.
andrewkolos
added
the
autosubmit
2 participants
In service of #143348.
Issue. The
equalsimplementation ofAssetsEntryis incorrect. It comparesflavorslists using reference equality. This PR addresses this.This also adds a test to make sure valid asset
flavorsdeclarations are parsed correctly.While we are here, this PR also includes a couple of refactorings:
flutter_manifest_test.dartis a bit large. To better match our style guide, I've factored out some related tests into their own file._validateListTypefunction influtter_manifest.dart:_validateListand now accepts anObject?instead of anYamlList. If the argument is null, an appropriate error message is contained in the output. This saves callers that are only interested in validation from having to write their own null-check, which they all did before.