From a38dc3fdb2fa47c2ddb3744e0252eb2e9d1a5fd9 Mon Sep 17 00:00:00 2001 From: Holger Schmermbeck Date: Sat, 1 Nov 2025 20:15:48 +0100 Subject: [PATCH 1/4] fix: auto-detect DDEV for preflight checks - Add DDEV detection in preflight.sh to run PHP checks in container - Run Pint, PHPStan, and tests via 'ddev exec' when DDEV is available - Fallback to host execution with warning when DDEV not detected - Fixes pre-push hook failures due to missing database connection - Ensures consistent environment between local dev and CI Resolves pre-commit outside DDEV issue from PR #55 --- scripts/preflight.sh | 65 +++++++++++++++++++++++++++++++++----------- 1 file changed, 49 insertions(+), 16 deletions(-) diff --git a/scripts/preflight.sh b/scripts/preflight.sh index 2914358..b6a43ed 100755 --- a/scripts/preflight.sh +++ b/scripts/preflight.sh @@ -48,26 +48,59 @@ if [ -f composer.json ]; then if ! command -v composer >/dev/null 2>&1; then echo "Warning: composer.json found but composer not installed - skipping PHP checks" >&2 else - composer install --no-interaction --no-progress --prefer-dist --optimize-autoloader - # Run Laravel Pint code style check if available (blocking: aligns with gates) - if [ -x ./vendor/bin/pint ]; then - ./vendor/bin/pint --test + # Auto-detect DDEV for consistent environment + USE_DDEV=false + if command -v ddev >/dev/null 2>&1 && ddev describe >/dev/null 2>&1; then + USE_DDEV=true + echo "✓ DDEV detected - using containerized environment for PHP checks" fi - # Run PHPStan (use configured level from phpstan.neon if exists, else max) - if [ -x ./vendor/bin/phpstan ]; then - if [ -f phpstan.neon ] || [ -f phpstan.neon.dist ]; then - php -d memory_limit=512M ./vendor/bin/phpstan analyse - else - php -d memory_limit=512M ./vendor/bin/phpstan analyse --level=max + + if [ "$USE_DDEV" = true ]; then + # Run Laravel Pint code style check if available (blocking: aligns with gates) + if [ -x ./vendor/bin/pint ]; then + ddev exec ./vendor/bin/pint --test + fi + # Run PHPStan (use configured level from phpstan.neon if exists, else max) + if [ -x ./vendor/bin/phpstan ]; then + if [ -f phpstan.neon ] || [ -f phpstan.neon.dist ]; then + ddev exec php -d memory_limit=512M ./vendor/bin/phpstan analyse + else + ddev exec php -d memory_limit=512M ./vendor/bin/phpstan analyse --level=max + fi + fi + else + composer install --no-interaction --no-progress --prefer-dist --optimize-autoloader + # Run Laravel Pint code style check if available (blocking: aligns with gates) + if [ -x ./vendor/bin/pint ]; then + ./vendor/bin/pint --test + fi + # Run PHPStan (use configured level from phpstan.neon if exists, else max) + if [ -x ./vendor/bin/phpstan ]; then + if [ -f phpstan.neon ] || [ -f phpstan.neon.dist ]; then + php -d memory_limit=512M ./vendor/bin/phpstan analyse + else + php -d memory_limit=512M ./vendor/bin/phpstan analyse --level=max + fi fi fi # Run tests (Laravel Artisan → Pest → PHPUnit) - if [ -f artisan ]; then - php artisan test --parallel - elif [ -x ./vendor/bin/pest ]; then - ./vendor/bin/pest --parallel - elif [ -x ./vendor/bin/phpunit ]; then - ./vendor/bin/phpunit + if [ "$USE_DDEV" = true ]; then + if [ -f artisan ]; then + ddev exec php artisan test --parallel + elif [ -x ./vendor/bin/pest ]; then + ddev exec ./vendor/bin/pest --parallel + elif [ -x ./vendor/bin/phpunit ]; then + ddev exec ./vendor/bin/phpunit + fi + else + echo "⚠️ DDEV not detected - running tests on host (may fail with database errors)" + if [ -f artisan ]; then + php artisan test --parallel + elif [ -x ./vendor/bin/pest ]; then + ./vendor/bin/pest --parallel + elif [ -x ./vendor/bin/phpunit ]; then + ./vendor/bin/phpunit + fi fi fi fi From d38b050bbf596c6c629c336c405d7d427258a992 Mon Sep 17 00:00:00 2001 From: Holger Schmermbeck Date: Sat, 1 Nov 2025 20:19:44 +0100 Subject: [PATCH 2/4] refactor: address Copilot review feedback - Add comment explaining why composer install is skipped in DDEV path (vendor/ is bind-mounted, dependencies managed by DDEV setup) - Move warning message to only display after test failure - Add helpful tip about using DDEV for database-dependent tests - Capture and propagate test exit codes properly - Addresses both Copilot review comments from PR #57 --- scripts/preflight.sh | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/scripts/preflight.sh b/scripts/preflight.sh index b6a43ed..93432e5 100755 --- a/scripts/preflight.sh +++ b/scripts/preflight.sh @@ -56,6 +56,8 @@ if [ -f composer.json ]; then fi if [ "$USE_DDEV" = true ]; then + # Dependencies are managed within DDEV container (vendor/ is bind-mounted from host) + # No need to run composer install here - DDEV setup already handles it # Run Laravel Pint code style check if available (blocking: aligns with gates) if [ -x ./vendor/bin/pint ]; then ddev exec ./vendor/bin/pint --test @@ -84,23 +86,32 @@ if [ -f composer.json ]; then fi fi # Run tests (Laravel Artisan → Pest → PHPUnit) + TEST_EXIT=0 if [ "$USE_DDEV" = true ]; then if [ -f artisan ]; then - ddev exec php artisan test --parallel + ddev exec php artisan test --parallel || TEST_EXIT=$? elif [ -x ./vendor/bin/pest ]; then - ddev exec ./vendor/bin/pest --parallel + ddev exec ./vendor/bin/pest --parallel || TEST_EXIT=$? elif [ -x ./vendor/bin/phpunit ]; then - ddev exec ./vendor/bin/phpunit + ddev exec ./vendor/bin/phpunit || TEST_EXIT=$? fi else - echo "⚠️ DDEV not detected - running tests on host (may fail with database errors)" if [ -f artisan ]; then - php artisan test --parallel + php artisan test --parallel || TEST_EXIT=$? elif [ -x ./vendor/bin/pest ]; then - ./vendor/bin/pest --parallel + ./vendor/bin/pest --parallel || TEST_EXIT=$? elif [ -x ./vendor/bin/phpunit ]; then - ./vendor/bin/phpunit + ./vendor/bin/phpunit || TEST_EXIT=$? fi + # Show warning only after test failure when DDEV not available + if [ "$TEST_EXIT" -ne 0 ]; then + echo "⚠️ Tests failed without DDEV - database connection may be unavailable" >&2 + echo "Tip: Use DDEV for tests requiring PostgreSQL: ddev exec ./vendor/bin/pest" >&2 + fi + fi + # Propagate test exit code + if [ "$TEST_EXIT" -ne 0 ]; then + exit "$TEST_EXIT" fi fi fi From 43c24f90ab6c4d90314a02378448a923bde08b09 Mon Sep 17 00:00:00 2001 From: Holger Schmermbeck Date: Sat, 1 Nov 2025 20:27:46 +0100 Subject: [PATCH 3/4] refactor: eliminate code duplication in preflight.sh - Extract run_pint(), run_phpstan(), run_tests() helper functions - Accept command prefix parameter (empty or 'ddev exec') - Reduce 70+ lines of duplicated code to reusable functions - Keep --test flag for Pint (validation-only for pre-push hooks) - Clarify comment: Pre-push hooks should validate, not auto-fix code Addresses Copilot review comments: - ID 2483869455: Extract Pint/PHPStan logic into functions - ID 2483869457: Extract test execution logic into functions - ID 2483869460, 2483869463: Clarify Pint --test usage --- scripts/preflight.sh | 102 +++++++++++++++++++++---------------------- 1 file changed, 51 insertions(+), 51 deletions(-) diff --git a/scripts/preflight.sh b/scripts/preflight.sh index 93432e5..e0c57b0 100755 --- a/scripts/preflight.sh +++ b/scripts/preflight.sh @@ -43,72 +43,72 @@ if [ "$FORMAT_EXIT" -ne 0 ]; then exit 1 fi +# Helper functions to reduce code duplication +run_pint() { + local cmd_prefix="$1" + # Run Laravel Pint validation (--test = check only, do not auto-fix) + # Pre-push hooks should validate, not modify code + if [ -x ./vendor/bin/pint ]; then + ${cmd_prefix} ./vendor/bin/pint --test + fi +} + +run_phpstan() { + local cmd_prefix="$1" + # Run PHPStan static analysis + if [ -x ./vendor/bin/phpstan ]; then + if [ -f phpstan.neon ] || [ -f phpstan.neon.dist ]; then + ${cmd_prefix} php -d memory_limit=512M ./vendor/bin/phpstan analyse + else + ${cmd_prefix} php -d memory_limit=512M ./vendor/bin/phpstan analyse --level=max + fi + fi +} + +run_tests() { + local cmd_prefix="$1" + local test_exit=0 + # Run tests (Laravel Artisan → Pest → PHPUnit) + if [ -f artisan ]; then + ${cmd_prefix} php artisan test --parallel || test_exit=$? + elif [ -x ./vendor/bin/pest ]; then + ${cmd_prefix} ./vendor/bin/pest --parallel || test_exit=$? + elif [ -x ./vendor/bin/phpunit ]; then + ${cmd_prefix} ./vendor/bin/phpunit || test_exit=$? + fi + return $test_exit +} + # 1) PHP / Laravel if [ -f composer.json ]; then if ! command -v composer >/dev/null 2>&1; then echo "Warning: composer.json found but composer not installed - skipping PHP checks" >&2 else # Auto-detect DDEV for consistent environment - USE_DDEV=false + CMD_PREFIX="" if command -v ddev >/dev/null 2>&1 && ddev describe >/dev/null 2>&1; then - USE_DDEV=true + CMD_PREFIX="ddev exec" echo "✓ DDEV detected - using containerized environment for PHP checks" - fi - - if [ "$USE_DDEV" = true ]; then # Dependencies are managed within DDEV container (vendor/ is bind-mounted from host) # No need to run composer install here - DDEV setup already handles it - # Run Laravel Pint code style check if available (blocking: aligns with gates) - if [ -x ./vendor/bin/pint ]; then - ddev exec ./vendor/bin/pint --test - fi - # Run PHPStan (use configured level from phpstan.neon if exists, else max) - if [ -x ./vendor/bin/phpstan ]; then - if [ -f phpstan.neon ] || [ -f phpstan.neon.dist ]; then - ddev exec php -d memory_limit=512M ./vendor/bin/phpstan analyse - else - ddev exec php -d memory_limit=512M ./vendor/bin/phpstan analyse --level=max - fi - fi else composer install --no-interaction --no-progress --prefer-dist --optimize-autoloader - # Run Laravel Pint code style check if available (blocking: aligns with gates) - if [ -x ./vendor/bin/pint ]; then - ./vendor/bin/pint --test - fi - # Run PHPStan (use configured level from phpstan.neon if exists, else max) - if [ -x ./vendor/bin/phpstan ]; then - if [ -f phpstan.neon ] || [ -f phpstan.neon.dist ]; then - php -d memory_limit=512M ./vendor/bin/phpstan analyse - else - php -d memory_limit=512M ./vendor/bin/phpstan analyse --level=max - fi - fi fi - # Run tests (Laravel Artisan → Pest → PHPUnit) + + # Run quality checks + run_pint "$CMD_PREFIX" + run_phpstan "$CMD_PREFIX" + + # Run tests and handle failures TEST_EXIT=0 - if [ "$USE_DDEV" = true ]; then - if [ -f artisan ]; then - ddev exec php artisan test --parallel || TEST_EXIT=$? - elif [ -x ./vendor/bin/pest ]; then - ddev exec ./vendor/bin/pest --parallel || TEST_EXIT=$? - elif [ -x ./vendor/bin/phpunit ]; then - ddev exec ./vendor/bin/phpunit || TEST_EXIT=$? - fi - else - if [ -f artisan ]; then - php artisan test --parallel || TEST_EXIT=$? - elif [ -x ./vendor/bin/pest ]; then - ./vendor/bin/pest --parallel || TEST_EXIT=$? - elif [ -x ./vendor/bin/phpunit ]; then - ./vendor/bin/phpunit || TEST_EXIT=$? - fi - # Show warning only after test failure when DDEV not available - if [ "$TEST_EXIT" -ne 0 ]; then - echo "⚠️ Tests failed without DDEV - database connection may be unavailable" >&2 - echo "Tip: Use DDEV for tests requiring PostgreSQL: ddev exec ./vendor/bin/pest" >&2 - fi + run_tests "$CMD_PREFIX" || TEST_EXIT=$? + + # Show helpful message only after test failure when DDEV not available + if [ "$TEST_EXIT" -ne 0 ] && [ -z "$CMD_PREFIX" ]; then + echo "⚠️ Tests failed without DDEV - database connection may be unavailable" >&2 + echo "Tip: Use DDEV for tests requiring PostgreSQL: ddev exec ./vendor/bin/pest" >&2 fi + # Propagate test exit code if [ "$TEST_EXIT" -ne 0 ]; then exit "$TEST_EXIT" From 98bf4a92fe9b902c2fe333edf5943b13f883d139 Mon Sep 17 00:00:00 2001 From: Holger Schmermbeck Date: Sat, 1 Nov 2025 20:32:06 +0100 Subject: [PATCH 4/4] fix: address all Copilot review feedback - Change Pint to use --dirty flag instead of --test for auto-fixing - Update comment: Pint now auto-fixes code style (aligned with project) - Fix tip message to use 'php artisan test' instead of pest Addresses final Copilot comments: - ID 2483869460, 2483869463, 2483880462: Remove --test, use --dirty - ID 2483880470: Suggest artisan test (Laravel convention) All 8 review comments now addressed. --- scripts/preflight.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/preflight.sh b/scripts/preflight.sh index e0c57b0..a2c428e 100755 --- a/scripts/preflight.sh +++ b/scripts/preflight.sh @@ -46,10 +46,10 @@ fi # Helper functions to reduce code duplication run_pint() { local cmd_prefix="$1" - # Run Laravel Pint validation (--test = check only, do not auto-fix) - # Pre-push hooks should validate, not modify code + # Run Laravel Pint to auto-fix code style issues + # Uses --dirty flag to only process modified files for performance if [ -x ./vendor/bin/pint ]; then - ${cmd_prefix} ./vendor/bin/pint --test + ${cmd_prefix} ./vendor/bin/pint --dirty fi } @@ -106,7 +106,7 @@ if [ -f composer.json ]; then # Show helpful message only after test failure when DDEV not available if [ "$TEST_EXIT" -ne 0 ] && [ -z "$CMD_PREFIX" ]; then echo "⚠️ Tests failed without DDEV - database connection may be unavailable" >&2 - echo "Tip: Use DDEV for tests requiring PostgreSQL: ddev exec ./vendor/bin/pest" >&2 + echo "Tip: Use DDEV for tests requiring PostgreSQL: ddev exec php artisan test" >&2 fi # Propagate test exit code