-
Notifications
You must be signed in to change notification settings - Fork 47
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
MBS-10301 [test-okhttp] add nothingCaptured check #1322
Conversation
bae4051
to
ccf118f
Compare
ccf118f
to
07b12d9
Compare
assertThat("", requests.size == 1) | ||
assertThat( | ||
"Single request should be captured, Currently matched: $requests", | ||
requests.size == 1 |
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.
Could hasSize(1)
matcher give better error message?
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.
Btw, should this single size check be here?
Sometimes there are multiple requests at the same time, that have similar paths, and I have to do some workarounds to bypass this check.
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 have an alternative proposal.
Why don't we give a list of all requested requests?
The client can easily check collection by any condition.
Trying to express all use cases without accessing requests bloats API.
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.
Yeah, I would partially agree with that.
RequestChecks interface seems to be trivial and might be eliminated.
But FormUrlEncodedBodyChecks should be preserved as it hides raw data internals.
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'd merge this PR, because discussion is out of it's scope
Btw, do you want to make such change? It would be be a nice task to do
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.
@dsvoronin I'm not quite sure, this's something crucial to do now.
But I'll bear in mind this task, we just need to figure out a way to do this
No description provided.