Skip to content
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

[HttpClient] Fix not calling the on progress callback when canceling a MockResponse #49796

Merged

Conversation

fancyweb
Copy link
Contributor

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets -
License MIT
Doc PR -

I want to assert that a response is canceled under certain circumstances.

My first approach was to get the info from the MockResponse I pass as the response factory to the MockHttpClient but this one is lost since a new instance is created when the request method is called.

My next approach was to wrap the MockHttpClient I pass to the class I'm testing with a TraceableHttpClient. But it doesn't work because the traced info is outdated. The problem seems to be that the on-progress callback is not called when a MockResponse is canceled.

@nicolas-grekas
Copy link
Member

Is the callback called when a real response is canceled?
What about the "info" of the reponse? Don't they give the needed clue?

@fancyweb
Copy link
Contributor Author

Is the callback called when a real response is canceled?

Yes.

What about the "info" of the response? Don't they give the needed clue?

No, since the original mock response reference is lost, we can't access it and get the info from it.

@fancyweb
Copy link
Contributor Author

After retesting, it works with CurlHttpClient and not with NativeHttpClient... So it might not be the best solution.

@stof
Copy link
Member

stof commented Mar 29, 2023

No, since the original mock response reference is lost, we can't access it and get the info from it.

Can you check what happens when you get the info on the MockResponse being returned by $client->request() ?

@fancyweb
Copy link
Contributor Author

The info on the MockResponse returned by $client->request() is good but I don't have access to it.

Here's a simplified gist that highlights the problem: https://gist.github.com/fancyweb/0a54ae6293eaf02c3823e9b921cada5a

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

Maybe we could try to reuse the $mock request if it's a MockResponse? Not as a bugfix but as an improvement? I don't know if this would be possible of not :)

@nicolas-grekas
Copy link
Member

Good catch, thanks @fancyweb.

@nicolas-grekas nicolas-grekas merged commit ba8080d into symfony:5.4 Mar 30, 2023
9 of 11 checks passed
@fancyweb fancyweb deleted the http-client/fix-mock-response-cancel branch March 30, 2023 14:26
This was referenced Mar 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants