From e07a40627ecc5e8ab7779f54f1ab19f5b13f5890 Mon Sep 17 00:00:00 2001 From: Jonathan Danse Date: Sun, 12 Jul 2026 09:17:35 +0200 Subject: [PATCH] feat: Env helper centralising $_ENV + getenv() fallback PHP's variables_order defaults often omit 'E', so process env vars set by the shell (GitHub Actions env: block, KEY=v php ..., docker run -e ...) never reach $_ENV. The lib historically read via $_ENV['PRESTAFLOW_...'], so those callsites silently fell back to defaults in CI unless the consumer wrote a .env file. Env::get($key, $default = null) reads $_ENV first (preserving the dotenv-populated, normalized values TestsSuite writes at load time), then falls back to getenv(), then to the default. Env::has() mirrors that on both sources. Refactor: - src/Utils/Env.php: new helper, 7 unit tests - TestsSuite.php: ~27 read sites converted; the 4 normalization blocks now use Env::has()/get() but keep writing to $_ENV so downstream direct reads see the normalized bool - Version.php: 1 site - Screenshots.php: 1 site - ExecuteSuite.php: 1 site (PRESTAFLOW_TZ) Behaviour preserved everywhere: Env::get returns $_ENV values verbatim when set (including empty string), matching the previous '?? $default' idiom. Only new behaviour is picking up process env vars that weren't in $_ENV. Co-Authored-By: Claude Opus 4.7 --- src/Command/ExecuteSuite.php | 6 +-- src/Tests/TestsSuite.php | 68 ++++++++++++++--------------- src/Traits/Version.php | 4 +- src/Utils/Env.php | 48 +++++++++++++++++++++ src/Utils/Screenshots.php | 4 +- tests/Unit/Utils/EnvTest.php | 83 ++++++++++++++++++++++++++++++++++++ 6 files changed, 171 insertions(+), 42 deletions(-) create mode 100644 src/Utils/Env.php create mode 100644 tests/Unit/Utils/EnvTest.php diff --git a/src/Command/ExecuteSuite.php b/src/Command/ExecuteSuite.php index 9e2bb1d..ebb62f4 100644 --- a/src/Command/ExecuteSuite.php +++ b/src/Command/ExecuteSuite.php @@ -3,6 +3,7 @@ namespace PrestaFlow\Library\Command; use Error; +use PrestaFlow\Library\Utils\Env; use PrestaFlow\Library\Utils\Output; use PrestaFlow\Library\Reports\JUnitReport; use PrestaFlow\Library\Reports\TestRunSummary; @@ -279,11 +280,8 @@ public function execute(InputInterface $input, OutputInterface $output): int if ($visualPath !== null) { // Fuseau du stamp : option CLI > env PRESTAFLOW_TZ > UTC. Fallback UTC // si l'identifiant est invalide (ne casse jamais la génération du rapport). - // On lit à la fois $_ENV et getenv() : selon variables_order de PHP, - // seul l'un ou l'autre peut être peuplé par le shell parent (setup-php CI - // ne peuple pas $_ENV par défaut). $tzName = $input->getOption('visual-report-tz') - ?: ($_ENV['PRESTAFLOW_TZ'] ?? getenv('PRESTAFLOW_TZ') ?: 'UTC'); + ?: Env::get('PRESTAFLOW_TZ', 'UTC'); try { $tz = new \DateTimeZone($tzName); } catch (\Exception $e) { diff --git a/src/Tests/TestsSuite.php b/src/Tests/TestsSuite.php index b20139e..ed431ed 100644 --- a/src/Tests/TestsSuite.php +++ b/src/Tests/TestsSuite.php @@ -17,6 +17,7 @@ use PrestaFlow\Library\Traits\ImportPage; use PrestaFlow\Library\Traits\Locale; use PrestaFlow\Library\Traits\Version; +use PrestaFlow\Library\Utils\Env; use PrestaFlow\Library\Utils\Output; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\ErrorHandler\Error\FatalError; @@ -371,16 +372,11 @@ public static function getBrowser(bool $headless = true, bool $force = true) // Dimensions de la fenêtre : PRESTAFLOW_WINDOW_SIZE_WIDTH/HEIGHT en // env (utile pour émuler mobile/tablet/desktop). Défaut FHD 1920×1080. - // On lit à la fois $_ENV et getenv() : selon variables_order de PHP, - // seul l'un ou l'autre peut être peuplé par le shell parent (setup-php - // CI ne peuple pas $_ENV par défaut). - $envWidth = $_ENV['PRESTAFLOW_WINDOW_SIZE_WIDTH'] ?? getenv('PRESTAFLOW_WINDOW_SIZE_WIDTH'); - $envHeight = $_ENV['PRESTAFLOW_WINDOW_SIZE_HEIGHT'] ?? getenv('PRESTAFLOW_WINDOW_SIZE_HEIGHT'); - $winWidth = (int) ($envWidth ?: 1920); - $winHeight = (int) ($envHeight ?: 1080); + $winWidth = (int) (Env::get('PRESTAFLOW_WINDOW_SIZE_WIDTH') ?: 1920); + $winHeight = (int) (Env::get('PRESTAFLOW_WINDOW_SIZE_HEIGHT') ?: 1080); $options = [ - 'userAgent' => $_ENV['PRESTAFLOW_USER_AGENT'] ?? getenv('PRESTAFLOW_USER_AGENT') ?: 'PrestaFlow', + 'userAgent' => Env::get('PRESTAFLOW_USER_AGENT', 'PrestaFlow'), 'keepAlive' => true, 'windowSize' => [$winWidth, $winHeight], 'headless' => (bool) $headless, @@ -508,8 +504,8 @@ public function before($headless = null, bool $getBrowser = true) */ protected function presetBasicAuth(): void { - $user = $_ENV['PRESTAFLOW_BASIC_USER'] ?? null; - $pass = $_ENV['PRESTAFLOW_BASIC_PASS'] ?? null; + $user = Env::get('PRESTAFLOW_BASIC_USER'); + $pass = Env::get('PRESTAFLOW_BASIC_PASS'); if ($user === null || $user === '') { return; } @@ -583,7 +579,7 @@ public static function applyExtraHttpHeaders(): void */ protected function presetEnvCookies(): void { - $raw = $_ENV['PRESTAFLOW_COOKIES'] ?? null; + $raw = Env::get('PRESTAFLOW_COOKIES'); if (!$raw) { return; } @@ -722,36 +718,36 @@ public function loadGlobals() $dotenv = Dotenv::createImmutable(__DIR__.'/../../../../../', ['.env.local', '.env']); $dotenv->safeLoad(); - if (isset($_ENV['PRESTAFLOW_DEBUG'])) { - $_ENV['PRESTAFLOW_DEBUG'] = filter_var($_ENV['PRESTAFLOW_DEBUG'], FILTER_VALIDATE_BOOLEAN); + if (Env::has('PRESTAFLOW_DEBUG')) { + $_ENV['PRESTAFLOW_DEBUG'] = filter_var(Env::get('PRESTAFLOW_DEBUG'), FILTER_VALIDATE_BOOLEAN); } else { $_ENV['PRESTAFLOW_DEBUG'] = false; } - if (isset($_ENV['PRESTAFLOW_HEADLESS'])) { - $_ENV['PRESTAFLOW_HEADLESS'] = filter_var($_ENV['PRESTAFLOW_HEADLESS'], FILTER_VALIDATE_BOOLEAN); + if (Env::has('PRESTAFLOW_HEADLESS')) { + $_ENV['PRESTAFLOW_HEADLESS'] = filter_var(Env::get('PRESTAFLOW_HEADLESS'), FILTER_VALIDATE_BOOLEAN); } else { $_ENV['PRESTAFLOW_HEADLESS'] = true; } - if (isset($_ENV['PRESTAFLOW_PREFIX_LOCALE'])) { - $_ENV['PRESTAFLOW_PREFIX_LOCALE'] = filter_var($_ENV['PRESTAFLOW_PREFIX_LOCALE'], FILTER_VALIDATE_BOOLEAN); + if (Env::has('PRESTAFLOW_PREFIX_LOCALE')) { + $_ENV['PRESTAFLOW_PREFIX_LOCALE'] = filter_var(Env::get('PRESTAFLOW_PREFIX_LOCALE'), FILTER_VALIDATE_BOOLEAN); } else { $_ENV['PRESTAFLOW_PREFIX_LOCALE'] = false; } - if (isset($_ENV['PRESTAFLOW_VERBOSE'])) { - $_ENV['PRESTAFLOW_VERBOSE'] = filter_var($_ENV['PRESTAFLOW_VERBOSE'], FILTER_VALIDATE_BOOLEAN); + if (Env::has('PRESTAFLOW_VERBOSE')) { + $_ENV['PRESTAFLOW_VERBOSE'] = filter_var(Env::get('PRESTAFLOW_VERBOSE'), FILTER_VALIDATE_BOOLEAN); } else { $_ENV['PRESTAFLOW_VERBOSE'] = true; } - $frontOfficeUrl = $_ENV['PRESTAFLOW_FO_URL'] ?? 'https://localhost/'; + $frontOfficeUrl = Env::get('PRESTAFLOW_FO_URL', 'https://localhost/'); if (!str_ends_with($frontOfficeUrl, '/')) { $frontOfficeUrl .= '/'; } - $backOfficeUrl = $_ENV['PRESTAFLOW_BO_URL'] ?? $frontOfficeUrl . 'admin-dev/'; + $backOfficeUrl = Env::get('PRESTAFLOW_BO_URL', $frontOfficeUrl . 'admin-dev/'); if (!str_starts_with($backOfficeUrl, 'https://') && !str_starts_with($backOfficeUrl, 'http://')) { $backOfficeUrl = $frontOfficeUrl . $backOfficeUrl; } @@ -760,31 +756,31 @@ public function loadGlobals() } $this->globals = [ - 'PS_VERSION' => $_ENV['PRESTAFLOW_PS_VERSION'] ?? '8.1.0', - 'LOCALE' => $_ENV['PRESTAFLOW_LOCALE'] ?? 'en', - 'PREFIX_LOCALE' => (bool) $_ENV['PRESTAFLOW_PREFIX_LOCALE'] ?? false, + 'PS_VERSION' => Env::get('PRESTAFLOW_PS_VERSION', '8.1.0'), + 'LOCALE' => Env::get('PRESTAFLOW_LOCALE', 'en'), + 'PREFIX_LOCALE' => (bool) Env::get('PRESTAFLOW_PREFIX_LOCALE', false), 'BO' => [ 'URL' => $backOfficeUrl, - 'EMAIL' => $_ENV['PRESTAFLOW_BO_EMAIL'] ?? 'demo@prestashop.com', - 'PASSWD' => $_ENV['PRESTAFLOW_BO_PASSWD'] ?? 'Correct Horse Battery Staple', + 'EMAIL' => Env::get('PRESTAFLOW_BO_EMAIL', 'demo@prestashop.com'), + 'PASSWD' => Env::get('PRESTAFLOW_BO_PASSWD', 'Correct Horse Battery Staple'), ], 'FO' => [ 'URL' => $frontOfficeUrl, - 'EMAIL' => $_ENV['PRESTAFLOW_FO_EMAIL'] ?? 'pub@prestashop.com', - 'PASSWD' => $_ENV['PRESTAFLOW_FO_PASSWD'] ?? '123456789', + 'EMAIL' => Env::get('PRESTAFLOW_FO_EMAIL', 'pub@prestashop.com'), + 'PASSWD' => Env::get('PRESTAFLOW_FO_PASSWD', '123456789'), ], - 'DEBUG' => (bool) $_ENV['PRESTAFLOW_DEBUG'] ?? false, - 'VERBOSE' => (bool) $_ENV['PRESTAFLOW_VERBOSE'] ?? true, + 'DEBUG' => (bool) Env::get('PRESTAFLOW_DEBUG', false), + 'VERBOSE' => (bool) Env::get('PRESTAFLOW_VERBOSE', true), 'BROWSER' => [ - 'HEADLESS' => (bool) $_ENV['PRESTAFLOW_HEADLESS'] ?? true, - 'WINDOW_SIZE_HEIGHT' => $_ENV['PRESTAFLOW_WINDOW_SIZE_HEIGHT'] ?? 1920, - 'WINDOW_SIZE_WIDTH' => $_ENV['PRESTAFLOW_WINDOW_SIZE_WIDTH'] ?? 1000, - 'USER_AGENT' => $_ENV['PRESTAFLOW_USER_AGENT'] ?? 'PrestaFlow', + 'HEADLESS' => (bool) Env::get('PRESTAFLOW_HEADLESS', true), + 'WINDOW_SIZE_HEIGHT' => Env::get('PRESTAFLOW_WINDOW_SIZE_HEIGHT', 1920), + 'WINDOW_SIZE_WIDTH' => Env::get('PRESTAFLOW_WINDOW_SIZE_WIDTH', 1000), + 'USER_AGENT' => Env::get('PRESTAFLOW_USER_AGENT', 'PrestaFlow'), ], ]; - $this->exctractVersions($_ENV['PRESTAFLOW_PS_VERSION'] ?? '8.1.0'); - $this->setLocale($_ENV['PRESTAFLOW_LOCALE'] ?? 'en'); + $this->exctractVersions(Env::get('PRESTAFLOW_PS_VERSION', '8.1.0')); + $this->setLocale(Env::get('PRESTAFLOW_LOCALE', 'en')); } public function isVerboseMode(): bool diff --git a/src/Traits/Version.php b/src/Traits/Version.php index a4e67f7..774e1ff 100644 --- a/src/Traits/Version.php +++ b/src/Traits/Version.php @@ -2,6 +2,8 @@ namespace PrestaFlow\Library\Traits; +use PrestaFlow\Library\Utils\Env; + trait Version { const SUPPORTED_VERSIONS = [ @@ -48,7 +50,7 @@ public function resolveVersion(): void $version = $this->psVersionOverride ?? $propertyVersion - ?? ($_ENV['PRESTAFLOW_PS_VERSION'] ?? null) + ?? Env::get('PRESTAFLOW_PS_VERSION') ?? ($this->globals['PS_VERSION'] ?? null) ?? '8.1.0'; diff --git a/src/Utils/Env.php b/src/Utils/Env.php new file mode 100644 index 0000000..96d3c9a --- /dev/null +++ b/src/Utils/Env.php @@ -0,0 +1,48 @@ +envBackup = []; + foreach (['PF_TEST_A', 'PF_TEST_B', 'PF_TEST_EMPTY', 'PF_TEST_MISSING'] as $k) { + $this->envBackup[$k] = [ + 'env' => array_key_exists($k, $_ENV) ? $_ENV[$k] : null, + 'envHas' => array_key_exists($k, $_ENV), + 'getenv' => getenv($k), + ]; + unset($_ENV[$k]); + putenv($k); + } + } + + protected function tearDown(): void + { + foreach ($this->envBackup as $k => $snap) { + unset($_ENV[$k]); + putenv($k); + if ($snap['envHas']) { + $_ENV[$k] = $snap['env']; + } + if ($snap['getenv'] !== false) { + putenv("$k={$snap['getenv']}"); + } + } + } + + public function testGetReturnsDefaultWhenAbsentEverywhere(): void + { + $this->assertNull(Env::get('PF_TEST_MISSING')); + $this->assertSame('fallback', Env::get('PF_TEST_MISSING', 'fallback')); + } + + public function testGetReadsFromEnvSuperglobalFirst(): void + { + $_ENV['PF_TEST_A'] = 'from-env-superglobal'; + putenv('PF_TEST_A=from-process'); + $this->assertSame('from-env-superglobal', Env::get('PF_TEST_A')); + } + + public function testGetFallsBackToGetenvWhenSuperglobalAbsent(): void + { + putenv('PF_TEST_B=from-process-only'); + $this->assertSame('from-process-only', Env::get('PF_TEST_B')); + } + + public function testGetReturnsEmptyStringFromSuperglobalVerbatim(): void + { + // Preserves `?? $default` semantics: a key set to '' still wins over the default. + $_ENV['PF_TEST_EMPTY'] = ''; + $this->assertSame('', Env::get('PF_TEST_EMPTY', 'default')); + } + + public function testHasTrueWhenInSuperglobal(): void + { + $_ENV['PF_TEST_A'] = 'x'; + $this->assertTrue(Env::has('PF_TEST_A')); + } + + public function testHasTrueWhenInProcessOnly(): void + { + putenv('PF_TEST_B=x'); + $this->assertTrue(Env::has('PF_TEST_B')); + } + + public function testHasFalseWhenAbsentEverywhere(): void + { + $this->assertFalse(Env::has('PF_TEST_MISSING')); + } +}