CI matrix for MySQL, MariaDB, PostgreSQL and PHP, with fixes it uncovered - #1
Merged
Merged
Conversation
Test on a baseline of PHP 8.4, MySQL 8.4 and PostgreSQL 18, plus one job per other version of each axis (MySQL 5.7 and 8.0, PostgreSQL 14 and 16, PHP 8.2, 8.3 and 8.5). Database data lives on tmpfs so the suite's tens of thousands of small commits do not wait on fsync. A compose.yaml offers the same servers locally, and CONTRIBUTING.md explains how to use them. Fixes uncovered while making the matrix pass: - PgsqlAdapter read pg_attrdef.adsrc, removed in PostgreSQL 12, so schema introspection failed on every supported PostgreSQL. Use pg_get_expr(). - MysqlAdapter emitted datetime values with a time zone offset, which MySQL before 8.0.19 (and MariaDB) rejects under strict sql_mode. Format plain wall-clock time for those servers. - DatabaseTest left an unreachable adapter as the default connection when setUp() skipped, because PHPUnit 11 does not run tearDown() then. That silently skipped 278 unrelated tests whenever PostgreSQL was down. - The connect-with-port test ignored the configured port. - phpunit.xml.dist used PHPUnit 9 attributes. Remove the obsolete Travis configuration.
The first CI run was the first time the memcached tests ran since the models and exceptions moved into namespaces: - ActiveRecordCacheTest::setUp() called parent::set_up(), which the snake_case __call() helper turned back into setUp() on the same class. The infinite recursion hit the stack limit on PHP 8.3+ after minutes and segfaulted PHP 8.2. Call parent::setUp() directly. - CacheModelTest reflected on the bare class names "Author" and "Publisher", which no longer exist; use the namespaced classes. The model cache key now carries the namespace, so expect that too. - lib/cache/Memcached.php threw ActiveRecord\CacheException, which no longer exists, so a failed connection surfaced as a class-not-found Error instead of ActiveRecord\Exceptions\CacheException.
The OCI adapter was never completed, has no CI coverage, and its tests have been skipped everywhere for years. Drop the adapter, its tests and SQL fixtures, the Oracle-only branches in Model and the test helpers, and the mentions in the documentation.
The base datetime format appended PHP's time zone offset. MySQL before 8.0.19, MariaDB and SQLite do not understand it, and MySQL 8.0.19+ makes it worse by converting the value to the session time zone, so a value only round-tripped when that happened to match PHP's zone; with PHP in Europe/Madrid and a UTC server, 18:00 came back as 16:00. Use 'Y-m-d H:i:s' everywhere, drop the per-version MySQL override and the now identical SQLite override, and add a round-trip test that runs with PHP in a non-UTC zone on every adapter.
Table checked `$this->conn instanceof PgsqlAdapter` without importing ActiveRecord\Adapters\PgsqlAdapter, so since the PSR-4 move the check was always false, the table name was quoted, and the pg_catalog lookup matched nothing. Models on PostgreSQL therefore had no columns: no type casting, no defaults, no primary key detection. The adapter tests never noticed because they query the connection directly; the new datetime round-trip test is the first model-level test that runs on PostgreSQL.
Remove every require/require_once from lib: SQLBuilder and Model pulled in Expressions and Validations by hand, Cache loaded its backend from a file path, and Connection and Table still carried commented-out ones. Move the memcached backend to lib/Cache/Memcached.php as ActiveRecord\Cache\Memcached, where PSR-4 finds it; Cache::initialize() now resolves the backend class by scheme and throws CacheException for an unknown one instead of failing on a missing file. Add the imports that Reflections and Serialization were missing for their exception classes since the move to ActiveRecord\Exceptions, and fix three tests importing DatabaseException from its old location. The has-many-through test models in the foo\bar\biz namespace move from an included helper file to test/models/biz with an autoload-dev entry.
The jpfuentes2GH-101 test needs models whose namespace differs from the configured model namespace, so that relationships must resolve related classes through the owning model's namespace rather than the Config fallback. A sub-namespace of TestModels satisfies that and is already covered by the existing autoload-dev rule, so the extra foo\bar\biz entry goes.
The test bootstrap now reads PHPAR_ADAPTER (mysql, pgsql or sqlite) to pick the default connection, replacing the --adapter switch that PHPUnit 11 rejects, so the model-level tests can run against every adapter. PHPAR_SLOW_TESTS=true replaces --slow-tests. Running the suite with PostgreSQL as the default exposed these library bugs, which MySQL hid: - Expressions rendered a null hash condition as "IS ?" with a bound NULL. MySQL's emulated prepares interpolate that into IS NULL; PostgreSQL rejects a parameter after IS. Render the literal instead. - Writes and hash conditions used the inflected attribute name as the column name (some_date for some_Date). MySQL matches column names case-insensitively; PostgreSQL does not once the name is quoted. Table::column_name_for() resolves the real column name, and eager loading uses it for the related table's keys. - find() with a non-numeric value for an integer primary key raised a type error on PostgreSQL, where MySQL coerced it to 0 and found nothing. Throw RecordNotFound up front on every adapter. Test suite adjustments for engine differences: - PostgreSQL and SQLite schemas made events.host_id NOT NULL while MySQL allowed null; the belongs-to-null test needs the null. - Two tests used MySQL-only SQL (double-quoted string literal, backtick quoting); use standard quotes and quote_name(). - update_all row counts: MySQL reports rows changed, PostgreSQL and SQLite rows matched, so the expectation depends on the adapter. - The SQL builder test expects IS NULL and no bound null; eager-loading assertions read the real column name; the connection-manager test uses the configured default instead of "mysql"; the date-format test reloads by id because first() without an order is arbitrary after an update on PostgreSQL.
The three PostgreSQL rows (14, 16, 18) now set PHPAR_ADAPTER=pgsql so the model-level tests run against each supported PostgreSQL version. The remaining rows keep MySQL as the default, so MySQL coverage is unchanged. Document PHPAR_ADAPTER in CONTRIBUTING.md. Bump actions/checkout from v4 to v7: v4 runs on Node 20, which the hosted runners are retiring. shivammathur/setup-php stays on v2, its current major.
MariaDB reaches the library through the MySQL adapter and is what Debian, Ubuntu and RHEL ship as their MySQL, yet it diverges from MySQL 8 where the adapter is sensitive: no time zone offsets in datetime literals, integer display widths still reported, a looser default sql_mode and a different version string. Three rows cover 10.11 (Debian 12, Ubuntu 24.04, RHEL 9/10), 11.8 (Debian 13) and the current 12.3 LTS. They reuse the MySQL service definition: the image name comes from the matrix, and the health check uses the image's healthcheck.sh because the 12.x image no longer ships mysqladmin. compose.yaml gets the same three servers for local runs.
Mechanical style update, no behaviour change: - array() becomes [] everywhere, in code and in docblock examples, README and the examples directory. Three docblock examples had a missing parenthesis in the original text and were fixed by hand. - list() destructuring becomes [...]. - Function aliases join, sizeof and is_integer become implode, count and is_int. - Static properties that relied on the implicit public get it spelled out. - A commented-out duplicate has_one definition in the Author test model is removed. phpcs.xml enforces the new state: Generic.Arrays.DisallowLongArraySyntax, Generic.PHP.ForbiddenFunctions with the alias table, and the PSR-12 property visibility rule is no longer excluded. The examples directory joins lib and test in the checked paths.
Add a "Coding standard" job that runs vendor/bin/phpcs with the rules from phpcs.xml, so a pull request that breaks the standard fails CI. Trigger the workflow on pushes to master and on pull requests only. With both events unfiltered, every push to a branch with an open pull request started two identical runs. A concurrency group cancels a run that a newer push to the same branch or pull request has superseded.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a GitHub Actions matrix and fixes everything it found. Every job runs the full suite in about a minute.
Matrix: PHP 8.2–8.5, MySQL 5.7/8.0/8.4, MariaDB 10.11/11.8/12.3, PostgreSQL 14/16/18 (with PostgreSQL as the default adapter for the model-level tests), SQLite through each PHP build, memcached, plus a phpcs gate. Database data lives on tmpfs so the suite's tens of thousands of small commits do not wait on fsync.
compose.yamlprovides the same servers locally; see CONTRIBUTING.md.Library fixes uncovered by the matrix
pg_attrdef.adsrc(removed in PG 12) in column introspection; missingPgsqlAdapterimport inTableleft models on PostgreSQL with no column metadata;IS ?with a bound null; inflected attribute names used as quoted column names; non-numeric primary-key lookups raising a type error.setUp(segfault on PHP 8.2) and referenced pre-namespace class names;Memcachedbackend threw a non-existent exception class.setUpleft its adapter as the default and silently skipped 278 unrelated tests.Other changes
requireleft, backend moved tolib/Cache/Memcached.php, missing exception imports added.PHPAR_ADAPTERselects the default adapter for the model-level tests (replaces the dead--adapterswitch).[], no function aliases, explicit property visibility; phpcs.xml enforces it and now coversexamples.actions/checkoutv7; workflow triggers on pushes to master and on pull requests, with superseded runs cancelled.Test plan