feat: Add workflow to test Drupal installations#23
Conversation
This commit introduces a new GitHub Actions workflow (`test-drupal.yml`) designed to test Drupal installations. The workflow supports multiple PHP versions (8.4, 8.5) and different web server variants (Apache, FrankenPHP, FPM with Nginx). The workflow performs the following steps for each matrix combination: - Builds a Docker image specific to the PHP version and web server variant. - Executes Drupal installation using Composer and Drush. - Configures the web server (Apache rewrite module or Nginx). - Attempts to install Drupal using the SQLite database. - Verifies a successful installation by making an HTTP request and checking for a 200 OK status. This ensures that Drupal can be successfully installed and run with the configured environments.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new GitHub Actions workflow to run Drupal install tests (Apache/FrankenPHP and PHP‑FPM) across PHP 8.4/8.5, and adjusts the docker-buildx workflow trigger and PHP variant matrix formatting. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as GitHub Actions
participant Runner as Runner
participant Docker as Docker Engine
participant Compose as docker-compose
participant Container as Web container
participant Site as Drupal (Composer/Drush)
participant Health as curl
GH->>Runner: start test-drupal workflow
Runner->>Docker: build/run image(s) (apache/frankenphp variants)
Runner->>Compose: write nginx config & docker-compose up (fpm job)
Docker->>Container: start webserver + PHP
Compose->>Container: start services (nginx + php-fpm)
Runner->>Site: run Composer, run Drush site-install inside container
Runner->>Health: curl http://container:port
Health-->>Runner: HTTP 200 / non-200
Runner->>Docker: stop containers / docker-compose down (cleanup)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.github/workflows/test-drupal.yml:
- Around line 50-54: The HTTP status check is fragile because it greps for
"HTTP/1.1 200 OK"; replace the grep-based check with a protocol-agnostic curl
write-out check (use curl -sI or -s with -o /dev/null and -w "%{http_code}"
against http://localhost:$PORT and assert the result equals 200) to handle
HTTP/2 and other variants; update the curl check in the block that follows the
site-install command (the line with curl -sI http://localhost:$PORT | grep
"HTTP/1.1 200 OK") and apply the identical change in the test-fpm job (the same
curl-based check referenced at line 124).
- Around line 45-48: The current workflow runs docker exec $CONTAINER_NAME
service apache2 restart which can stop the container when Apache is PID 1;
replace that restart call with a graceful reload using apache2ctl (e.g. run
docker exec $CONTAINER_NAME apache2ctl -k graceful) after enabling the module
with docker exec $CONTAINER_NAME a2enmod rewrite so the container isn't killed —
update the lines referencing $CONTAINER_NAME and the service apache2 restart
invocation accordingly.
- Line 113: The workflow step currently invokes the deprecated v1 CLI string
"docker-compose -f docker-compose.test.yml up -d --build"; update that command
to use Docker Compose v2 by replacing the hyphenated binary with the plugin form
"docker compose -f docker-compose.test.yml up -d --build" so the GitHub Actions
runner (which provides Compose v2) can execute the step successfully.
🧹 Nitpick comments (3)
.github/workflows/test-drupal.yml (3)
27-54: Missing container cleanup on failure.The
test-fpmjob has anif: always()cleanup step, but this job does not. If any step fails, the containertest-containeris left running (albeit on an ephemeral runner). More importantly, if you ever move to self-hosted runners, leaked containers become a real problem. Add a cleanup step for consistency.Proposed fix — add cleanup step after the test step
sleep 10 # Use curl to check for a successful installation curl -sI http://localhost:$PORT | grep "HTTP/1.1 200 OK" + - name: Cleanup + if: always() + run: docker rm -f test-container || true
37-38: Consider a retry loop instead of fixedsleep+ singlecurl.Two
sleep 10calls (lines 38 and 52) add 20 seconds of fixed wait time, and the final health check is a single attempt with no retry. If the server is slightly slow to start (especially during Drupal bootstrap), the check fails flakily. A poll loop is more robust and typically faster:Example retry pattern
# Replace the final sleep + curl with: for i in $(seq 1 30); do HTTP_CODE=$(curl -so /dev/null -w "%{http_code}" --max-time 5 http://localhost:$PORT 2>/dev/null) if [ "$HTTP_CODE" = "200" ]; then echo "Drupal responded with 200 OK" exit 0 fi echo "Attempt $i: got $HTTP_CODE, retrying..." sleep 2 done echo "Drupal did not respond with 200 after 30 attempts" exit 1Also applies to: 52-54
92-110: Nginx config is functional but missing$uri/intry_files.The typical Drupal Nginx recipe uses
try_files $uri $uri/ /index.php?$query_string;to also handle directory index lookups. Omitting$uri/won't break the basic health check but could cause issues with clean URL resolution for directory-style paths. Minor for a test workflow, but worth noting.Proposed fix
location / { - try_files \$uri /index.php?\$query_string; + try_files \$uri \$uri/ /index.php?\$query_string; }
This commit makes the following changes:
- **docker-buildx.yml**:
- Changes the `push` trigger to `workflow_run` to only build Docker images after the "Test Drupal Installation" workflow succeeds.
- Adds a condition to the `buildx` job to prevent it from running if the workflow was triggered by `workflow_run` and the previous workflow failed.
- **test-drupal.yml**:
- Replaces `docker-compose` with `docker compose` for consistency and to leverage the `docker-compose-plugin`.
- Adds a step to install `docker-compose-plugin` to ensure compatibility.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/test-drupal.yml:
- Around line 72-94: The heredoc blocks generating docker-compose.test.yml (the
`cat <<EOF > docker-compose.test.yml` block) and the nginx config are indented
inside the workflow run step so their leading spaces are preserved and produce
invalid YAML; fix by using an unindented heredoc: move the `cat <<'EOF' >
docker-compose.test.yml` (and the nginx `cat <<'EOF' > ./nginx.conf` block) and
their EOF-delimited content to column 1 (no leading spaces) so the produced
files start at column 1, or alternatively use a heredoc that strips indentation
(e.g., align with `<<-EOF` and use tabs) and quote the delimiter (`'EOF'`) to
prevent shell expansion while preserving proper YAML/NGINX formatting.
🧹 Nitpick comments (4)
.github/workflows/test-drupal.yml (4)
18-18:actions/checkout@v4vs@v6indocker-buildx.yml.This workflow uses
actions/checkout@v4whiledocker-buildx.ymluses@v6. Consider aligning to the same version for consistency.
66-70:docker-compose-pluginis likely already available onubuntu-latest.GitHub Actions
ubuntu-latest(Ubuntu 24.04) ships with Docker Compose v2 (2.38+). Thedocker composesubcommand is already available, making thisapt-get install docker-compose-pluginstep redundant. It adds ~10s of CI time for no benefit.Consider removing this step or guarding it with a version check.
27-54: Missing container cleanup fortest-apache-frankenphpjob.The
test-fpmjob has anif: always()cleanup step, buttest-apache-frankenphpdoesn't stop/remove the container on failure. Add a cleanup step for consistency and to ensure clean teardown.Proposed fix — add after the test step
- name: Stop container if: always() run: docker rm -f test-container
37-38:sleep 10is a fragile wait strategy.Fixed sleeps are unreliable — the container might not be ready in 10s on a slow runner, or you're wasting time on a fast one. Consider polling for readiness:
for i in $(seq 1 30); do curl -sf http://localhost:$PORT > /dev/null && break sleep 1 doneThis applies to all
sleepcalls used for waiting on services (lines 38, 52, 123, 129).
.github/workflows/test-drupal.yml
Outdated
| - name: Create docker-compose for testing | ||
| run: | | ||
| cat <<EOF > docker-compose.test.yml | ||
| services: | ||
| php-fpm: | ||
| build: | ||
| context: ./php8/fpm-alpine | ||
| args: | ||
| PHP_VERSION: ${{ matrix.php_version }} | ||
| container_name: php-fpm | ||
| volumes: | ||
| - drupal-code:/var/www/html | ||
| nginx: | ||
| image: nginx:alpine | ||
| container_name: nginx | ||
| ports: | ||
| - "8080:80" | ||
| volumes: | ||
| - drupal-code:/var/www/html | ||
| - ./nginx.conf:/etc/nginx/conf.d/default.conf | ||
| volumes: | ||
| drupal-code: | ||
| EOF |
There was a problem hiding this comment.
Critical: Heredoc indentation will produce an invalid docker-compose.test.yml.
cat <<EOF preserves all leading whitespace verbatim. Since every line inside the heredoc (lines 75–93) is indented to match the workflow YAML, the resulting file will have services: at ~column 11 instead of column 1. Docker Compose will fail to parse this as valid YAML.
Strip the indentation from the heredoc content so the generated file has correct YAML structure:
Proposed fix
- name: Create docker-compose for testing
run: |
- cat <<EOF > docker-compose.test.yml
- services:
- php-fpm:
- build:
- context: ./php8/fpm-alpine
- args:
- PHP_VERSION: ${{ matrix.php_version }}
- container_name: php-fpm
- volumes:
- - drupal-code:/var/www/html
- nginx:
- image: nginx:alpine
- container_name: nginx
- ports:
- - "8080:80"
- volumes:
- - drupal-code:/var/www/html
- - ./nginx.conf:/etc/nginx/conf.d/default.conf
- volumes:
- drupal-code:
- EOF
+ cat <<'EOF' > docker-compose.test.yml
+services:
+ php-fpm:
+ build:
+ context: ./php8/fpm-alpine
+ args:
+ PHP_VERSION: ${{ matrix.php_version }}
+ container_name: php-fpm
+ volumes:
+ - drupal-code:/var/www/html
+ nginx:
+ image: nginx:alpine
+ container_name: nginx
+ ports:
+ - "8080:80"
+ volumes:
+ - drupal-code:/var/www/html
+ - ./nginx.conf:/etc/nginx/conf.d/default.conf
+volumes:
+ drupal-code:
+EOFNote: Using <<'EOF' (quoted) prevents shell expansion inside the heredoc, which is fine here since ${{ matrix.php_version }} is expanded by GitHub Actions before the shell sees the script. The same indentation issue applies to the nginx config heredoc (lines 96–116) — though nginx is whitespace-tolerant, it's good practice to fix it too.
🤖 Prompt for AI Agents
In @.github/workflows/test-drupal.yml around lines 72 - 94, The heredoc blocks
generating docker-compose.test.yml (the `cat <<EOF > docker-compose.test.yml`
block) and the nginx config are indented inside the workflow run step so their
leading spaces are preserved and produce invalid YAML; fix by using an
unindented heredoc: move the `cat <<'EOF' > docker-compose.test.yml` (and the
nginx `cat <<'EOF' > ./nginx.conf` block) and their EOF-delimited content to
column 1 (no leading spaces) so the produced files start at column 1, or
alternatively use a heredoc that strips indentation (e.g., align with `<<-EOF`
and use tabs) and quote the delimiter (`'EOF'`) to prevent shell expansion while
preserving proper YAML/NGINX formatting.
Replaces the manual installation of `docker-compose-plugin` with the `docker/setup-compose-action@v1` action for a more robust and maintainable setup.
This commit installs `rsync` in the Docker container used for Drupal testing and modifies the `composer create-project` command to use `rsync`. This changes how the Drupal project is set up within the container, ensuring that all project files are placed directly in the web root.
Replaces `curl -sI ... | grep "HTTP/1.1 200 OK"` with `curl -v --fail` for more robust HTTP status code checking in the Drupal test workflow. Also updates the Apache restart command to use `apache2ctl -k graceful` on the `apache-trixie` variant.
This commit refactors the Drupal test workflow to enhance its reliability and debugging capabilities. Key changes include: - **Introduced environment variables** for `WEB_ROOT`, `PORT`, and `CONTAINER_NAME` to promote consistency and readability. - **Separated dependency installation** from the Drupal installation step for clearer execution flow. - **Configured Apache rewrite module** for `apache-trixie` variant. - **Added comprehensive logging** for failed test runs, including file permissions, container logs, and Apache/Nginx error logs, to facilitate faster troubleshooting. - **Improved log output formatting** using `::group::` for better organization. - **Standardized verification** using `curl` for both `apache-trixie` and `php-fpm` test jobs.
This commit enhances the Drupal testing workflows by enabling PHP error display and setting the error reporting level to `E_ALL`. This change will make it easier to debug issues encountered during the test runs. Additionally, it refactors the `Install Drupal site` step to correctly determine the web server user for `chown` operations, ensuring proper file ownership.
The ownership of the drupal project files was not correctly set, causing issues during the installation process. This commit adds a `chown` command to ensure that the `www-data` user has the correct ownership of the project files within the container.
This commit modifies the GitHub Actions workflows to use `tail -n 100` instead of `cat` when displaying Apache and Nginx error logs. This change is intended to reduce the amount of log data outputted, making it more concise and easier to review during CI runs.
Adds a 10-second timeout to `docker exec` commands for collecting Apache and Nginx error logs. This prevents the workflow from hanging indefinitely if the commands fail to execute. Also, the order of log collection for Nginx and PHP-FPM has been adjusted for better readability.
…tion This change modifies the GitHub Actions workflow for testing Drupal. It ensures that the `web/sites/default/files` directory is created and made writable (using `chmod -R 777`) before the Drupal site installation command is executed. This is crucial for the site installation process, especially when using SQLite as the database, as it needs to write files to this directory. The change addresses two instances within the workflow: 1. In the `Install Drupal site` job for the `apache-trixie` variant, the permissions for `web/sites/default/files` are now set to `777` instead of the previous attempt to set ownership to `www-data:www-data` which might not be sufficient or correctly configured in all container environments. 2. A similar adjustment is made for the primary Drupal installation steps, ensuring the directory is writable. This fix prevents potential installation failures due to file permission issues within the Docker container during the CI build.
The chown command was specific to the apache-trixie variant and is no longer needed as part of the standard Drupal installation process in the CI workflow.
The previous configuration was setting file permissions on `files/` which is unnecessary. This change relaxes those permissions to `sites/default/` to ensure proper site installation in the CI environment.
This change modifies the conditions under which Docker images are built and pushed to Docker Hub. Previously, images were pushed for all events except pull requests. Now, pushes are restricted to the `main` branch, specifically when triggered by a `workflow_run` event originating from the `main` branch, or directly on the `main` branch for other event types. This ensures that only production-ready images are published.
This change simplifies the `chmod -R 777` command in the Drupal test workflow to directly target the web root directory, removing the need to explicitly create the `files` directory beforehand.
Moves Docker Compose configurations into separate files and sets environment variables for better readability and maintainability.
|
Replacing with #24 |
1 participant
This commit introduces a new GitHub Actions workflow (
test-drupal.yml) designed to test Drupal installations. The workflow supports multiple PHP versions (8.4, 8.5) and different web server variants (Apache, FrankenPHP, FPM with Nginx).The workflow performs the following steps for each matrix combination:
This ensures that Drupal can be successfully installed and run with the configured environments.
Summary by CodeRabbit
Tests
Chores