Skip to content

Add any_map support#158

Merged
kevmoo merged 4 commits into
masterfrom
any_map
May 16, 2018
Merged

Add any_map support#158
kevmoo merged 4 commits into
masterfrom
any_map

Conversation

@kevmoo

@kevmoo kevmoo commented May 16, 2018

Copy link
Copy Markdown
Collaborator

No description provided.

@kevmoo
kevmoo requested review from jakemac53 and natebosch May 16, 2018 18:17
test('empty', () async {
var builder = jsonSerializable(BuilderOptions.empty) as PartBuilder;
expect(builder, isNotNull);
expect(records, isEmpty);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Note that since the logs come through asynchronously you would never see them at this point, you would need to add an await null or something along those lines. Not really sure the absolute best way to deal with it honestly.

config[entry.key] = entry.value;

expect(() => jsonSerializable(new BuilderOptions(config)),
_throwsCastError);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Worth checking the actual message here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I don't think so. They will all be 'String is not a bool' etc.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ya I was just wondering if you wanted to check the cast messages were correct, but its probably overkill

as PartBuilder;
expect(builder, isNotNull);

expect(records.single.message,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't get how this is passing given my comment above...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

as discussed...

? null
: new Map<String, DateTime>.fromIterables(
(json['stringDateTimeMap'] as Map<String, dynamic>).keys,
(json['stringDateTimeMap'] as Map).keys.cast<String>(),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: should we be using .map instead?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Same-same, right? This seems easier...

@kevmoo
kevmoo merged commit 2c60bb3 into master May 16, 2018
@kevmoo
kevmoo deleted the any_map branch May 16, 2018 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants