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

Make GuzzleRestClient Mockable / Testable #31

Open
drzraf opened this issue Feb 4, 2022 · 1 comment
Open

Make GuzzleRestClient Mockable / Testable #31

drzraf opened this issue Feb 4, 2022 · 1 comment

Comments

@drzraf
Copy link
Contributor

drzraf commented Feb 4, 2022

https://github.com/eventfarm/restforcephp/blob/master/src/Rest/GuzzleRestClient.php#L34
uses the following construct:

    public function setBaseUriForRestClient(string $baseUri): void {
        $this->client = new \GuzzleHttp\Client($config);   // $this->client is private
    }

This makes impossible to test an app relying upon restforcephp because we can hardly¹ mock the responses.

  1. You shouldn't use absolute class namespace.
  2. The GuzzleHttp\Client is a dependency that should be passed as an argument
  3. With $this->client being private, we can't overload it.

Could you please improve the situation so that SF depending upon restforce can be tested?

¹ It's actually possible, but using a tons of boilerplate code and workarounds

@jasonraimondi
Copy link
Contributor

jasonraimondi commented Feb 4, 2022

Great suggestions!

You are more than welcome to submit a pull request with these improvements. As long as there are no backwards compatibility issues, I'm sure we could get it merged in a future release without any issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants