[MAINTENANCE] Add the per-case tier exclusion accessor - #12034
Draft
joshua-stauffer wants to merge 1 commit into
Draft
[MAINTENANCE] Add the per-case tier exclusion accessor#12034joshua-stauffer wants to merge 1 commit into
joshua-stauffer wants to merge 1 commit into
Conversation
A backend joins a tier as a whole. Occasionally one case within that tier's suite is not meaningful for a dialect, and the choices without a mechanism are both bad: withdraw the backend from the tier entirely, losing every other case, or bury a backend-specific skip inside a shared case, where nobody can see it. The declaration already carries a per-case exclusion mapping, with a required reason and a ceiling enforced at registration. This adds the one accessor that makes it take effect: a tier's members, in label order, minus any declaring an exclusion for the named case. It takes a tier rather than being fixed to one. Per-case exclusion is a property of tiers generally, and a version hard-coded to a single tier would have to be unpicked the first time a second tier needed it. A test pins that by giving two tiers one member each and asserting the accessor answers for whichever tier it was asked about - without it, an implementation that ignored its argument passed every other test here, since they all ask about the same tier. The behavior-preservation assertion deliberately compares the accessor against a live read of tier membership rather than against the module-level list. Those answer different questions inside this module: the list is built once at import, while the registry is cleared around every test here, so comparing the two would pass today only because both happen to be empty, and would fail for an unrelated reason the moment a backend joins the tier. The list-referencing form of the same oracle belongs where the real registry is live.
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 #12033 — review the earlier PRs in this series first. This PR shows only its own commit;
GitHub retargets it as its parents merge.
A backend joins a tier as a whole. Occasionally one case within that tier's suite is not meaningful
for a dialect, and without a mechanism both available choices are bad: withdraw the backend from the
tier entirely and lose every other case, or bury a backend-specific skip inside a shared case where
nobody can see it.
The declaration already carries a per-case exclusion mapping, with a required reason and a ceiling
enforced at registration. This adds the one accessor that makes it take effect: a tier's members, in
label order, minus any declaring an exclusion for the named case. Nothing else filters tier
membership by case, so "did this exclusion take effect" stays a one-module question.
Tier-general on purpose, and pinned
The accessor takes a tier rather than being fixed to one — per-case exclusion is a property of tiers
generally, and a version hard-coded to a single tier would have to be unpicked the first time a
second tier needed it.
That is pinned by a test giving two tiers one member each and asserting the accessor answers for
whichever tier it was asked about. Worth stating why it exists: without it, an implementation that
ignored its
tierargument entirely passed every other test here, because they all happen to askabout the same tier.
Why the oracle compares against a live read, not the module-level list
The behavior-preservation assertion compares the accessor against a live read of tier membership
rather than against the module-level curated list.
Those answer different questions in this module. The list is built once, at import, from whatever
was registered then; the registry is cleared around every test here. Comparing the two would pass
today only because both happen to be empty, and would fail for a reason unrelated to exclusions the
moment a backend joins that tier. Verified by simulating exactly that: with a backend added to the
curated tier, the literal form evaluates false while the call-time form still holds.
The list-referencing form of the same oracle belongs where the real registry is live, and lands with
the suite that publishes the case keys.
Verification
Both tests are mutation-killable: dropping the filter, inverting it, reversing the order, returning
classes instead of instances, filtering on a fixed key, and hard-coding either tier each turn at
least one red. The throwaway backends are registered non-alphabetically, so the ordering assertion
distinguishes label order from insertion order rather than passing either way.
The accessor is pure — repeated calls leave the registry and every declaration untouched and return
a fresh list — and returns sensibly for an empty tier, an unknown case key, and an empty exclusion
mapping.
-m project50 → 53. SQLite 556, PostgreSQL 472, MySQL 160, SQL Server 181 and generic-SQL 138 allexactly at baseline;
tests/integrationandtests/metricscollect unchanged.ruffclean.mypyadds no diagnostics over an untouched-file baseline.