Skip to content

ci: Run the Flagsmith SDK behavioural spec - #89

Merged
Zaimwa9 merged 2 commits into
mainfrom
feat/sdk-spec
Oct 21, 2026
Merged

Zaimwa9 merged 2 commits into
mainfrom
feat/sdk-spec

Conversation

@Zaimwa9

@Zaimwa9 Zaimwa9 commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Runs the Flagsmith SDK behavioural spec against this SDK on every pull request and push to main.

  • sdk-spec/steps.kt: the spec's steps, one function per When step, under the spec's Kotlin harness. It doubles as a usage example.
  • sdk-spec/Dockerfile: builds the SDK from source with the steps file and the harness. sdk-spec/build.gradle.kts is a standalone Gradle JVM build that compiles FlagsmithClient/src/main/java directly, with minimal stubs in sdk-spec/src for the android.* classes the SDK references, so it runs in a plain Linux container without the Android SDK or an emulator. The client is created with analytics disabled, since analytics needs an Android Context; caching and realtime updates are off by default.
  • .github/workflows/sdk-spec.yml: calls the spec's reusable workflow, pinned to v0.5.0, with language: kotlin. The workflow starts a real Flagsmith instance, seeds it through the Admin API and runs the scenarios through the harness.
  • renovate.json: bumps the uses: ref and spec-ref together. It only enables the regex manager for the spec, so Renovate doesn't start updating other dependencies.

The spec currently covers getting environment flags and getting identity flags with traits. The identity scenario caught trait values being sent as strings, fixed in #90.

The dependency versions in sdk-spec/build.gradle.kts duplicate those in FlagsmithClient/build.gradle.kts and need to be kept in sync.

@Zaimwa9

Zaimwa9 commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

@themis-blindfold review

@Zaimwa9
Zaimwa9 marked this pull request as ready for review October 21, 2026 15:08
@Zaimwa9
Zaimwa9 requested a review from a team as a code owner October 21, 2026 15:08
@Zaimwa9
Zaimwa9 requested review from talissoncosta and removed request for a team October 21, 2026 15:08
@Zaimwa9

Zaimwa9 commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

@themis-blindfold review

@themis-blindfold

Copy link
Copy Markdown

⚖️ Themis review: 🟠 Fix before merge

The SDK behavioural spec is now wired into CI and the trait serializer retains the common JSON types, but the new adapter still rejects a permitted null trait and can round large integer traits. All completed CI checks passed, including the behavioural spec, unit tests, and the build check.

Area Score
🎯 Correctness 3/5
🧪 Test coverage 3/5
📐 Code quality 3/5
🚀 Product impact 3/5

🟠 Majors

  • spec-adapter/src/main/kotlin/com/flagsmith/specadapter/Main.kt:80 — trait requests do not faithfully preserve every value accepted by the adapter protocol.
📝 Walkthrough
  • Trait serialisation - sends string, integer, decimal, and boolean values as JSON values rather than strings.
  • Spec adapter - wraps environment and identity flag calls in the protocol HTTP service used by the behavioural runner.
  • CI workflow - invokes the pinned reusable SDK-spec workflow for pull requests and pushes to main.
🧪 How to verify
  1. Build the adapter with docker build -t sdk-spec-adapter:local -f spec-adapter/Dockerfile ..
  2. Send an identity-flags request containing {"traits":{"cleared":null,"accountId":9007199254740993}} through a client backed by a mock Flagsmith API.
  3. Assert the mock receives JSON null and the exact integer value in its /identities/ request body.
  4. Run ./gradlew :FlagsmithClient:testDebugUnitTest.
  5. Run the behavioural spec workflow against the built adapter.

Automate: add adapter coverage for a null trait and an integer above 2^53.

Product take: A solid CI safeguard for SDK behaviour, but it should cover the complete trait value contract before it becomes the regression gate.

Big picture: Observed: the adapter maintains a separate dependency list and Android stub surface from the SDK build. Predicted: future SDK dependency or framework changes could make only the spec build drift; add a parity check or derive those inputs from a shared source.

🧭 Assumptions & unverified claims

No unverified assumptions or claims.

A sharper spec harness needs one more edge in its trait drawer · reviewed at 32f5af6

1 finding(s) anchored outside the diff (not posted inline)
  • spec-adapter/src/main/kotlin/com/flagsmith/specadapter/Main.kt:80 🟠 Major · ⚡ Quick win

Preserve every trait value accepted by the adapter protocol

Observed: value.asJsonPrimitive rejects JSON null, and the fallback converts every integer outside the Int range to Double (for example, 9007199254740993 rounds). Predicted: a protocol-conforming null trait would return 500, while a large integer could reach the Flagsmith API with a different value. Pass the original JsonElement through to Trait rather than coercing it to Int or Double, and add null and large-integer adapter coverage.

@Zaimwa9
Zaimwa9 force-pushed the feat/sdk-spec branch 2 times, most recently from 3ebeda6 to 1f2dc92 Compare October 21, 2026 15:34
@Zaimwa9

Zaimwa9 commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

@themis-blindfold review

Comment thread renovate.json
@themis-blindfold

Copy link
Copy Markdown

⚖️ Themis review: 🟠 Fix before merge

The behavioural spec and existing test checks passed, and the trait serialization fix preserves primitive JSON types. However, the Renovate rule updates only one of the two workflow references that are required to stay aligned; the first tag-based update will split them.

Area Score
🎯 Correctness 3/5
🧪 Test coverage 4/5
📐 Code quality 3/5
🚀 Product impact 2/5

🟠 Majors

  • renovate.json:9 — Renovate would update only spec-ref, leaving the reusable workflow on its previous release.
📝 Walkthrough
  • Trait serialization — preserves string, numeric, and boolean JSON values; serializer and client-request tests cover the changed behaviour.
  • SDK spec runner — compiles the client with minimal Android stubs and executes the Kotlin behavioural-spec harness in CI.
  • Spec release maintenance — pins the reusable workflow and spec checkout to the same revision, but the dependency updater currently tracks only one of them.
🧪 How to verify
  1. Run Renovate in dry-run mode after temporarily changing both workflow references to the same vX.Y.Z tag; confirm its proposed update changes both fields to the next tag.
  2. Build the runner with docker build -f sdk-spec/Dockerfile --build-context harness=<sdk-spec>/harness/kotlin ..
  3. Run ./gradlew :FlagsmithClient:testDebugUnitTest to exercise the Android client tests.
  4. Confirm the SDK-spec workflow still completes against the pinned spec revision.
    Automate: add a Renovate configuration test or fixture asserting that both workflow references are replaced together.

Product take: Behavioural-spec coverage is a solid safety improvement, but it needs reliable release-pin maintenance to keep that protection current.

🧭 Assumptions & unverified claims

No unverified assumptions or claims.

The spec caught a real trait regression; its two release pins should travel together · reviewed at 1f2dc92

@themis-blindfold

Copy link
Copy Markdown

⚖️ Themis review: 🟠 Fix before merge

The behavioural-spec workflow, unit tests, and build check all completed successfully. The new runner exercises environment and identity evaluation, but the configured Renovate update would leave the reusable workflow pinned to its old revision while changing only spec-ref; repair that before relying on release-tag updates.

Area Score
🎯 Correctness 3/5
🧪 Test coverage 4/5
📐 Code quality 3/5
🚀 Product impact 4/5

🟠 Majors

  • renovate.json: a Renovate update will desynchronise the reusable workflow reference and spec-ref.
📝 Walkthrough
  • Behavioural-spec CI - adds a reusable workflow for Kotlin SDK conformance runs on pull requests and main pushes.
  • Standalone JVM runner - compiles the SDK source with minimal Android stubs, the Kotlin harness, and the spec step functions in a container.
  • Trait serialisation - preserves string, numeric, and boolean JSON values so identity targeting receives their intended types.
  • Dependency maintenance - tracks the spec revision, but the current custom regex replaces only one of the two coupled references.
🧪 How to verify
  1. Build the runner with docker build -f sdk-spec/Dockerfile --build-context harness=<path-to-sdk-spec>/harness/kotlin ..
  2. Run the resulting image against the behavioural-spec harness and confirm environment and identity-with-traits scenarios pass.
  3. Change both workflow references to the same published vX.Y.Z tag, then run a Renovate dry-run and confirm its update changes both references together.
  4. Run ./gradlew :FlagsmithClient:testDebugUnitTest to confirm typed trait serialisation remains covered.
    Automate: add a Renovate fixture or config test that asserts a tag update rewrites both coupled workflow references.

Product take: This adds valuable cross-SDK compatibility coverage and fixes trait typing that directly affects segment evaluation. The benefit is meaningful, provided spec revision updates cannot silently split the runner from its harness.

🧭 Assumptions & unverified claims

No unverified assumptions or claims.

Two pins, one source of truth — just make the robot move both · reviewed at 1f2dc92

@Zaimwa9

Zaimwa9 commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

@themis-blindfold review

@Zaimwa9

Zaimwa9 commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

@themis-blindfold review

@themis-blindfold

Copy link
Copy Markdown

⚖️ Themis review: ✅ Ship it

The behavioural-spec job adds meaningful end-to-end coverage for environment and identity flag retrieval without changing the shipped SDK. The captured SDK-spec, unit-test, and build checks were still running; static validation found the JVM adapter disables the Android-only analytics path and keeps its runtime dependencies aligned with the client build.

Area Score
🎯 Correctness 4/5
🧪 Test coverage 4/5
📐 Code quality 4/5
🚀 Product impact 3/5
📝 Walkthrough
  • SDK-spec workflow - runs the Kotlin behavioural harness for pull requests and pushes to main.
  • JVM adapter - compiles the Android client with narrow framework stubs so the spec can execute in Linux containers.
  • Spec steps - maps the environment- and identity-flags scenarios, including typed traits, to the public client API.
  • Dependency maintenance - Renovate extracts each workflow reference independently and groups their updates.
🧪 How to verify
  1. Open a pull request and confirm the SDK-spec job passes both environment and identity-with-traits scenarios.
  2. Build the harness image with docker build -f sdk-spec/Dockerfile --build-context harness=<sdk-spec>/harness/kotlin ..
  3. Run ./gradlew check -x koverVerify -P excludeIntegrationTests.
  4. Run a Renovate extraction dry run after changing both v0.5.0 references to confirm they are grouped in one update.
    Automate: Keep the reusable SDK-spec workflow required for pull requests.

Product take: Solid quality improvement: the SDK now exercises its server-facing flag and trait contract against a real Flagsmith instance on every change.

🧭 Assumptions & unverified claims

No unverified assumptions or claims.

A useful little guardrail: may the flags stay where the harness put them · reviewed at 6464104

@themis-blindfold

Copy link
Copy Markdown

⚖️ Themis review: ✅ Ship it

TL;DR: The new behavioural-spec job builds the SDK from source in a JVM-only container and passed alongside the existing unit and build checks. The step adapter covers the stated environment-flag and identity-with-traits paths, and the Renovate rules keep both spec references in the same update group.

Area Score
🎯 Correctness 5/5
🧪 Test coverage 4/5
📐 Code quality 4/5
🚀 Product impact 4/5
📝 Walkthrough
  • sdk-spec build - compiles the production SDK sources with minimal Android shims and the Kotlin harness.
  • Spec steps - adapt environment and identity flag fetches, including typed traits, to the behavioural-spec contract.
  • CI and dependency maintenance - runs the shared spec workflow on pull requests and main, with grouped Renovate updates for both spec refs.
🧪 How to verify
  1. Run ./gradlew :FlagsmithClient:testDebugUnitTest.
  2. Check out Flagsmith/sdk-spec at v0.5.0, then build with docker build -f sdk-spec/Dockerfile --build-context harness=<sdk-spec>/harness/kotlin ..
  3. Run the resulting image against a local Flagsmith instance seeded with environment and identity trait scenarios.
  4. Confirm a Renovate extraction dry run detects both run-spec.yml and spec-ref as Flagsmith/sdk-spec dependencies.

Automate: retain the reusable SDK-spec workflow as a required pull-request check.

Product take: Solid regression protection for the public flag-evaluation contract. It raises confidence in routine SDK changes, particularly around identity traits, without changing the shipped client.

Big picture: The standalone spec build duplicates the production runtime dependency list. That gives one client source two dependency owners, so future library upgrades need changes in both builds to keep the behavioural check representative. Keeping those versions in a shared Gradle convention or generated dependency source would contain that drift risk.

🧭 Assumptions & unverified claims

No unverified assumptions or claims.

A welcome new canary for trait-shaped trouble · reviewed at 6464104

talissoncosta
talissoncosta previously approved these changes Oct 21, 2026

@talissoncosta talissoncosta left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Comment thread renovate.json Outdated
@Zaimwa9
Zaimwa9 merged commit 169807e into main Oct 21, 2026
3 checks passed
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.

3 participants