chore(ci): move PHP unit tests to GitHub workflows

[phil-davis]

No description provided.

[phil-davis]

Frustrating - 2 FTP servers were used in the testing on drone.
I can't find a combination for starting either of them, the tests always report:

League\Flysystem\ConnectionRuntimeException: Could not login with connection: localhost::21, username: test

I tried explicitly putting 127.0.0.1 and/or 172.17.0.1 with the same result.
I tried explicitly exposing port 20 and 21, but actually that is not needed when using --network host

This kind of thing worked easily with a test mail server:

          docker run -d --rm --network host \
            --name email \
            inbucket/inbucket

It was available on the standard email ports, and the tests found and used it.

What do I need to do to get an FTP server responding on ports 20/21?

[dj4oC]

Adds a new reusable php-unit-ftp.yml GitHub Actions workflow plus two new CI matrix jobs (proftp, vsftp) to replace the old Drone-based PHP unit test run, including FTP test servers.

Findings

Security: No exposed real secrets or credential leakage — the test/test FTP login is a dummy, ephemeral value confined to a throwaway --network host container that only lives for the job's lifetime. permissions: contents: read is preserved, and no existing security-relevant CI gate (code style, semantic-commit check) is touched or weakened by this change.

Stability (blocking): This is very likely the actual root cause of the FTP login failures the author is stuck on. In both the Run proftp service and Run vsftp service steps, the credential/config values are declared only as step-level shell env vars (FTP_USERNAME/FTP_PASSWORD for proftp; FTP_USER/FTP_PASS/PASV_MIN_PORT/PASV_MAX_PORT/PASV_ADDR_RESOLVE/PASV_ADDRESS for vsftp) — but the docker run commands never forward them into the container via -e:

docker run -d --rm --network host \
  --name proftp \
  driesva/proftpd:1.3.8

Neither driesva/proftpd:1.3.8 nor fauria/vsftpd will boot with a test/test account or the intended passive-port range; they'll fall back to whatever default account/config ships in the image. That matches exactly the Could not login with connection ...username: test error reported in the PR comment — it isn't a port-20/21-exposure problem (host networking already avoids that), it's that the containers were never actually told to create the test user. Suggested fix: add -e FTP_USERNAME="$FTP_USERNAME" -e FTP_PASSWORD="$FTP_PASSWORD" to the proftp docker run, and -e FTP_USER="$FTP_USER" -e FTP_PASS="$FTP_PASS" -e PASV_MIN_PORT="$PASV_MIN_PORT" -e PASV_MAX_PORT="$PASV_MAX_PORT" -e PASV_ADDR_RESOLVE="$PASV_ADDR_RESOLVE" -e PASV_ADDRESS="$PASV_ADDRESS" to the vsftp one (worth double-checking each image's documented env-var names, since proftpd images vary on FTP_USER vs FTP_USERNAME). Secondary/minor: there's no wait or health-check after docker run -d before the server is relied on later in the job — once login works, a short retry/wait loop would guard against races on a cold container start.

Performance: Nothing blocking — the added matrix scope (proftp/vsftp × php × db) mirrors the existing php-unit reusable workflow pattern and isn't excessive.

Test coverage: n/a — CI infra only, no PHP source or assertions changed. Once the containers actually boot with the intended credentials, these jobs will give this app its first GH Actions coverage of the FTP backend (previously only exercised on Drone).

Verdict

Changes requested

🤖 Automated review by Claude Code (security · stability · performance · coverage)

---

Generated by Claude Code

[dj4oC]

• Security: no findings
• Stability: 1 finding — see review
• Performance: no findings
• Test coverage: n/a (CI infra change)
• TODOs found: none
• Dependency touched: no
• CI status: failing: PHP Unit with proftp service (8.3, postgres:10.21), PHP Unit with vsftp service (8.3, postgres:10.21) — remaining proftp/vsftp matrix jobs cancelled via fail-fast
• Stale review: no

🤖 Automated review by Claude Code (security · stability · performance · coverage)

---

Generated by Claude Code

[phil-davis]

The unit tests with proftpd passed:
https://github.com/owncloud/files_external_ftp/actions/runs/29001823657/job/86063976145?pr=118

PHPUnit 9.6.35 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.32 with Xdebug 3.5.3
Configuration: ./phpunit.xml

................................................................. 65 / 85 ( 76%)
....................                                              85 / 85 (100%)

Time: 00:35.392, Memory: 24.00 MB

OK (85 tests, 365 assertions)

with vsftpd they failed:
https://github.com/owncloud/files_external_ftp/actions/runs/29001823657/job/86063975527?pr=118

PHPUnit 9.6.35 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.32 with Xdebug 3.5.3
Configuration: ./phpunit.xml

EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE 65 / 85 ( 76%)
EEEEEEEEEEEEEEEEEEEE                                              85 / 85 (100%)

Time: 00:00.598, Memory: 24.00 MB

There were 85 errors:

1) OCA\FilesExternalFtp\Tests\FtpTest::testStat
ftp_rawlist(): php_connect_nonb() failed: Operation now in progress (115)

/home/runner/work/files_external_ftp/files_external_ftp/lib/composer/league/flysystem/src/Adapter/Ftp.php:570
/home/runner/work/files_external_ftp/files_external_ftp/lib/composer/league/flysystem/src/Adapter/Ftp.php:405
/home/runner/work/files_external_ftp/files_external_ftp/lib/composer/league/flysystem/src/Adapter/AbstractFtpAdapter.php:610
/home/runner/work/files_external_ftp/files_external_ftp/lib/composer/league/flysystem/src/Filesystem.php:58
/home/runner/work/files_external_ftp/files_external_ftp/lib/private/Files/Storage/Flysystem.php:80
/home/runner/work/files_external_ftp/files_external_ftp/lib/private/Files/Storage/Flysystem.php:132
/home/runner/work/files_external_ftp/files_external_ftp/apps/files_external_ftp/tests/unit/FtpTest.php:40
...

All fail with the same problem, "Operation now in progress".

[phil-davis]

All fail with the same problem, "Operation now in progress"

Fixed that by specifying the correct host name "localhost" in tests/unit/config.json

Added a commit to delete drone.star - all CI that was done there is now done in the GitHub workflows.

[phil-davis]

And thanks to Claude for suggesting how to pass env variables through into the proftp and vsftp docker images.