Improve 3.x test and CI workflows (#146)

* Improve 3.x test and CI workflows

* Fix Craft 4 compatibility workflow resolution

* Document local Craft 4 compatibility workflow

* Use Uri query cleanup in UrlSigner

* Avoid phpstan version-specific generateUrl calls

* Ignore version-specific phpstan errors

* Use inline phpstan compatibility ignores

Author: timkelty

.github/workflows/ci.yml

@@ -3,8 +3,7 @@ on:
   workflow_dispatch:
   push:
     branches:
-      - 1.x
-      - 2.x
+      - 3.x
   pull_request:
 permissions:
   contents: read
@@ -18,4 +17,50 @@ jobs:
     with:
       node_version: '18'
       php_version: '8.3'
-      jobs: '["ecs", "phpstan", "prettier"]'
+      jobs: '["ecs", "prettier"]'
+
+  compatibility:
+    name: Compatibility (Craft ${{ matrix.craft }})
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        include:
+          - craft: '^4.6'
+            flysystem: '^1.0'
+          - craft: '^5'
+            flysystem: '^2.0'
+    services:
+      mysql:
+        image: mysql:8.0
+        ports:
+          - 33066:3306
+        env:
+          MYSQL_DATABASE: db
+          MYSQL_USER: db
+          MYSQL_PASSWORD: db
+          MYSQL_ROOT_PASSWORD: root
+        options: >-
+          --health-cmd="mysqladmin ping -h 127.0.0.1 -udb -pdb"
+          --health-interval=10s
+          --health-timeout=5s
+          --health-retries=10
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: shivammathur/setup-php@v2
+        with:
+          php-version: '8.3'
+          coverage: none
+
+      - name: Resolve dependencies
+        run: composer update "craftcms/cms:${{ matrix.craft }}" "craftcms/flysystem:${{ matrix.flysystem }}" --with-all-dependencies --no-interaction --no-progress --prefer-dist --no-audit
+
+      - name: Prepare test environment
+        run: composer test:init
+
+      - name: Run PHPStan
+        run: composer phpstan
+
+      - name: Run unit tests
+        run: composer test

.lintstagedrc.json

@@ -1,4 +1,4 @@
 {
-  "**/*.php": ["composer run fix-cs"],
+  "**/*.php": ["composer run cs:fix"],
   "*": "prettier --ignore-unknown --write"
 }

README.md

@@ -26,6 +26,31 @@ When you [deploy](https://craftcms.com/knowledge-base/cloud-deployment) a projec
 
 When setting up your project’s assets, use the provided **Craft Cloud** filesystem type. Read more about [managing assets in Cloud projects](https://craftcms.com/knowledge-base/cloud-assets).
 
+## Testing
+
+The Codeception `unit` suite on `3.x` boots Craft and expects a local test database.
+
+```bash
+composer test:init
+composer test:up
+composer test
+composer test:down
+```
+
+`composer test:init` will create `tests/.env` from `tests/.env.example` if it does not already exist. `composer test:up` uses that file when starting the MySQL service defined in `tests/docker-compose.yaml`.
+
+For local compatibility work on `3.x`, it can be helpful to keep your main checkout on the default/latest Craft 5 dependency set and use a separate Git worktree for Craft 4 so each checkout can keep its own `vendor/`, `composer.lock`, and `tests/.env` state.
+
+```bash
+git worktree add ../cloud-3x-craft4 3.x
+
+# In the Craft 4 worktree:
+composer update "craftcms/cms:^4.6" "craftcms/flysystem:^1.0" --with-all-dependencies --no-audit
+
+# In your main checkout:
+composer update "craftcms/cms:^5" "craftcms/flysystem:^2.0" --with-all-dependencies
+```
+
 ## Developer Features
 
 ### Template Helpers

composer.json

@@ -28,9 +28,15 @@
     "codeception/module-yii2": "^1.0"
   },
   "scripts": {
-    "check-cs": "ecs check --ansi",
-    "fix-cs": "ecs check --ansi --fix",
+    "cs:check": "ecs check --ansi",
+    "cs:fix": "ecs check --ansi --fix",
     "phpstan": "phpstan --memory-limit=1G",
+    "test:init": "@php -r \"file_exists('tests/.env') || copy('tests/.env.example', 'tests/.env');\"",
+    "test:up": [
+      "@test:init",
+      "docker compose --env-file tests/.env -f tests/docker-compose.yaml up -d --wait"
+    ],
+    "test:down": "docker compose --env-file tests/.env -f tests/docker-compose.yaml down",
     "test": "codecept run unit",
     "test:coverage": "codecept run unit --coverage"
   },

phpstan.neon

@@ -3,5 +3,6 @@ includes:
 
 parameters:
     level: 5
+    reportUnmatchedIgnoredErrors: false
     paths:
         - src

src/UrlSigner.php

@@ -26,9 +26,10 @@ public function sign(string $url): string
 
     private function getSigningData(string $url): string
     {
-        return Modifier::wrap($url)
+        return (string) Modifier::wrap($url)
             ->removeQueryParameters($this->signatureParameter)
-            ->sortQuery();
+            ->sortQuery()
+            ->removeEmptyQueryPairs();
     }
 
     public function verify(string $url): bool

src/imagetransforms/ImageTransformer.php

@@ -26,10 +26,11 @@ class ImageTransformer extends Component implements ImageTransformerInterface
     public function getTransformUrl(Asset $asset, \craft\models\ImageTransform $imageTransform, bool $immediately): string
     {
         if (version_compare(Craft::$app->version, '5.0', '>=')) {
+            // @phpstan-ignore argument.type, arguments.count (Craft 5 compatibility)
             $assetUrl = Html::encodeSpaces(Assets::generateUrl($asset));
         } else {
             $fs = $asset->getVolume()->getTransformFs();
-            /** @phpstan-ignore argument.type (Craft 4 compatibility) */
+            // @phpstan-ignore argument.type, arguments.count (Craft 4 compatibility)
             $assetUrl = Html::encodeSpaces(Assets::generateUrl($fs, $asset));
         }
 

tests/docker-compose.yaml

@@ -8,6 +8,16 @@ services:
       MYSQL_DATABASE: ${CRAFT_DB_DATABASE}
       MYSQL_USER: ${CRAFT_DB_USER}
       MYSQL_PASSWORD: ${CRAFT_DB_PASSWORD}
+    healthcheck:
+      test:
+        [
+          'CMD-SHELL',
+          'mysqladmin ping -h 127.0.0.1 -u$$MYSQL_USER -p$$MYSQL_PASSWORD --silent',
+        ]
+      interval: 5s
+      timeout: 5s
+      retries: 20
+      start_period: 10s
 #  postgres:
 #    image: postgres:15
 #    ports:

tests/unit/AssetsControllerTest.php

@@ -0,0 +1,50 @@
+<?php
+
+namespace craft\cloud\tests\unit;
+
+use Codeception\Test\Unit;
+use Craft;
+use craft\cloud\controllers\AssetsController;
+use craft\models\Volume;
+use ReflectionMethod;
+
+class AssetsControllerTest extends Unit
+{
+    /**
+     * @var \UnitTester
+     */
+    protected $tester;
+
+    public function testVolumeSubpathReturnsEmptyStringOnCraft4(): void
+    {
+        $volume = new Volume();
+
+        if (method_exists($volume, 'getSubpath')) {
+            $this->markTestSkipped('Craft 5 volume subpath behavior is covered by a separate test.');
+        }
+
+        $this->assertSame('', $this->invokeVolumeSubpath($volume));
+    }
+
+    public function testVolumeSubpathReturnsVolumeSubpathOnCraft5(): void
+    {
+        $volume = new Volume();
+
+        if (!method_exists($volume, 'getSubpath')) {
+            $this->markTestSkipped('Craft 4 volumes do not implement getSubpath().');
+        }
+
+        $volume->setSubpath('volume-prefix');
+
+        $this->assertSame('volume-prefix/', $this->invokeVolumeSubpath($volume));
+    }
+
+    private function invokeVolumeSubpath(Volume $volume): string
+    {
+        $controller = new AssetsController('cloud-assets', Craft::$app);
+        $method = new ReflectionMethod($controller, 'volumeSubpath');
+        $method->setAccessible(true);
+
+        return $method->invoke($controller, $volume);
+    }
+}