Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a type error that occurs comparing two large maps with deepEquals. #2442

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix a type error that occurs comparing two large maps with deepEquals. #2442

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1bfa2f0
Fix a type error that occurs comparing two large maps with deepEquals.
matanlurey Dec 29, 2024
bf27c19
Update CHANGELOG, I think.
matanlurey Dec 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions pkgs/checks/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
- Add `containsMatchingInOrder` and `containsEqualInOrder` to replace the
combined functionality in `containsInOrder`.
- Replace `pairwiseComparesTo` with `pairwiseMatches`.
- Fix a bug where printing the result of a failed deep quality check would
fail with a `TypeError` when comparing large `Map` instances
- Increase SDK constraint to ^3.5.0.

## 0.3.0
Expand Down
6 changes: 5 additions & 1 deletion pkgs/checks/lib/src/describe.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,11 @@ Iterable<String> _prettyPrint(
Iterable<String> _prettyPrintCollection(
String open, String close, List<Iterable<String>> elements, int maxLength) {
if (elements.length > _maxItems) {
elements.replaceRange(_maxItems - 1, elements.length, [
// Force inference as List<String>, otherwise the type is
// List<Iterable<String>> which ends up as a type error in dart:collection
// when the underlying list is actually a List<List<String>>. See
// https://github.com/dart-lang/test/issues/2441 for more details.
elements.replaceRange(_maxItems - 1, elements.length, <List<String>>[
Copy link
Contributor

@jakemac53 jakemac53 Jan 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another option could be to make this a top level variable (const?)? Then we wouldn't have to force inference and it would be shared across invocations.

['...']
]);
}
Expand Down
59 changes: 59 additions & 0 deletions pkgs/checks/test/extensions/map_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,65 @@ void main() {
test('values', () {
check(_testMap).values.contains(1);
});
test('can be described failing compared to another large map', () {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the same issue exists for lists so it would be good to also test that (although I think your fix will work for both).

const expected = {
1: -5,
2: -4,
3: -4,
4: -3,
5: -3,
6: -2,
7: -2,
8: -1,
9: -1,
10: 0,
11: 0,
12: 1,
13: 1,
14: 2,
15: 2,
16: 3,
17: 3,
18: 4,
19: 4,
20: 5,
21: 5,
22: 6,
23: 6,
24: 7,
25: 7,
26: 8,
};
final actual = {
1: -4,
2: -4,
3: -3,
4: -3,
5: -2,
6: -2,
7: -1,
8: -1,
9: 0,
10: 0,
11: 0,
12: 1,
13: 1,
14: 2,
15: 2,
16: 3,
17: 3,
18: 4,
19: 4,
20: 5,
21: 5,
22: 6,
23: 6,
24: 7,
25: 7,
26: 8,
};
check(actual).not((a) => a.deepEquals(expected));
});

group('operator []', () {
test('succeeds for a key that exists', () {
Expand Down
Loading