Fix false positives and modernise the 3.x test suite#8
Merged
Conversation
Ported from MasoniteFramework/orm#967, reconciled with the merged connection-caching, driver-options and increments-primary changes. - Fix tests that had no assertions (technical false-positive passes) and enable previously commented-out tests. - Add scripts/check_test_asserts.py plus a CI step ensuring every test contains at least one assertion. - Remove the tracked orm.sqlite3 fixture: a session conftest now builds the SQLite database from migrations and seeds under tests/integrations/ (moved there from the root databases/ folder, which is removed). - Move grammar query strings into the tests for readability. - CI: split migrate/test steps, point migrations at tests/integrations/migrations, modern action versions, lint via make check before the matrix runs. - Remove unused files (app observer fixture, legacy test-database config). Conflict resolutions during the port: kept the post-#7 platform SQL expectations (inline increments primary keys, no implicit UNSIGNED on SQLite) and the .primary() calls in the relocated migrations; restored Python 3.8 to the CI matrix since the package still supports it.
PRAGMA table_info returns bare storage types, and the increments family shares those storage types with the integer family (INTEGER, TINYINT, BIGINT). Reversing the full type map let the increments variants win, so any table with an INTEGER column raised 'increments() not supported on non-primary key columns' when an ALTER rebuilt it. Exclude the increments family from the reversal: rebuilt columns are plain integer types and the primary key is preserved separately.
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
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.
What this does
scripts/check_test_asserts.pyand a CI step that fails if any test lacks an assertion, so false positives can't reappear.orm.sqlite3fixture — a sessionconftest.pynow builds the SQLite test database from migrations and seeds living undertests/integrations/(moved there from the rootdatabases/folder, which is gone).*.sqlite3is ignored, so test runs no longer dirty the working tree.tests/integrations/migrations, modern action versions, concurrency cancellation.config/test-database.py).Reconciliation notes (this port vs the original branch)
The original PR was written against pre-#7
3.x; the port keeps the now-merged behavior wherever they collided:*_increments_primarySQL (e.g.[id] INT IDENTITY NOT NULL PRIMARY KEY) instead of the old table-level primary key constraint expectations.UNSIGNEDon SQLite morphs/foreign columns..primary()kept on the relocated migrations'increments()columns.Bug found and fixed during the port
The new
test_alter_drop_on_table_schema_table(SQLite) exposed a reverse-type-map collision:PRAGMA table_inforeturns bare storage types, and reversing the full type map letincrements/tiny_increments/big_incrementsshadowinteger/tiny_integer/big_integer(same storage types). Any ALTER that rebuilt a table with an INTEGER column raisedQueryException: increments() not supported on non-primary key columns. The increments family is now excluded from the reversal (separate commit).Tests
Full suite: 1058 passed, 0 failed — including the Postgres alter test that previously required a live server (now exercised through the rewritten suite), and the assert-checker reports every test asserts.