Repository navigation
FFL-3568: Run shared precomputed response fixtures - #1534
leoromanovsky wants to merge 8 commits into
Conversation
| Do not add local exclusions to hide failures. | ||
|
|
||
| Numeric getters use lossless conversion regardless of the `integer`, `float`, or `number` wire label. | ||
| Integer getters accept whole values within their native range. Double getters reject integer values that lose precision. |
There was a problem hiding this comment.
🤖 Comment from Claude, working with Aaron:
[P2] The Number getter returns rounded values.
This line says Double getters reject integer values that lose precision. The offline path returns rounded values with TARGETING_MATCH.
A local probe used configurationFromString, setConfiguration, and getNumberDetails. All three numeric labels returned the same results:
9007199254740993→90071992547409929223372036854775807→92233720368547760009007199254740993.5→9007199254740994
This behavior predates this PR. JSON.parse rounds the input before decoding. The shared contract requires the default and TYPE_MISMATCH for inexact conversions.
The three numeric-boundaries-* fixtures exclude React Native at the case level. Their evaluation filters permit 27 Number reads. A local probe bypassed the case filters. Eighteen results matched; nine differed because the getter accepted rounded values.
Could we detect precision loss before parsing, or document a React Native exception in the shared contract and correct this section? Please also narrow the fixture filters so the other 18 reads run.
| }; | ||
| expect(actual.value).toEqual(expectedResult.value); | ||
| expect(actual).toMatchObject(expectedResult); | ||
| if (!details.errorCode) { |
There was a problem hiding this comment.
🤖 Comment from Claude, working with Aaron:
[P2] The runner does not check the fixture event expectations.
The runner checks one trackEvaluation call per successful read and none for errors. It does not read expectations or noUnmatchedEvents.
For React Native, type-mismatch-all-types expects five evaluation events from five reads. Four reads return errors. empty-flags-response expects three events, and all three reads return errors. Only the second fixture excludes web from that event matcher.
FlagsClient returns these errors before calling native tracking. This suite therefore does not verify the expected error events.
Should React Native track error evaluations, or should the shared matchers define a React Native exception? Could we then add tests for the applicable event counts and noUnmatchedEvents? Bridge-call counts alone do not verify emitted events.
| return; | ||
| } | ||
| const decoded = decodePrecomputedFlags(fixture.response); | ||
| jest.spyOn( |
There was a problem hiding this comment.
🤖 Comment from Claude, working with Aaron:
[P2] The shared suite does not test native response decoding.
This mock returns the JavaScript decoder output from setEvaluationContext. The online path instead uses native SDKs and bridge conversion. The existing dependency pins are iOS 3.16.1 and Android 3.15.0.
Two differences need coverage:
- The Android bridge uses
variationValue.toIntOrNull()for"integer"values. Text such as"5.0"or"2147483648"fails that conversion. The bridge then returns the text as the flag value. - iOS 3.16.1 uses
decode(Int.self)for"integer"values. A local probe with those models rejected the complete response when one flag contained1.25.
Could the README say that this suite tests the JavaScript decoder used by the offline path, plus getters and tracking arguments? Please add native response and bridge tests in a follow-up. The Android conversion also needs review when updating native dependencies.
|
🤖 Comment from Claude, working with Aaron: A few small points (P3):
|
What does this PR do?
Run shared precomputed responses through the JavaScript decoder and public Number API. Numeric labels no longer reject otherwise valid numbers. Reject unsupported encoding metadata and preserve safe numeric serial IDs in the bridge payload.
Motivation
Mobile SDKs need consistent response handling and automatic coverage when canonical JSON cases are added. Merge the canonical fixture contract first. Ref: FFL-3568.
Additional Notes
Use a pinned snapshot of 57 shared JSON fixtures. A generic runner discovers new cases automatically. Weekly automation opens draft snapshot updates; integrity checks and README instructions explain maintenance. No Git submodule is introduced. Upstream CI enforces generated plain/obfuscated fixture parity.
Coverage includes public results, decoding failures, and native tracking-call arguments. Native wire-event serialization is not validated. The pinned iOS SDK lacks serial-ID support; that native path needs a dependency update. Positive obfuscation and native Integer precision cases are explicitly excluded because this API does not support them.
Bundle size: The package file list excludes all Flags test sources and fixtures; the builder also excludes test directories from compiled output. The package dry run confirms that neither the shared JSON corpus nor its runner is shipped. The additional test cases therefore add no fixture payload to customer app bundles. Runtime decoder and tracking fixes also change SDK code; the total binary-size difference was not measured.
Review checklist (to be filled by reviewers)