test: setup a basic integration in the CI#1018
test: setup a basic integration in the CI#1018JohnVillalovos wants to merge 1 commit intodevelopfrom
Conversation
72e2265 to
21ae493
Compare
There was a problem hiding this comment.
Pull request overview
Adds an integration smoke-test harness that boots the app via PHP’s built-in web server and runs HTTP-level checks in CI.
Changes:
- Introduces a PHP built-in server router script and a config generator for integration testing.
- Adds a PHPUnit
integrationtestsuite with a Guzzle-basedSmokeTest. - Adds a GitHub Actions workflow to provision MySQL, load schema/data, and run the integration suite.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Integration/router.php | Implements rewrite-like routing for php -S to mimic .htaccess behavior during tests. |
| tests/Integration/create-test-config.php | Generates config/config.php from config.dist.php using env-driven DB settings for CI. |
| tests/Integration/SmokeTest.php | Starts a real web server, logs in via HTTP, and smoke-tests authenticated pages. |
| phpunit.xml.dist | Excludes integration tests from the default suite and adds a dedicated integration suite. |
| composer.json | Adds Guzzle as a dev dependency and adds a test:integration script. |
| composer.lock | Updates lockfile metadata to match the new dependency set. |
| .github/workflows/integration-tests.yml | Adds CI job that provisions DB, seeds data, generates config, and runs smoke tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Start PHP built-in server in the background | ||
| $command = sprintf( | ||
| 'php -S 127.0.0.1:%d -t %s %s > /dev/null 2>&1 & echo $!', | ||
| self::$port, | ||
| escapeshellarg($rootDir), | ||
| escapeshellarg($routerScript) | ||
| ); | ||
| $pid = trim((string) shell_exec($command)); | ||
| self::$serverPid = (int) $pid; | ||
|
|
There was a problem hiding this comment.
The server startup relies on shell_exec() and assumes it returns a PID (& echo $!). If shell_exec is disabled or PID parsing fails, self::$serverPid becomes 0 and tearDownAfterClass() won’t stop the server even if it started. Add checks for is_callable('shell_exec') and that the PID is a positive integer; otherwise fail/skip with a clear message.
| public static function tearDownAfterClass(): void | ||
| { | ||
| if (self::$serverPid !== null && self::$serverPid > 0) { | ||
| posix_kill(self::$serverPid, SIGTERM); |
There was a problem hiding this comment.
posix_kill(self::$serverPid, SIGTERM) will fatal if the posix extension isn’t available and/or SIGTERM isn’t defined. Since these tests may be run outside Linux CI, consider guarding with function_exists('posix_kill')/defined('SIGTERM') and falling back to a portable termination strategy (e.g., tracking a proc_open handle) or skipping cleanup with a warning.
| posix_kill(self::$serverPid, SIGTERM); | |
| if (function_exists('posix_kill') && defined('SIGTERM')) { | |
| posix_kill(self::$serverPid, SIGTERM); | |
| } else { | |
| // POSIX functions or SIGTERM are not available; skip cleanup instead of fatal error. | |
| trigger_error( | |
| 'POSIX extension or SIGTERM not available; skipping PHP built-in server cleanup in SmokeTest::tearDownAfterClass().', | |
| E_USER_WARNING | |
| ); | |
| } |
| // Ensure tpl_c exists for Smarty template compilation | ||
| $tplCDir = $rootDir . '/tpl_c'; | ||
| if (!is_dir($tplCDir)) { | ||
| mkdir(directory: $tplCDir, permissions: 0777, recursive: true); |
There was a problem hiding this comment.
mkdir(...) is not checked for failure. If tpl_c/ can’t be created (permissions, read-only FS), the tests will continue and later failures may be less clear. Check the return value and fail early with an actionable error message.
| mkdir(directory: $tplCDir, permissions: 0777, recursive: true); | |
| $created = @mkdir(directory: $tplCDir, permissions: 0777, recursive: true); | |
| if (!$created && !is_dir($tplCDir)) { | |
| self::fail( | |
| sprintf( | |
| 'Failed to create Smarty compile directory "%s". Please ensure it exists and is writable.', | |
| $tplCDir | |
| ) | |
| ); | |
| } |
|
|
||
| $uri = $_SERVER['REQUEST_URI']; | ||
| $path = parse_url($uri, PHP_URL_PATH); | ||
|
|
There was a problem hiding this comment.
parse_url($uri, PHP_URL_PATH) can return null/false (e.g., malformed REQUEST_URI). $path is then passed into str_starts_with() and concatenated into paths, which can trigger a TypeError. Consider defaulting $path to '/' when parse_url doesn’t return a string, and/or failing with a 400.
| if (!is_string($path)) { | |
| http_response_code(400); | |
| echo "Bad Request: Malformed request URI\n"; | |
| return; | |
| } |
| // Replicate root .htaccess: redirect non-/Web/ requests to /Web/ | ||
| if (!str_starts_with($path, '/Web/') && $path !== '/Web') { | ||
| header('Location: /Web' . $path, true, 302); | ||
| return; |
There was a problem hiding this comment.
The redirect meant to replicate the root .htaccess drops the query string because it uses $path instead of the original REQUEST_URI. Apache’s rewrite preserves the query string by default, so requests like /?a=1 would become /Web/ without a=1. Preserve the original query string in the Location header (or reconstruct it from parse_url).
| $config['settings']['logging']['level'] = 'none'; | ||
| $config['settings']['email']['enabled'] = false; | ||
|
|
||
| file_put_contents($configPath, "<?php\nreturn " . var_export($config, true) . ";\n"); |
There was a problem hiding this comment.
file_put_contents() isn’t checked for failure. If the runner’s filesystem is read-only or config/ isn’t writable, the script will still print success and the integration workflow will fail later in harder-to-diagnose ways. Check the return value and exit(1) with an error on failure.
| file_put_contents($configPath, "<?php\nreturn " . var_export($config, true) . ";\n"); | |
| $bytesWritten = file_put_contents($configPath, "<?php\nreturn " . var_export($config, true) . ";\n"); | |
| if ($bytesWritten === false) { | |
| fwrite(STDERR, "ERROR: Failed to write config file to: $configPath\n"); | |
| exit(1); | |
| } |
| $rootDir = realpath(__DIR__ . '/../../'); | ||
| $routerScript = realpath(__DIR__ . '/router.php'); | ||
|
|
There was a problem hiding this comment.
realpath() can return false (e.g., unexpected working directory/layout). Passing that into escapeshellarg()/string concatenation can yield an invalid php -S command and confusing failures. Validate $rootDir and $routerScript are non-false and fail the test early with a clear message if resolution fails.
| $rootDir = realpath(__DIR__ . '/../../'); | |
| $routerScript = realpath(__DIR__ . '/router.php'); | |
| $rootDirPath = __DIR__ . '/../../'; | |
| $routerScriptPath = __DIR__ . '/router.php'; | |
| $rootDir = realpath($rootDirPath); | |
| $routerScript = realpath($routerScriptPath); | |
| if ($rootDir === false || $routerScript === false) { | |
| self::fail(sprintf( | |
| 'Failed to resolve paths for integration smoke test. rootDirPath="%s" (realpath: %s), routerScriptPath="%s" (realpath: %s)', | |
| $rootDirPath, | |
| var_export($rootDir, true), | |
| $routerScriptPath, | |
| var_export($routerScript, true) | |
| )); | |
| } |
02dbb53 to
6cc6631
Compare
This sets up a basic integration test in GitHub CI. Where it will do a quick smoke test to see if it can login to the server and then access a few pages.
6cc6631 to
ca7863e
Compare
This sets up a basic integration test in GitHub CI. Where it will do a
quick smoke test to see if it can login to the server and then access a
few pages.