How I tried to improve Laravel and only made it worse

Introduction

Laravel is a cool PHP framework, we use it all the time at the company. But as you know, nothing in the world is perfect, you can always suggest improvements.

A few weeks ago I tried to make one small improvement on the Laravel test area, opened two pull requests ( # 1 and # 2 ). Both pull requests were rejected by the author of the framework, Taylor, but in the end, on the same day, he published his own implementation of the same functionality, which he even boasted about on Twitter. And, oh gods, the realization is terrible!

Context

We prefer integration testing on our own, as it allows you to achieve a good balance between the cost of automated testing and confidence in your deployments. Often, in our integration tests, we do something on behalf of the user, and then we want to make sure that the user eventually received some kind of letter. To do this, Laravel implements the standard way:

<?php
public function test_orders_can_be_shipped()
{
    Mail::fake();

    Mail::assertSent(OrderShipped::class);
}
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

, , , - , ? , " - ", " - ".

Laravel , :

<?php
Mail::assertSent(OrderShipped::class, function ($mail) use ($user) {
    return $mail->hasTo($user->email) &&
           $mail->somePublicProperty == 'someValue';
});
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

, - To/Cc/Bcc (, , ). Mailable Laravel , . render() Mailable, :

<?php
public function render()
{
    return $this->withLocale($this->locale, function () {
        Container::getInstance()->call([$this, 'build']);
        return Container::getInstance()->make('mailer')->render(
            $this->buildView(), $this->buildViewData()
        );
    });
}
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

render(), . , buildView() buildViewData() - .

, :

• Mailable, . . .

  
  
  
  
  

• Mailable Macroable, . , Mailable , "with", Macroable . .

  
  
  
  
  

• - -, proxy, Mailable, , , , - seeInHtml(), seeInText(). .

  
  
  
  
  

( -) Mockery, Mailable , :

<?php
public function email_confirmation_is_correct()
{
    Mail::fake();

    event(new TestEvent());
    config(['app.name' => 'Test App']);

    Mail::assertSent(TestMail::class, function (TestMail $mail) {
        return $mail->hasTo('test@test.com') 
          && $mail->seeInHtml('shipped')
          && $mail->dontSeeInHtml('failed') 
          && $mail->seeInText('Test App');
        });
    }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

, API - Laravel, - , ( MailFake, TestMailable). production- .

, , - - :

, , - , , , , - ! API , "assert".

– , , .

– , , , . , – .

production- Mailable, , . , , .

, , Mailable:

<?php
/**
 * @param  string  $string
 * @return void
 */
public function assertSeeInText($string)
{
    [$html, $text] = $this->renderForAssertions();

    PHPUnit::assertTrue(
        Str::contains($text, $string),
        "Did not see expected text [{$string}] within text email body."
    );
}
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

:

• Laravel, production-, . , - . , - MailFake, - Mailable

  
  
  
  
  

• Mailable PHPUnit, development-only, composer.json. , production-

  
  
  
  
  

:

[$html, $text] = $this->renderForAssertions();
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

, , . , , , .

Our goal was to test the combat code - render () and send () methods. We tested a newly created new method that no one will use in practice.

Let's take a look at this method:

<?php
protected function renderForAssertions()
{
    if ($this->assertionableRenderStrings) {
        return $this->assertionableRenderStrings;
    }

    return $this->assertionableRenderStrings = $this->withLocale($this->locale, function () {
        Container::getInstance()->call([$this, 'build']);

        $html = Container::getInstance()->make('mailer')->render(
            $view = $this->buildView(), $this->buildViewData()
        );

        $text = $view['text'] ?? '';

        if (! empty($text) && ! $text instanceof Htmlable) {
            $text = Container::getInstance()->make('mailer')->render(
                $view['text'], $this->buildViewData()
            );
        }

        return [(string) $html, (string) $text];
    });
}
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

Oh, not a method, but a monster!

Not only is it very difficult to read, it also repeats a lot of lines from the render () method, so now there is a chance that the methods will diverge someday. And every time you edit render (), now you need to remember to fix renderForAssertions ().

In my opinion, it is fundamentally wrong to test methods specially designed for tests, instead of real code. This is some kind of nonsense.

I find that with this commit my favorite framework has gotten a bit worse :(