-
Notifications
You must be signed in to change notification settings - Fork 36
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
Refactor to kotlin.assert and power-assert: example #191
Conversation
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.
Looks great @tibtof! Two smalls nits.
Sorry for the slow review, I was sick all week 🤒
response.status shouldBe HttpStatusCode.OK | ||
response.body<TagsResponse>().tags.toSet() shouldBe emptySet() | ||
assert(response.status == HttpStatusCode.OK) | ||
assert(response.body<TagsResponse>().tags.isEmpty()) |
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 think here we need to use == emptyList()
in order to get a proper error message from power-assert.
assert(response.body<TagsResponse>().tags.isEmpty()) | |
assert(response.body<TagsResponse>().tags == emptyList()) |
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.
@nomisRev don't worry about it, and I hope you are well. I will also refactor the other asserts before resubmitting the review. |
Hi @nomisRev, I implemented your suggestions, refactored the rest of the assertions and rebased the latest main. Please review it again. Thanks! |
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.
Looks great! Thank you @tibtof.
Sorry for the late response, I needed some time off.
A first example for #173