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.

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

pkg/matcher: isEmpty should fail for null or non-collections. #21562

Closed
DartBot opened this Issue Nov 10, 2014 · 7 comments

Comments

Projects
None yet
3 participants
@DartBot
Copy link

DartBot commented Nov 10, 2014

This issue was originally filed by bl...@google.com


As there is no isNotEmpty matcher, you are forced to use isNot(isEmpty). However, expect(null, isNot(isEmpty)) passes because isEmpty returns false for non-collections or null. So, likewise, any other non-collection object would also wrongly pass such an expect call.

My suggested solution would be for isEmpty to throw an exception for "wrong" types.

Alternatively, an isNotEmpty matcher could be added.

Thanks,
 Andreas

@lrhn

This comment has been minimized.

Copy link
Member

lrhn commented Nov 11, 2014

Removed Type-Defect label.
Added Type-Enhancement, Area-Pkg, Pkg-Unittest, Triaged labels.

@kevmoo

This comment has been minimized.

Copy link
Member

kevmoo commented Nov 24, 2014

@DartBot

This comment has been minimized.

Copy link

DartBot commented Nov 24, 2014

This comment was originally written by bl...@google.com


I'd call this a defect because except(myCollection, isNot(isEmpty)) not failing for null references can hide nasty bugs.

@kevmoo

This comment has been minimized.

Copy link
Member

kevmoo commented Nov 24, 2014

Fair.

We can wrap up a new isNotEmpty up as the new feature – depending on how this lands.

This would be an easy fix in pkg/matcher. Patches welcome.


Removed Type-Enhancement label.
Added Type-Defect label.

@kevmoo

This comment has been minimized.

Copy link
Member

kevmoo commented Nov 24, 2014

Removed Pkg-Unittest label.
Added Pkg-matcher label.

@kevmoo

This comment has been minimized.

Copy link
Member

kevmoo commented Jan 17, 2015

@DartBot

This comment has been minimized.

Copy link

DartBot commented Jun 5, 2015

This issue has been moved to dart-lang/matcher#23.

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment