Add property-based round-trip spec and concurrency spec#143
Merged
Conversation
7906b43 to
5398898
Compare
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
Implements #126 (B3 — property-based
ValueRoundTripSpec) and #127 (B4 — concurrency spec) from the 1.0-readiness milestone. Independent of the other open PRs — branches offmain.B3 —
ValueRoundTripSpecProperty-based coverage in
core/src/test/scala/zio/openfeature/ValueRoundTripSpec.scala, 10 properties at 200 samples each (viaTestAspect.samples(200)).The interesting finding: the cross-boundary conversion is intentionally lossy because the OpenFeature Java SDK normalises every numeric to a single
Doubletype, soIntValueandLongValueare mediated throughDoubleand may come back as a different concreteAttributeValuecase. The properties pin the actual contract callers depend on rather than asserting a (false) exactness:BoolValue, non-empty unicodeStringValue, andIntValueround-trip exactly.LongValueround-trips asLongValueonly when the value is large enough to be distinct fromIntAND ≤2^53 - 1(Double mantissa limit). Smaller values come back asIntValue.DoubleValueround-trips for finite, fractional doubles; whole-number doubles convert toIntValue/LongValueon the way back.DoubleValue(Double.NaN)survives as a NaN double (not equal-by-==, butisNaNholds).LongValue(Long.MaxValue)lands asDoubleValue— pinned via a dedicated test that documents the converter'sLong.MaxValue.toDoublesaturation-avoidance.StructValue/ListValuepreserve leaf values via their typed accessors.EvaluationContext.targetingKey+ mixed attributes round-trips cleanly.If a future converter change breaks any of these, the property test surfaces a counterexample.
B4 —
ConcurrentEvaluationSpec200 fibers × 10 evaluations each (= 2000 concurrent ops) against
TestFeatureProvider, with a background fiber drivingProviderStatus.Ready → Error → Readymid-burst. Lives intestkit/src/test/scala/zio/openfeature/testkit/rather thancore/(the issue suggestedcore) — usingTestFeatureProvider.setStatusis the cleanest API for driving the transition andcoredoesn't depend ontestkit. The behaviour under test is core's evaluation path, exercised the same way an app would.Assertions:
FiberCount × EvalsPerFiber(no leaked / dropped evaluations).Unexpectedoutcomes (anything other thanProviderNotReadyis treated as a concurrency bug).providerStatus == Ready.TestAspect.timeout(20.seconds)so a deadlock surfaces as a test failure.notReadycount is logged for visibility but not asserted — the burst can race the status driver on a fast machine and complete before the ERROR window opens, which would make the test flaky on Scala 2.13's marginally faster scheduling. Deterministic failure-path coverage already lives inProviderInitFailureSpec(B2, #142).Closes #126
Closes #127