test: additional semver coverage and error assertions - #29
Open
greghuels wants to merge 2 commits into
Open
Conversation
Add a waterfall flag whose non-last allocation carries an invalid configured
SemVer comparand, followed by a valid default allocation. Evaluating the
flag must return ERROR/PARSE_ERROR because the whole flag is rejected during
parsing; an implementation that skips the invalid allocation and falls
through to the default would instead return the fallback value with a
STATIC reason.
This directly encodes the scenario from libdatadog PR #2353 review comments
r3775906801 ('we only return default for the last allocation in a
waterfall') and r3775928397 ('invalid configuration should reject the whole
flag during parsing'), which the existing single-allocation
semver-invalid-comparand-test fixture could not distinguish from a
runtime skip-and-fall-through.
Audit of dd-trace-go's openfeature unit tests (semver_test.go, evaluator_test.go, remoteconfig_test.go) found several SEMVER_* behaviors that were only covered by Go/dotnet unit tests but absent from the shared system-test fixtures. Since the shared fixtures are consumed by every FFE SDK (Go, .NET, Java, JS, Python, Ruby, PHP, libdatadog), promote them so all SDKs exercise the same edge cases. New flag semver-precedence-test + test-case-semver-precedence-flag.json (12 cases) isolates precedence rules that are easy to get wrong: - Multi-digit numeric prerelease ordering (beta.2 < beta.11), the canonical lexicographic-trap bug - Build metadata ignored across every operator (EQ/NEQ/LT/LTE/GT/GTE); previously only EQ was covered - Alphanumeric prerelease ordering (alpha < alpha.1 < alpha.beta < beta < release) Each allocation gates on a unique SEMVER_EQ attribute version so test attributes cannot cross-match other allocations; negative build-metadata cases (NEQ/LT/GT) fall through to DEFAULT. Extended test-case-semver-validation-flag.json (+12 cases) covers the remaining SemVer 2.0.0 9 grammar rejections as attributes (empty string, single component, four components, leading-zero minor/patch, empty prerelease/build delimiters, underscore, multiple + delimiters, leading/trailing whitespace) and a non-string attribute (JSON number) returning DEFAULT rather than erroring. New test-case-semver-invalid-comparand-categories.json (6 cases) covers invalid configured comparand categories beyond the overflow case already exercised: invalid syntax (not-a-version), short (1.2), v-prefix (v1.2.3), leading zero (01.2.3), and non-string (JSON number). Each rejects its flag with PARSE_ERROR without poisoning the rest of the configuration. All behaviors are spec-defined and SDK-agnostic. Verified: - ci/validate-fixtures.py passes (298 cases, 44 flags, 40 files) - dd-trace-go TestEvaluateFlag_JSONFixtures: all 59 new semver cases pass - dd-trace-dotnet SemVer.cs/FeatureFlagsEvaluator.cs confirmed to match on non-string attribute (DEFAULT), non-string comparand (PARSE_ERROR), build-metadata-ignored, and numeric prerelease ordering The ufc-config.json change is purely additive (6 new flags); no existing flags were modified.
greghuels
marked this pull request as ready for review
August 14, 2026 09:37
greghuels
requested review from
pavlokhrebto and
sameerank
and removed request for
a team
August 14, 2026 09:37
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.
Promotes SEMVER_* behaviors that were only covered by Go/.NET unit tests into the shared system-test fixtures, so every FFE SDK exercises the same edge cases.
Changes
semver-precedence-testflag +test-case-semver-precedence-flag.json(12 cases): multi-digit numeric prerelease ordering (beta.2<beta.11), build-metadata-ignored across every operator (EQ/NEQ/LT/LTE/GT/GTE), and alphanumeric prerelease ordering (alpha<alpha.1<alpha.beta<beta< release).test-case-semver-validation-flag.json(+12 cases): remaining SemVer §9 grammar rejections as attributes (empty, single/four components, leading-zero minor/patch, empty prerelease/build delimiters, underscore, multiple+, whitespace) and a non-string attribute (JSON number) →DEFAULT.test-case-semver-invalid-comparand-categories.json(6 cases): invalid comparand categories beyond overflow (invalid syntax, short, v-prefix, leading zero, non-string) →PARSE_ERROR, with a no-poisoning sanity case.Verification
ci/validate-fixtures.pypasses (298 cases, 44 flags, 40 files)dd-trace-goTestEvaluateFlag_JSONFixtures: all 59 new semver cases passdd-trace-dotnetSemVer.cs/FeatureFlagsEvaluator.csconfirmed to match on non-string attribute (DEFAULT), non-string comparand (PARSE_ERROR), build-metadata-ignored, and numeric prerelease orderingufc-config.jsonis purely additive (6 new flags); no existing flags modified.