-
Notifications
You must be signed in to change notification settings - Fork 366
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
Add tests for Message #90
Conversation
@nex3 the tests were straight up copied from Only other question was around |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only other question was around
contentLength
. So inshelf
it'll automatically get set to0
if body isnull
. So wondering if we need to do anything toMessage.isEmpty
.
I'm not sure if I follow. If the body is null
, Message.isEmpty
should be true
, right?
pubspec.yaml
Outdated
test: '^0.12.18' | ||
# Override dependency on package_resolver to enable test | ||
dependency_overrides: | ||
package_resolver: '^1.0.0' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's with all the formatting changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can just move it all back to using "
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted
There isn't currently a check for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There isn't currently a check for
null
its just checking for length being 0.
It looks like Body.contentLength
is 0
when the user passes in a null
body, so that should be fine.
test: "^0.12.18" | ||
# Override dependency on package_resolver to enable test | ||
dependency_overrides: | ||
package_resolver: '^1.0.0' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be nice to add some newlines to make the pubspec a bit more readable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At this point I was figuring we just fix it up when releasing 0.12.0 into the wild
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure.
This adds tests for the
Message
class and begins the migration fromunittest
totest
.