Add Layer-1 thread-safety invariant specs#2820
Open
ericproulx wants to merge 1 commit into
Open
Conversation
Two deterministic tests that turn the validation layer's concurrency invariants into permanent regression guards — no timing or thread interleaving involved, so no flakiness: - Frozen-graph canary: walks every object reachable from the compiled routes' validator lists (validators, iterators, scopes, coercers — including oneof variant validators reached through nesting) and asserts each contract-class instance is frozen. Catches classes that never adopted the FreezeOnNew contract, not just regressions in those that did. A census assertion proves the walk actually swept the whole taxonomy, and a self-test proves the walker reports unfrozen objects. - Cache growth canary: snapshots every Grape::Util::Cache key set after definition + compile!, then fires first-ever requests (valid, invalid, wrong-shape, 404, OPTIONS, 405) and asserts no cache gained keys. Pins the "caches are written only at boot" property that makes their unsynchronized reads safe; would have caught the pre-#2817 ArrayCoercer seeding DryTypes::ParamsCache on first request. Test-only; no production changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Danger ReportNo issues found. |
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.
Summary
Test-only PR adding the two deterministic invariant tests from the thread-safety plan ("Layer 1") — the follow-up to #2819. Instead of trying to observe races with threads (which the GVL hides on MRI), these make the invariants that prevent races fail loudly and deterministically when violated. Zero timing sensitivity, zero flakiness.
1. Frozen-graph canary (
spec/grape/validations/frozen_validation_graph_spec.rb)Builds one API touching the full taxonomy — every stock validator, primitive/array/set/nested-array/multiple/variant/custom coercers,
oneofvariants, nested and lateral (given) scopes — then walks every object reachable from the compiled routes' validator lists and asserts each instance of a contract class (validators, iterators,ParamsScope, all coercer hierarchies) is frozen.Design notes:
Dry::internals (external library, outside grape's contract). Cycle-safe via an identity-keyed seen set.FreezeOnNewfails this spec, which is stronger than re-testing the classes that already opted in.oneofvariant validators reached through nesting andMultipleAttributesIterator) — so the spec can't pass vacuously by walking nothing. The prototype run visits ~276 objects.2. Cache growth canary (in
spec/grape/util/cache_spec.rb)Snapshots every
Grape::Util::Cachesubclass's key set after definition +compile!, then fires first-ever requests — valid, invalid, wrong-shape payloads, unmatched path (404),OPTIONS, and un-routedPATCH(405) — and asserts no cache gained a key.This pins the property that makes the caches' unsynchronized reads safe: they are written only at boot. #2819's Monitor makes concurrent writes safe; this spec ensures request-time writes don't (re)appear silently. It would have caught the pre-#2817
ArrayCoercerlazily seedingDryTypes::ParamsCacheon the first request. Discovered viaClass#subclasses, so future caches are covered automatically.Results
Both canaries pass on first run against the current stack — independent confirmation that #2816–#2819 left no request-time mutation in the validation layer, including on error/404/405 paths.
Full suite: 2388 examples, 0 failures. RuboCop clean. No CHANGELOG entry (test-only, matching #2818's precedent).
🤖 Generated with Claude Code