Skip to content

test: exercise SDK APIs in macOS compliance coverage - #805

Open
dustinbyrne wants to merge 4 commits into
mainfrom
ci/sdk-harness-coverage-2026-09-07
Open

dustinbyrne wants to merge 4 commits into
mainfrom
ci/sdk-harness-coverage-2026-09-07

Conversation

@dustinbyrne

Copy link
Copy Markdown
Contributor

💡 Motivation and Context

Make compliance results exercise the SDK rather than adapter-written flags HTTP, retries, parsing, and called-events.

  • Forward capture timestamps through the existing public Date argument and observe SDK-generated UUIDs.
  • Use public identity/group setters, reload callbacks, and cached getters; retain native retries and side effects.
  • Replace idle-network flush completion with conservative acknowledgment observation. Unresolved captures time out rather than claiming delivery or retry exhaustion.
  • Run harness 1.0.0 without filters; verify the expected 47-case inventory and label reports as macOS shared-core coverage.

Adapter/CI changes only: no shipped SDK source, public API, defaults, or release changes.

💚 How did you test it?

  • Adapter make test: 6 Swift and 5 Python tests passed.
  • Public-entry make smoke: timestamp/UUID/503 flush, invalid timestamps, and native flags 502/504 retry/result/group/called-event scenarios passed.
  • Scoped formatting/lint, shell syntax, and YAML checks passed.
  • Pinned harness 1.0.0 source on macOS arm64: 47 selected, 32 passed, 15 failed — capture 29/30, flags 3/17. All expected test IDs executed; this is not an all-green compliance verdict.

The capture failure is the adapter's explicit timeout when retry exhaustion cannot be observed through a public drain callback. Flags failures expose extra identity/group-triggered requests, native field/lifecycle differences, and one-shot fixtures consumed before the explicit reload. The initial expectation was 34 passes; returned-value and called-event-value fixtures exposed two additional failures. Endpoint-specific smoke fixtures verify those SDK paths separately without replacing harness results.

Local Docker was unavailable; the hybrid Docker/Colima CI run remains to be checked. No iOS simulator/device validation is claimed. See sdk_compliance_adapter/README.md for profile limitations.

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Implemented with Pi worker using file and shell tools, followed by a fresh read-only agent review. The profile preserves SDK ownership of transport and side effects; known contract mismatches remain visible rather than filtered. Human review is required. The local agent session is not publicly accessible.

@dustinbyrne dustinbyrne self-assigned this Sep 7, 2026
@github-actions

github-actions Bot commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

Profile: macOS shared core (not iOS-device validation), gzip /batch, preload/autocapture disabled, public identity/group + reload + cached getter. SDK-triggered reloads are retained; unresolved queue acknowledgments time out. See sdk_compliance_adapter/README.md for contract limitations.

posthog-ios Compliance Report

Date: 2026-09-14T19:43:18.138518+00:00
Duration: 152391ms

⚠️ Some Tests Failed

32/47 tests passed, 15 failed


Capture Tests

⚠️ 29/30 tests passed, 1 failed

View Details
Test Status Duration
Format Validation.Event Has Required Fields ✅ 501ms
Format Validation.Event Has Uuid ✅ 363ms
Format Validation.Event Has Lib Properties ✅ 376ms
Format Validation.Distinct Id Is String ✅ 242ms
Format Validation.Token Is Present ✅ 406ms
Format Validation.Custom Properties Preserved ✅ 203ms
Format Validation.Event Has Timestamp ✅ 294ms
Format Validation.Non Utc Event Timestamp Is Converted To Utc ✅ 214ms
Retry Behavior.Retries On 503 ✅ 9261ms
Retry Behavior.Does Not Retry On 400 ✅ 2255ms
Retry Behavior.Does Not Retry On 401 ✅ 2187ms
Retry Behavior.Respects Retry After Header ✅ 8680ms
Retry Behavior.Implements Backoff ✅ 22254ms
Retry Behavior.Retries On 500 ✅ 6748ms
Retry Behavior.Retries On 502 ✅ 6908ms
Retry Behavior.Retries On 504 ✅ 6640ms
Retry Behavior.Max Retries Respected ❌ 32215ms
Deduplication.Generates Unique Uuids ✅ 403ms
Deduplication.Preserves Uuid On Retry ✅ 6635ms
Deduplication.Preserves Uuid And Timestamp On Retry ✅ 13831ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry ✅ 10184ms
Deduplication.No Duplicate Events In Batch ✅ 229ms
Deduplication.Different Events Have Different Uuids ✅ 209ms
Compression.Sends Gzip When Enabled ✅ 231ms
Batch Format.Uses Proper Batch Structure ✅ 224ms
Batch Format.Flush With No Events Sends Nothing ✅ 118ms
Batch Format.Multiple Events Batched Together ✅ 310ms
Error Handling.Does Not Retry On 403 ✅ 2321ms
Error Handling.Does Not Retry On 413 ✅ 2255ms
Error Handling.Retries On 408 ✅ 6834ms

Failures

retry_behavior.max_retries_respected

Expected 4 requests, got 6

Feature_Flags Tests

⚠️ 3/17 tests passed, 14 failed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id ❌ 558ms
Request Payload.Flags Request Uses V2 Query Param ❌ 502ms
Request Payload.Flags Request Hits Flags Path Not Decide ❌ 431ms
Request Payload.Flags Request Omits Authorization Header ❌ 402ms
Request Payload.Token In Flags Body Matches Init ✅ 309ms
Request Payload.Groups Round Trip ❌ 430ms
Request Payload.Groups Default To Empty Object ❌ 402ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False ❌ 418ms
Request Payload.Disable Geoip Omitted Defaults To False ❌ 544ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key ❌ 475ms
Request Lifecycle.No Flags Request On Init Alone ✅ 110ms
Request Lifecycle.No Flags Request On Normal Capture ✅ 315ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests ❌ 515ms
Request Lifecycle.Mock Response Value Is Returned To Caller ❌ 449ms
Retry Behavior.Retries Flags On 502 ❌ 629ms
Retry Behavior.Retries Flags On 504 ❌ 795ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event ❌ 492ms

Failures

request_payload.request_with_person_properties_device_id

Expected 1 /flags requests, got 2

request_payload.flags_request_uses_v2_query_param

Expected 1 /flags requests, got 2

request_payload.flags_request_hits_flags_path_not_decide

Expected 1 /flags requests, got 2

request_payload.flags_request_omits_authorization_header

Expected 1 /flags requests, got 2

request_payload.groups_round_trip

Field 'company' not found in /flags request body at path 'groups.company'. Available keys: []

request_payload.groups_default_to_empty_object

Field 'group_properties' not found in /flags request body at path 'group_properties'. Available keys: ['person_properties', 'groups', '$anon_distinct_id', 'api_key', 'distinct_id', 'timezone', '$device_id']

request_payload.disable_geoip_false_propagates_as_geoip_disable_false

Field 'geoip_disable' not found in /flags request body at path 'geoip_disable'. Available keys: ['groups', '$anon_distinct_id', '$device_id', 'api_key', 'distinct_id', 'timezone', 'person_properties']

request_payload.disable_geoip_omitted_defaults_to_false

Field 'geoip_disable' not found in /flags request body at path 'geoip_disable'. Available keys: ['groups', 'distinct_id', 'person_properties', 'api_key', '$anon_distinct_id', '$device_id', 'timezone']

request_payload.flag_keys_to_evaluate_contains_only_requested_key

Field 'flag_keys_to_evaluate' not found in /flags request body at path 'flag_keys_to_evaluate'. Available keys: ['timezone', '$device_id', 'distinct_id', 'person_properties', 'api_key', 'groups', '$anon_distinct_id']

request_lifecycle.two_flag_calls_produce_two_remote_requests

Expected 2 /flags requests, got 3

request_lifecycle.mock_response_value_is_returned_to_caller

Expected last action result 'variant-a', got None

retry_behavior.retries_flags_on_502

Expected 2 /flags requests, got 3

retry_behavior.retries_flags_on_504

Expected 2 /flags requests, got 3

side_effect_events.get_feature_flag_captures_feature_flag_called_event

Event '$feature_flag_called' property '$feature_flag_response': expected True, got None

@dustinbyrne

Copy link
Copy Markdown
Contributor Author

Compliance failures and follow-up plan

At f41039d612175d43e50d0616a0bd51035bddb8d9, the published harness 1.0.0 report is 32/47 passed, 15 failed: capture 29/30, flags 3/17. These failures concern the harness's observation/contract assumptions, rather than demonstrated SDK retry or flag-evaluation defects.

Failures Classification and cause Plan
Seven flag request-count assertions: person-properties request, v2 query, flags path, absent Authorization, two remote calls, and 502/504 retries Harness issue. Counts include legitimate identify/group-triggered reloads as well as the measured reload. Make shared client tests correlate the intended request and its retries, rather than require stateless request counts. Preserve genuine SDK side effects.
feature_flags.request_payload.groups_round_trip Harness issue. The assertion inspects a request made before group association. Assert against the completed group-aware request, with context/response correlation and a negative control for genuinely missing groups.
groups_default_to_empty_object, disable_geoip_false_propagates_as_geoip_disable_false, disable_geoip_omitted_defaults_to_false in feature_flags.request_payload Harness issue. The SDK omits empty group properties and false/default GeoIP fields that the assertions require literally. Accept the established equivalent omissions in the shared assertions, while continuing to reject null, wrong types and incorrect values.
feature_flags.request_payload.flag_keys_to_evaluate_contains_only_requested_key Harness applicability issue. A public cached getter does not make its preceding reload a singleton remote evaluation. Use client reload/cache tests for this API; retain singleton filtering assertions for SDK APIs that expose that control.
feature_flags.request_lifecycle.mock_response_value_is_returned_to_caller and feature_flags.side_effect_events.get_feature_flag_captures_feature_flag_called_event Harness fixture issue. An identity-triggered reload can consume the one-shot value before the explicit reload. Use stable flag fixtures and wait for the relevant response/cache boundary; verify the returned value and actual SDK called-event.
capture.retry_behavior.max_retries_respected — /flush returns 504 Harness completion-contract issue. Repeated rejection produces no delivery acknowledgment, and the public SDK exposes no queue-drain callback. The timeout is not proof of excess retries. Separate delivery, rejected-send observation and unobservable exhaustion in the shared flush contract. Check retry behavior without manufacturing a successful delivery or inferring an empty queue.

The plan is to make these changes in the shared harness, adopt the compatible client preparation/profile bindings, and rerun the affected cases with positive and negative controls. The current reports remain red; later local results do not replace this PR's CI evidence.

This PR is being put up for review as macOS shared-core adapter/CI coverage, not iOS-device or all-green conformance certification. Profile and public-API boundaries. Workflow success is advisory for these assertions.

@dustinbyrne
dustinbyrne marked this pull request as ready for review September 11, 2026 18:34
@dustinbyrne
dustinbyrne requested a review from a team as a code owner September 11, 2026 18:34
@greptile-apps

greptile-apps Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor
Prompt To Fix All With AI
### Issue 1
sdk_compliance_adapter/Sources/RequestTracker.swift:24-36
**Capture UUIDs can collide**

Concurrent `/capture` requests can read the same `captured.count` index before either finishes. The global `beforeSend` callback then appends both UUIDs, but both requests can return the first UUID, corrupting capture responses and compliance observations. Serialize the SDK operation through UUID retrieval or correlate each callback with its originating capture instead of using a shared array index.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "test: exercise SDK APIs in macOS complia..." | Re-trigger Greptile

Comment on lines +24 to +36
func beginCapture() -> Int {
lock.lock()
defer { lock.unlock() }
unobservedCaptures += 1
return captured.count
}

func finishCapture(after index: Int) -> String? {
lock.lock()
defer { lock.unlock() }
guard captured.count > index else { return nil }
unobservedCaptures -= 1
return captured[index]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Capture UUIDs can collide

Concurrent /capture requests can read the same captured.count index before either finishes. The global beforeSend callback then appends both UUIDs, but both requests can return the first UUID, corrupting capture responses and compliance observations. Serialize the SDK operation through UUID retrieval or correlate each callback with its originating capture instead of using a shared array index.

Knowledge Base Used: Analytics capture and identity

Prompt To Fix With AI
This is a comment left during a code review.
Path: sdk_compliance_adapter/Sources/RequestTracker.swift
Line: 24-36

Comment:
**Capture UUIDs can collide**

Concurrent `/capture` requests can read the same `captured.count` index before either finishes. The global `beforeSend` callback then appends both UUIDs, but both requests can return the first UUID, corrupting capture responses and compliance observations. Serialize the SDK operation through UUID retrieval or correlate each callback with its originating capture instead of using a shared array index.

**Knowledge Base Used:** [Analytics capture and identity](https://app.greptile.com/posthog-org-19734/-/custom-context/knowledge-base/posthog/posthog-ios/-/docs/analytics-capture-and-identity.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

@turnipdabeets turnipdabeets left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, one greptile that might be worth addressing

@github-actions

Copy link
Copy Markdown
Contributor

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.

@github-actions github-actions Bot added the stale label Sep 22, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants