markhuot / craft-pest

https://craft-pest.com
Other
38 stars 11 forks source link

Add support for fluent-based configs #70

Closed aaronbushnell closed 1 year ago

aaronbushnell commented 1 year ago

If your general.php file uses the fluent-style for your config you'll get an error when running your tests:

  • Tests\ExampleTest > get '/' → assertOk 
   PHPUnit\Framework\ExceptionWrapper 

  array_merge(): Argument #1 must be of type array, craft\config\GeneralConfig given

  at vendor/markhuot/craft-pest/src/services/Config.php:19
     15▕         if ($filename === 'app.web') {
     16▕             $overrides = require __DIR__ . '/../config/app.web.php';
     17▕         }
     18▕ 
  ➜  19▕         return array_merge($original, $overrides);
     20▕     }
     21▕ }
     22▕ 

Example config (using fluent syntax):

<?php

use craft\config\GeneralConfig;
use craft\helpers\App;

return GeneralConfig::create()
    ->cpTrigger(App::env('CP_TRIGGER'))
    ->securityKey(App::env('SECURITY_KEY'))
    ->devMode(App::parseBooleanEnv('$DEV_MODE'));

Changing this to the standard array-based config will allow it to work:

PASS  Tests\ExampleTest
  ✓ get '/' → assertOk 

  Tests:  1 passed
  Time:   0.49s

Example config (using arrays):

<?php

use craft\config\GeneralConfig;
use craft\helpers\App;

return [
    'cpTrigger' => App::env('CP_TRIGGER'),
    'securityKey' => App::env('SECURITY_KEY'),
    'devMode' => App::parseBooleanEnv('$DEV_MODE')
];
aaronbushnell commented 1 year ago

Thanks again for your help on that PR, @markhuot!

aaronbushnell commented 1 year ago

Shoot, I should've caught this, @markhuot, but because the :array return type got added in I get the original error again:

❯ php craft pest/test

   FAIL  Tests\ExampleTest
  ⨯ it loads the homepage
  ⨯ it allows an admin to sign in

  ---

  • Tests\ExampleTest > it loads the homepage
   PHPUnit\Framework\ExceptionWrapper 

  craft\services\Config::getConfigFromFile(): Return value must be of type array, craft\config\GeneralConfig returned

  at vendor/markhuot/craft-pest/src/services/Config.php:17
     13▕     {
     14▕         $original = parent::getConfigFromFile($filename);
     15▕ 
     16▕         if (!is_array($original) || $filename !== 'app.web') {
  ➜  17▕             return $original;
     18▕         }
     19▕ 
     20▕         return array_merge($original, require __DIR__ . '/../config/app.web.php');
     21▕     }

Couple ways around this:

  1. (PHP 8 only) Use a second return type of :array|BaseConfig. But the downside, like you mentioned earlier, is an older version of Craft on PHP 7 won't have this option.
  2. (For simpler backward compatibility) Modify the conditional return to coerce the BaseConfig back to an array using a collection:
<?php

namespace craft\services;

class Config extends \markhuot\craftpest\overrides\Config
{
    public function getConfigFromFile(string $filename): array
    {
        $original = parent::getConfigFromFile($filename);

        if (!is_array($original) || $filename !== 'app.web') {
            return collect($original)->toArray();
        }

        return array_merge($original, require __DIR__ . '/../config/app.web.php');
    }
}

Happy to deliver that change in a new PR if that's helpful!