Skip to content

feat: Add flag dependency test cases - #59

Draft
khvn26 wants to merge 6 commits into
mainfrom
test/flag-dependencies
Draft

feat: Add flag dependency test cases#59
khvn26 wants to merge 6 commits into
mainfrom
test/flag-dependencies

Conversation

@khvn26

@khvn26 khvn26 commented Aug 27, 2026

Copy link
Copy Markdown
Member

Corpus coverage for dependent flags (Flagsmith/flagsmith#8394, schema in Flagsmith/flagsmith#8396).

A dependency is expressed as an ordinary segment condition on the result of another flag, using a $.flags.<feature name> property:

"rules": [{ "type": "ALL", "conditions": [
  { "operator": "EQUAL", "property": "$.flags.prerequisite.enabled", "value": "true" }
]}],
"overrides": [{ "name": "dependent", "enabled": true, "value": "on", "key": "2" }]

Segments could previously all be evaluated before any flag, whereas a segment conditioned on $.flags.prerequisite.enabled needs that flag resolved first.

These 18 cases pin the parts of that where implementations can plausibly diverge.

The dependency itself

Case Guards against
prerequisite_enabled__should_override evaluating all segments before any flag — the baseline dependency
prerequisite_disabled__should_not_override the negative, including the segment not being reported
absent_prerequisite__should_not_override treating a dependency on a feature absent from the context as an error rather than as no value

Fields other than enabled

Case Guards against
value__should_override assuming dependencies can only be conditioned on enabled
variant__should_override the same for a multivariate selection

Chains and cycles

Case Guards against
transitive__should_cascade resolving flags in context order; c → b → a is declared c, b, a, the reverse of resolution order
transitive_unmet__should_not_cascade applying an override on a merely present dependency rather than a satisfied one
cyclic__should_not_override a naive resolver recursing indefinitely

How the property is spelled

Case Guards against
bracketed_field__should_override $.flags.a['enabled'] going unrecognised, as it does when the property is matched as text rather than parsed
quoted_feature_name__should_override feature names that aren't bare JSONPath identifiers, e.g. containing a space
bracket_rooted_property__should_not_override honouring $['flags']['a'], which is a trait key: only a $.-prefixed property is a query
wildcard_property__should_not_override resolving flags for a query that may select several nodes, which conditions don't support

Rule and override structure

Case Guards against
nested_rule__should_override finding dependencies only in top-level conditions, not in nested rule groups
multiple_overrides__each_feature_gets_its_own taking a segment's first override rather than the one naming the feature being resolved
no_overrides__segment_still_reported resolving dependencies only for segments that carry overrides
competing_overrides__lowest_priority_wins letting resolution order disturb override precedence
segment_metadata__reported_with_dependency dropping a segment's metadata on the way to reporting membership

Reading $.flags from the context

Case Guards against
context_supplied_flags__should_be_ignored trusting a client-supplied $.flags

Some of these deserve a note:

cyclic__should_not_override covers input that should be unreachable, since cycles are meant to be rejected where dependencies are written; we need to make sure we won't get stack overflows in case of a bug, though!

context_supplied_flags__should_be_ignored: $.flags is readOnly in the context schema because the engine populates it during evaluation. The engine should expect no pre-populated flags in the context.

How did you test this code?

khvn26 added 3 commits August 27, 2026 17:13
Covers segments conditioned on another flag's result via a
`$.flags.<feature name>` condition property, as used by dependent flags:

- a dependency satisfied, and the same dependency unsatisfied
- a dependency on a flag's value rather than on whether it is enabled
- a transitive chain, declared in reverse dependency order so that
  resolving flags in context order is not enough to pass
- a cycle, which must terminate rather than recurse. Both flags resolve
  to their defaults whichever is resolved first, so the expectation does
  not depend on iteration order
- a context arriving with `$.flags` already populated, which must be
  discarded rather than allowed to satisfy its own dependency
REVERT BEFORE MERGE.

`EvaluationContext.flags` only exists on the schema branch of
Flagsmith/flagsmith#8396, so validating the new test cases against
`refs/heads/main` silently proves nothing: the key is simply unknown to
the schema, and `EvaluationContext` doesn't set `additionalProperties`
to false, so anything at all passes.

Points `schema.json` at the context schema on that branch, and the new
test cases at `schema.json` on this one, so that validation is
meaningful while both are in review. With this, `check-jsonschema`
rejects e.g. a non-boolean `$.context.flags.<name>.enabled`, which it
accepted before.

Once #8396 is merged, both refs should go back to `main` (and the test
cases to a tag, in line with the rest of the corpus).
Extends the dependent flags coverage with cases that were previously
carried as unit tests in flagsmith-engine, and so proved nothing about
any other implementation:

- `$.flags.a['enabled']` and `$.flags['my feature'].enabled`, two
  spellings an implementation matching the property as text rather than
  parsing it is liable to miss
- `$['flags']['a'].enabled`, which is deliberately *not* a dependency:
  only a `$.`-prefixed property is a JSONPath query, and a
  bracket-rooted one is a trait key
- a dependency in a nested rule group rather than a top-level condition
- a dependency on `variant` rather than `enabled`
- a transitive chain whose root is disabled, the counterpart to the
  cascading case
- a dependency on a feature absent from the context
- competing overrides, to pin that resolving dependencies doesn't
  disturb override precedence
- a segment conditioned on a flag but overriding nothing, which still
  has to be evaluated for its membership to be reported

Expected results were generated by running the engine, as in #57.
khvn26 added a commit to Flagsmith/flagsmith-engine that referenced this pull request Aug 27, 2026
Adds support for segments conditioned on another flag's result, via a
`$.flags.<feature name>` condition property, as the evaluation half of
dependent flags.

Segment conditions are already JSONPath, so a dependency needs no new
operator or condition type. What it does need is a change to the shape
of evaluation: segments were evaluated in full before any flag, but a
segment conditioned on `$.flags.checkout_v2.enabled` needs that flag
resolved first. Flags and segment membership are therefore resolved
lazily and memoised, with resolved flags accumulated into a single dict
the context refers to, so no context copying is required.

Which features a segment depends on is established by inspecting the
compiled JSONPath query's segments, rather than by matching the
property as text. Every spelling of a query normalises to the same
selectors before we look at it, so there is no grammar to reimplement
and no divergence to keep in sync across engines. Only a query naming a
single flag is a dependency: conditions resolve to a scalar, so a
wildcard, index, slice or descendant search is unsupported to begin
with, and is left undetected rather than resolving every flag on the
off-chance.

Contexts whose segments have no flag conditions keep the existing
single-pass evaluation, so results are unchanged unless dependencies
are in use. They do, however, pay for the scan that establishes there
are no dependencies: 10-19% of evaluation across the benchmark
contexts, and 11-15% for segment-heavy ones. Precomputing this where
segments are written, and carrying it on the context, would remove it;
`get_segment_dependencies` is public so that a caller holding a
long-lived environment document can at least scan once rather than per
evaluation.

A flag caught in a dependency cycle is left unresolved rather than
raising, so a cycle reaching the engine degrades to a non-matching
condition instead of breaking evaluation for the whole context. Cycles
are expected to be rejected where dependencies are written.

Behaviour is covered by test cases in Flagsmith/engine-test-data#59
rather than by unit tests here, so that every engine is held to it.
…wildcards

Three further cases, each covering a path the existing ones leave
untested in a reference implementation:

- a dependency segment carrying metadata, which has to survive
  resolution and be reported as any segment's metadata is
- one segment carrying overrides for two features, so that resolving one
  feature selects the override naming it rather than the segment's first
- a wildcard query over the flags mapping, which is unsupported because
  conditions resolve to a scalar. Every flag in that case is disabled,
  so the condition fails whichever node the query happens to select,
  and the expectation holds whether or not an implementation treats
  such a query as a dependency. What it resolves to when flags differ
  is deliberately left unpinned.
khvn26 added a commit to Flagsmith/flagsmith-engine that referenced this pull request Aug 27, 2026
REVERT BEFORE MERGE.

Dependent flags behaviour is covered by test cases in
Flagsmith/engine-test-data#59 rather than by unit tests here, so that
every engine is held to it. Those cases aren't in a release yet, so
without this the new code is exercised by nothing and CI's 100%
coverage gate fails at 78%.

Once #59 is merged and tagged, this goes back to a semver tag, which
Renovate now tracks as of #337.
One segment gating two features on the same flag, with two further
segments each depending on one of those features. A diamond rather than
a chain, so the shared segment is reached twice while resolving, once
per feature depending on it.

Guards two things a chain doesn't reach: reusing a segment's verdict for
the second feature, and selecting the override naming the feature being
resolved rather than the segment's first override.
khvn26 added a commit to Flagsmith/flagsmith-engine that referenced this pull request Aug 27, 2026
Adds support for segments conditioned on another flag's result, via a
`$.flags.<feature name>` condition property, as the evaluation half of
dependent flags.

Segment conditions are already JSONPath, so a dependency needs no new
operator or condition type. What it does need is for the flag to be
resolved by the time the condition reads it, and evaluation resolved
every segment before any flag.

Rather than establish up front which segments depend on which flags,
`$.flags` is a mapping that resolves a flag when a condition first
reads it. Resolving a flag evaluates the segments overriding it, which
in turn resolves whatever flags their conditions read, memoised
throughout. The existing single pass is otherwise untouched: a context
whose segments read no flag never enters the resolver, and pays only
for one empty mapping.

Resolving on read rather than in advance means there is no need to
recognise a dependency in a condition property, and so no need to
reimplement enough of the JSONPath grammar to tell that
`$.flags.a['enabled']` and `$.flags.a.enabled` are the same query.
Every spelling works because resolution is triggered by the read
itself. It also costs only what is read: rule short-circuiting means a
condition that is never evaluated resolves nothing.

Measured against main across the benchmark contexts, interleaved to
cancel drift: +3.4%, from the mapping and a shallow copy of the context
to hold it. Establishing up front that an environment has no
dependencies would remove that residual entirely, but needs a field on
the context to carry it.

A flag caught in a dependency cycle is left unresolved rather than
raising, so a cycle reaching the engine degrades to a non-matching
condition instead of breaking evaluation for the whole context. Cycles
are expected to be rejected where dependencies are written.

Behaviour is covered by test cases in Flagsmith/engine-test-data#59
rather than by unit tests here, so that every engine is held to it.
khvn26 added a commit to Flagsmith/flagsmith-engine that referenced this pull request Aug 27, 2026
Adds support for segments conditioned on another flag's result, via a
`$.flags.<feature name>` condition property, as the evaluation half of
dependent flags.

Segment conditions are already JSONPath, so a dependency needs no new
operator or condition type. What it does need is for the flag to be
resolved by the time the condition reads it, and evaluation resolved
every segment before any flag.

Rather than establish up front which segments depend on which flags,
`$.flags` is a mapping that resolves a flag when a condition first
reads it. Resolving a flag evaluates the segments overriding it, which
in turn resolves whatever flags their conditions read, memoised
throughout. The existing single pass is otherwise untouched: a context
whose segments read no flag never enters the resolver, and pays only
for one empty mapping.

Resolving on read rather than in advance means there is no need to
recognise a dependency in a condition property, and so no need to
reimplement enough of the JSONPath grammar to tell that
`$.flags.a['enabled']` and `$.flags.a.enabled` are the same query.
Every spelling works because resolution is triggered by the read
itself. It also costs only what is read: rule short-circuiting means a
condition that is never evaluated resolves nothing.

Measured against main across the benchmark contexts, interleaved to
cancel drift: +3.4%, from the mapping and a shallow copy of the context
to hold it. Establishing up front that an environment has no
dependencies would remove that residual entirely, but needs a field on
the context to carry it.

A flag caught in a dependency cycle is left unresolved rather than
raising, so a cycle reaching the engine degrades to a non-matching
condition instead of breaking evaluation for the whole context. Cycles
are expected to be rejected where dependencies are written.

Behaviour is covered by test cases in Flagsmith/engine-test-data#59
rather than by unit tests here, so that every engine is held to it.
khvn26 added a commit to Flagsmith/flagsmith-engine that referenced this pull request Aug 27, 2026
REVERT BEFORE MERGE.

Dependent flags behaviour is covered by test cases in
Flagsmith/engine-test-data#59 rather than by unit tests here, so that
every engine is held to it. Those cases aren't in a release yet, so
without this the new code is exercised by nothing and CI's 100%
coverage gate fails.

Once #59 is merged and tagged, this goes back to a semver tag, which
Renovate now tracks as of #337.
khvn26 added a commit to Flagsmith/flagsmith-engine that referenced this pull request Aug 27, 2026
Adds support for segments conditioned on another flag's result, via a
`$.flags.<feature name>` condition property, as the evaluation half of
dependent flags.

Segment conditions are already JSONPath, so a dependency needs no new
operator or condition type. What it does need is for the flag to be
resolved by the time the condition reads it, and evaluation resolved
every segment before any flag.

Rather than establish up front which segments depend on which flags,
`$.flags` is a mapping that resolves a flag when a condition first
reads it. Resolving a flag evaluates the segments overriding it, which
in turn resolves whatever flags their conditions read, memoised
throughout. The existing single pass is otherwise untouched: a context
whose segments read no flag never enters the resolver, and pays only
for one empty mapping.

Resolving on read rather than in advance means there is no need to
recognise a dependency in a condition property, and so no need to
reimplement enough of the JSONPath grammar to tell that
`$.flags.a['enabled']` and `$.flags.a.enabled` are the same query.
Every spelling works because resolution is triggered by the read
itself. It also costs only what is read: rule short-circuiting means a
condition that is never evaluated resolves nothing.

Measured against main across the benchmark contexts, interleaved to
cancel drift: +3.4%, from the mapping and a shallow copy of the context
to hold it. Establishing up front that an environment has no
dependencies would remove that residual entirely, but needs a field on
the context to carry it.

A flag caught in a dependency cycle is left unresolved rather than
raising, so a cycle reaching the engine degrades to a non-matching
condition instead of breaking evaluation for the whole context. Cycles
are expected to be rejected where dependencies are written.

Behaviour is covered by test cases in Flagsmith/engine-test-data#59
rather than by unit tests here, so that every engine is held to it.
khvn26 added a commit to Flagsmith/flagsmith-engine that referenced this pull request Aug 27, 2026
REVERT BEFORE MERGE.

Dependent flags behaviour is covered by test cases in
Flagsmith/engine-test-data#59 rather than by unit tests here, so that
every engine is held to it. Those cases aren't in a release yet, so
without this the new code is exercised by nothing and CI's 100%
coverage gate fails.

Once #59 is merged and tagged, this goes back to a semver tag, which
Renovate now tracks as of #337.
khvn26 added a commit to Flagsmith/flagsmith-engine that referenced this pull request Aug 27, 2026
Adds support for segments conditioned on another flag's result, via a
`$.flags.<feature name>` condition property, as the evaluation half of
dependent flags.

Segment conditions are already JSONPath, so a dependency needs no new
operator or condition type. What it does need is for the flag to be
resolved by the time the condition reads it, and evaluation resolved
every segment before any flag.

Rather than establish up front which segments depend on which flags,
`$.flags` is a mapping that resolves a flag when a condition first
reads it. Resolving a flag evaluates the segments overriding it, which
in turn resolves whatever flags their conditions read, memoised
throughout. The existing single pass is otherwise untouched: a context
whose segments read no flag never enters the resolver, and pays only
for one empty mapping.

Resolving on read rather than in advance means there is no need to
recognise a dependency in a condition property, and so no need to
reimplement enough of the JSONPath grammar to tell that
`$.flags.a['enabled']` and `$.flags.a.enabled` are the same query.
Every spelling works because resolution is triggered by the read
itself. It also costs only what is read: rule short-circuiting means a
condition that is never evaluated resolves nothing.

Measured against main across the benchmark contexts, interleaved to
cancel drift: +3.4%, from the mapping and a shallow copy of the context
to hold it. Establishing up front that an environment has no
dependencies would remove that residual entirely, but needs a field on
the context to carry it.

A flag caught in a dependency cycle is left unresolved rather than
raising, so a cycle reaching the engine degrades to a non-matching
condition instead of breaking evaluation for the whole context. Cycles
are expected to be rejected where dependencies are written.

Behaviour is covered by test cases in Flagsmith/engine-test-data#59
rather than by unit tests here, so that every engine is held to it.
khvn26 added a commit to Flagsmith/flagsmith-engine that referenced this pull request Aug 27, 2026
REVERT BEFORE MERGE.

Dependent flags behaviour is covered by test cases in
Flagsmith/engine-test-data#59 rather than by unit tests here, so that
every engine is held to it. Those cases aren't in a release yet, so
without this the new code is exercised by nothing and CI's 100%
coverage gate fails.

Once #59 is merged and tagged, this goes back to a semver tag, which
Renovate now tracks as of #337.
A flag whose dependencies form a cycle serves its environment default,
which was previously indistinguishable from a flag that was never gated
at all. Report `ERROR; code=CIRCULAR_DEPENDENCY` instead, so the
condition that could not be evaluated is visible to whoever is looking
at the result.

Only flags in the cycle are reported this way. A flag merely depending
on one resolves normally against whatever the cycle settled on.
khvn26 added a commit to Flagsmith/flagsmith-engine that referenced this pull request Aug 28, 2026
Adds support for segments conditioned on another flag's result, via a
`$.flags.<feature name>` condition property, as the evaluation half of
dependent flags.

Segment conditions are already JSONPath, so a dependency needs no new
operator or condition type. What it does need is for the flag to be
resolved by the time the condition reads it, and evaluation resolved
every segment before any flag.

Rather than establish up front which segments depend on which flags,
`$.flags` is a mapping that resolves a flag when a condition first
reads it. Resolving a flag evaluates the segments overriding it, which
in turn resolves whatever flags their conditions read, memoised
throughout. The existing single pass is otherwise untouched: a context
whose segments read no flag never enters the resolver, and pays only
for one empty mapping.

Resolving on read rather than in advance means there is no need to
recognise a dependency in a condition property, and so no need to
reimplement enough of the JSONPath grammar to tell that
`$.flags.a['enabled']` and `$.flags.a.enabled` are the same query.
Every spelling works because resolution is triggered by the read
itself. It also costs only what is read: rule short-circuiting means a
condition that is never evaluated resolves nothing.

Measured against main across the benchmark contexts, interleaved to
cancel drift: +3.4%, from the mapping and a shallow copy of the context
to hold it. Establishing up front that an environment has no
dependencies would remove that residual entirely, but needs a field on
the context to carry it.

A flag caught in a dependency cycle is left unresolved rather than
raising, so a cycle reaching the engine degrades to a non-matching
condition instead of breaking evaluation for the whole context. Such a
flag serves its environment default and reports
`ERROR; code=CIRCULAR_DEPENDENCY`, so that a flag which could not be
resolved is distinguishable from one that was never gated. Only flags
in the cycle are reported that way; a flag merely depending on one
resolves normally. Cycles are still expected to be rejected where
dependencies are written.

Behaviour is covered by test cases in Flagsmith/engine-test-data#59
rather than by unit tests here, so that every engine is held to it.
khvn26 added a commit to Flagsmith/flagsmith-engine that referenced this pull request Aug 28, 2026
REVERT BEFORE MERGE.

Dependent flags behaviour is covered by test cases in
Flagsmith/engine-test-data#59 rather than by unit tests here, so that
every engine is held to it. Those cases aren't in a release yet, so
without this the new code is exercised by nothing and CI's 100%
coverage gate fails.

Once #59 is merged and tagged, this goes back to a semver tag, which
Renovate now tracks as of #337.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant