ShipEngine / shipengine-php

The official PHP SDK for ShipEngine API
https://www.shipengine.com/docs/
Apache License 2.0
15 stars 11 forks source link

DX-1550 event implementation tests #12

Closed nate-williams-auctane closed 3 years ago

nate-williams-auctane commented 3 years ago

Background

This pull request adds a test for ensuring the RequestSentEvent is fired when a request is sent by the ShipEngine client. It should be similar to the js SDK tests, at: https://github.com/ShipEngine/shipengine-js/blob/6363c722569436a69c4ffa7fe5f1c51e17c12e8d/test/specs/events.spec.js#L9-L37.

There were a number of complications with testing the event using PHPUnit, I am open to documenting more of these in the code itself, subject to what is appropriate for the SDKs. Of note:

  1. final classes in PHPUnit cannot be directly tested, but here this is addressed by using the Mockery library, which also allows us to create a spy.
  2. There are a number of ways to test the arguments of a called function, here we want to test the event being passed in as an argument to the listener, see: on vs with vs withArgs at http://docs.mockery.io/en/latest/reference/argument_validation.html#complex-argument-validation
  3. When using a closure to test arguments coming into a tested function, one is a bit limited, and cannot issue assertions within the closure. The main contributor for PHPUnit has indicated "Do not call assertions in closures then, sorry." (See: https://github.com/sebastianbergmann/phpunit/issues/4371#issuecomment-751595971). Here we use a rather ugly side effect/reference capture to get the $event_result, for this spy and this is based on similar code for Mockery mocks, which use the same approach for its capture method. See vendor/mockery/mockery/library/Mockery.php line 434.

Testing

  1. You can run this test in isolation with ./vendor/bin/phpunit --process-isolation --filter RequestSentEventTest
  2. If one is so inclined, you can verify you get a nice failure by adding a failing assertion on line 34 of tests/Message/Events/RequestSentEventTest.php, like $this->assertEquals($event_result->retry, 1);.
  3. Ensure all tests pass by running composer phpunit.
  4. Ensure the assertions indicated in the task are all represented.
nate-williams-auctane commented 3 years ago

@KaseyCantu Thanks for pointing out the pre-commit config and pre-commit install and act tools. Once I ran those locally, the whitespace/braces were formatted and I eventually got the risky test error to appear and corrected that. Note that I switch the covers annotations to uses for some classes, let me know if that is not best practice here.