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

Feature | V2 Fixing Tests #108

Merged
merged 16 commits into from
Nov 17, 2022
Merged

Feature | V2 Fixing Tests #108

merged 16 commits into from
Nov 17, 2022

Conversation

Sammyjo20
Copy link
Collaborator

No description provided.

@what-the-diff
Copy link

what-the-diff bot commented Nov 17, 2022

  • Update guzzlehttp/guzzle to 7.5
  • Remove CarbonInterface from OAuthAuthenticator contract
  • Add Throwable type hint for Response::toException() and RequestException constructor parameters
  • Change FixtureData::fromGuzzleResponse parameter type hint from ResponseInterface to AbstractResponse (and update docblock)
  • Create MergeOptions class, which is used in the new mergeRequestProperties method on Connector base class
  • Renamed SendRequest to Dispatcher
  • Changed the return type of execute() from Response|PromiseInterface to ResponseContract|PromiseInterface
  • Added a new method getException() on SimulatedResponsePayload which returns an exception if one is set, otherwise null
  • Removed MockClient::findResponseByRequest(string $request) and replaced it with findResponseByRequestUrl(string $url) as this was more useful in practice (and easier for me!)
  • Updated all references of "SimulatedAbstractResponses" to just be called "responses". This makes sense because we can now use custom responses that implement PSR-7's response interface so they are no longer abstract! :)
  • Removed the PsrResponse class.
  • Added a send() method to the Request interface which returns an AbstractResponse object, and also added a sendAsync() method that returns PromiseInterface (which is what Guzzle uses).
  • Changed all of our tests from using $request->send($mockClient) to just $request->send(). This means we can now use MockClients in our unit tests without having to pass them into every single request call! We have removed this functionality for asynchronous requests as it's not possible with how Laravel works currently - see [Windows] Error on using SMTP laravel/framework#27098 (comment) for more information on why this isn't possible at present time :(
  • Removed the SimulatedResponse class.
  • Moved all response helpers to a trait called HasResponseHelpers.
  • Added an optional parameter for the PSR ResponseInterface in constructor of Response class, so we can pass either a real or simulated response object there and it will work fine with both cases (real responses from GuzzleHttp client and mocked/simulated responses).
  • Changed some method names like: body() -> stream(), headers() -> getHeaders(). This is because now that we are using PSR-7 interfaces, these methods should be named as per their interface definitions instead of having custom names which might confuse people who have worked on Laravel before but not Saloon yet :) . Also added type hints wherever possible to make things more clear about what's going on under the hood!
  • Changed the default response class to Saloon\Http\Responses\Response
  • Added a new trait Conditionable which allows you to conditionally execute code based on values in your connector or request.
  • Fixed an issue where when using GuzzleSender, if there was no body set it would throw an exception because of how we were trying to match against the type hint for $body::class (which is null). We now use instanceof instead and check each possible value that could be returned from getBody(). This also fixes another bug where if you had a StringBodyRepository but didn't have any data in it, then this would cause issues as well since we'd try and cast [] into a string rather than just returning '' like before.
  • Updated HasCustomResponses so that by default all responses will return Response objects unless otherwise specified with ->response()->setDefault(...); The old way still works too though! :) You can specify either 'Saloon\Http\Responses\PsrResponse' or 'psr'. If nothing is specified at all then PsrResponse will be used automatically anyway - however I think its better practice not having anything hardcoded here so people don't forget about setting their own custom response classes etc.. Also updated some documentation around this area too while I was at it! :D
  • Added a new trait HasResponseHelpers to the Response class.
  • Added a new trait HasSimulationMethods to the Response class.
  • Updated AsyncRequestTest, ConnectorRequestTest and MockRequestTest test cases for changes in response object structure due to addition of traits mentioned above (1 & 2).
  • Removed CarbonImmutable from AuthCodeFlowConnectorTest.php
  • Added PendingRequestTest to test merging of headers in a request and disabling the merge option for connector headers
  • Fixed Pool Test by changing ResponseInterface to ResponseContract as it was causing issues with mocked responses not being returned correctly when using promises (this is because we are now returning an instance of MockResponse instead)
  • Changed RequestTest so that it uses Saloon\Http\Responses\Response rather than Saloon\Http\ResponsesPsrResponse, this is due to us no longer having PSR-7 response objects but our own implementation which extends AbstractMessage and implements MessageInterface - This means we can add additional methods such as json() etc... without breaking any contracts or interfaces! We also added some tests around mocking requests too :)
  • Renamed HasJsonBody to HasBody
  • Added WithBody interface and implemented it in PostJsonRequest class
  • Changed defaultData() method name to defaultBody() in PostJsonRequest class
  • Added a new protected function called "defaultHeaders" which returns an array of headers for UserRequest Class
  • Removed the use statement from CustomResponse as we don't need it anymore since we are using AbstractResponses now instead of Responses
  • Changed the order of parameters in MockResponse::make()
  • Added createPendingRequest method to Request class and used it instead of new PendingRequest(...)
  • Fixed a bug where assertSentJson would fail if there were multiple requests sent before asserting that one was sent with json data matching an array or object (assertSentJson now works properly)
  • Changed the order of parameters in MockResponse::make()
  • Added a new parameter to MockResponse::fromRequest()
  • Removed unused code from tests/Unit/MockResponseTest.php:23-34, 59 and added missing use statement for StringBodyRepository at line 5
  • Fixed typo on tests/Unit/Oauth2AccessTokenAuthenticatorTest:14 (CarbonImmutable -> Date)
  • Updated Carbon\Carbon dependency version to ^1|^2|^3 as it is not compatible with PHP 8 yet https://github.com/briannesbitt/carbon#upgrading-to-v300x---laravel--symfony--illuminateframework----support
  • Added a new test file for the GuzzleHandler
  • Removed all tests from RequestManagerTest and moved them to their respective files (Request, SaloonResponse)
  • Fixed some typos in comments of existing code

Copy link
Collaborator Author

@Sammyjo20 Sammyjo20 left a comment

Choose a reason for hiding this comment

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

Self review

@Sammyjo20 Sammyjo20 marked this pull request as ready for review November 17, 2022 23:48
@Sammyjo20 Sammyjo20 merged commit 510d73b into v2 Nov 17, 2022
@Sammyjo20 Sammyjo20 deleted the feature/v2-fix-tests branch November 17, 2022 23:48
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

Successfully merging this pull request may close these issues.

None yet

1 participant