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

Added FakeFacade #199

Open
wants to merge 1 commit into
base: 5.x
Choose a base branch
from

Conversation

roblesterjr04
Copy link

This PR includes a fake facade so devs can write tests in their system and confirm that the messages are being triggered. It works by calling Twilio::fake() at the head of the test. They can then use Twilio::assertMessageSent and Twilio::assertCallSent

@aloha aloha requested review from hannesvdvreken and aloha May 8, 2024 15:04
public static function fake($jobsToFake = [])
{
static::swap($fake = new FakeFacade(static::getFacadeRoot()));
return $fake;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
return $fake;
return $fake;

*/
protected $twilio;

protected $history;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing docbloc


protected $history;

protected $settings = [
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing docblock

use Aloha\Twilio\TwilioInterface;
use Aloha\Twilio\Manager as Twilio;
use Illuminate\Support\Arr;
use PHPUnit\Framework\Assert;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will make phpunit a hard requirement (as opposed to dev-requirement) for this package. Not a good idea. Can we turn it around? for example:

getMessageSent(): Message
getCallSent(): Call

and let the test-suite which is testing it do the assertions.

@ainesophaur
Copy link

Is there going to be any traction on this PR? I would be willing to write and submit a new PR to achieve this functionality as it's extremely useful.

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.

3 participants