[MAINTENANCE] Consult the backend declaration when constructing harness tables - #12028
Draft
joshua-stauffer wants to merge 1 commit into
Draft
Conversation
Binds SQLBatchTestSetup's generic parameter to the SQL config base, so every concrete setup is statically tied to a config carrying a declared SqlBackendSpec and can read it off self.config without a cast. Exposes that declaration through a backend_spec property. Adds the dialect table-schema-item hook at the single table-construction site. A backend that needs dialect-required schema items declares a zero-argument factory; the harness calls it once per table and passes the returned items positionally after the generated columns, contributing nothing when no factory is declared. The factory shape is load-bearing rather than incidental. Such constructs are not accepted as Table keyword arguments, so they must arrive positionally; and they bind to the first table they are attached to, so reusing one instance across tables corrupts every table after the first. Calling a factory once per table is what makes each table's items its own. A test pins exactly that: a counting factory must be invoked once per created table and the items reachable from two tables must not be the same object - a design storing one instance would still put an item on both tables and pass a weaker check. Also pins the session-scoped batch-setup cache with a counting oracle. Two tests over the same declaration and data must share one setup; a regression there costs session time rather than turning a test red, so only a counter catches it. The second test asserts it sees the very object the first recorded, which is what stops it passing vacuously when run alone.
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.
Stacked on #12023 — review #12021, #12022 and #12023 first. This PR shows only its own commit;
GitHub retargets it as its parents merge.
First change in this series that touches the shared SQL setup rather than adding inert
declarations, so the existing backend suites become the real oracle from here.
Two bindings and one hook:
SQLBatchTestSetup's generic parameter moves from_ConfigTto the SQL config base, so everyconcrete setup is statically tied to a config that carries a declared
SqlBackendSpecand canread it off
self.configwithout a cast. Abackend_specproperty exposes it.passed positionally after the generated columns. Nothing is contributed when no factory is
declared, so behavior is unchanged for all nine existing configs.
Why a factory rather than a stored value
Two independent reasons, and the tests pin both. A dialect storage-engine construct is not
accepted as a
Tablekeyword argument, so it must arrive positionally. And it binds to the firsttable it is attached to, so reusing one instance across tables corrupts every table after the
first — each table needs its own.
That second property is what the call-count test exists for. A design that built the items once
and reused them would still put an item on both tables and pass a "does the item appear" check;
it fails here, because the factory must be invoked once per created table and the items reachable
from two tables must not be the same object.
_create_tablewas confirmed to still be the onlyTable(...)construction in the module and_create_table_dataits sole caller — invoked once for the primary table and once per extratable — so one edit covers both. No subclass overrides it.
Session-cache regression coverage
Equivalent batch setups are meant to be created and torn down once per session, not once per
test. A regression there costs session time rather than turning a test red, so it needs a
counting oracle rather than "the fixtures were not edited".
Two tests over the same declaration and the same data must share one setup. The second asserts it
sees the very object the first recorded — not merely that the setup count is still one, which
would hold either way if the test ran alone. Run in isolation it now fails outright, which is the
evidence the assertion binds.
Verification
SQLite 544 passed (539 before, +5 exactly the new tests); PostgreSQL 472 and MySQL 160 both
unchanged; collection 3332 → 3337. Registry suite unaffected at 39.
Every new assertion was mutation-tested rather than accepted on inspection. Removing the hook
turns both table-item tests red; memoizing the factory result leaves the first passing and fails
only the call-count and identity checks; forcing a cache miss fails the cache test; recording a
decoy object fails the identity assert on its own. All mutations reverted.
ruffclean.mypyadds no diagnostics over an untouched-file baseline, and a probe confirmedthe new generic bound is enforced: subscripting
SQLBatchTestSetupwith a config outside thebound is a hard
type-varerror.