Skip to content

fix: strengthen SDK tests and preserve fork-curl gzip payloads - #249

Draft
marandaneto wants to merge 1 commit into
mainfrom
fix/test-audit-and-gzip
Draft

marandaneto wants to merge 1 commit into
mainfrom
fix/test-audit-and-gzip

Conversation

@marandaneto

Copy link
Copy Markdown
Member

💡 Motivation and Context

Several tests could pass without checking the behavior they named. Some assertions were unreachable after an expected exception. Other tests used missing flag keys instead of the intended fixtures, accepted both null and false, or only checked that an event was queued. Older consumer tests also contacted live PostHog endpoints.

This audit replaces live requests with a loopback server and verifies the actual requests from all three network consumers. The gzip cases caught a production bug in fork-curl: shell echo could interpret escaped newlines and produce invalid JSON. Using printf '%s' preserves the payload. This is the only production code change.

The remaining changes strengthen flag results, local-only routing, group rollout bucketing, cache round trips, timestamp serialization, exception capture and context isolation. Shared test helpers now reject unexpected HTTP routes and restore nested clocks. Python adapter tests use ephemeral ports and restore their environment. CI now runs the adapter fidelity tests alongside PHPUnit.

The restored invalid-date tests retain PHP's existing exception behavior. That behavior differs from the current local-evaluator spec, which calls for no-match on malformed property dates and supports integral Unix seconds. Aligning those semantics remains a separate compatibility change.

💚 How did you test it?

Measured both versions with PHP 8.5.10, PHPUnit 11.5.55 and Xdebug 3.5.0 on macOS. Both runs used the same source filter and blocked external networking while allowing loopback.

Metric Before After
PHP test cases 696 729
Assertions 4,638 5,379
Line coverage 86.64% (2,360/2,724) 90.35% (2,461/2,724)
Method coverage 71.65% (187/261) 73.95% (193/261)

The baseline had one failure and one error under network isolation. The repaired suite passes all 729 PHP cases and 16 Python cases. No previously covered production lines were lost, and no coverage exclusions were added.

  • The full PHP suite also passed in randomized order with warnings and risky tests treated as failures.
  • Six controlled defects were detected, including the original gzip bug, wrong group bucketing, empty flag results, payload-access side effects, zero-valued Retry-After dates and broken nested-clock restoration.
  • Composer validation, the public API snapshot, workflow lint and whitespace checks passed.
  • PHP_CodeSniffer reported zero errors and the same 55 existing warnings. Existing PHP and PHPUnit deprecation notices remain.
  • Autoreview passed at b3b2860dd5226b90cf2ee9bd8f8327ad10e46df4 with no actionable findings.

Linux, PHP 8.2-8.4 and the external Docker compliance harness were not run locally. Coverage does not include manually launched PHP subprocesses or Python implementation code.

📝 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.

If releasing new changes

  • Ran pnpm change to generate a change intent file

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Pi performed the audit using file, shell and subagent tools. Two read-only subagents reviewed separate flag suites. Pi applied the fixes and ran the tests, coverage comparison and mutation checks. The isolated autoreview helper reviewed the committed branch. No session was published.

The audit kept the existing regression vectors and test layers. It added a loopback transport fixture instead of mocking the code that sends requests. The detailed audit ledger and logs remain local rather than adding machine-specific files to the repository. Human review is required.

@marandaneto marandaneto self-assigned this Sep 26, 2026
@marandaneto

Copy link
Copy Markdown
Member Author

Test coverage comparison

Baseline: c2c9dd294477b10bb513a93101ef3e394e076489
PR head: b3b2860dd5226b90cf2ee9bd8f8327ad10e46df4

Metric Before After Change
Line coverage 86.64% (2,360/2,724) 90.35% (2,461/2,724) +3.71 percentage points, +101 lines
Method coverage 71.65% (187/261) 73.95% (193/261) +2.30 percentage points, +6 methods
Fully covered classes 40.00% (10/25) 40.00% (10/25) No change
PHP test cases 696 729 +33
Assertions 4,638 5,379 +741
PHP failures / errors 1 / 1 0 / 0 Both resolved
Python tests 16 passing 16 passing No change

Both measurements used PHP 8.5.10, PHPUnit 11.5.55 and Xdebug 3.5.0 on macOS, with the same Composer lockfile and unchanged coverage filter covering all PHP files under lib, including uncovered files. External networking was blocked and loopback was allowed.

The baseline was not green under network isolation. Its file-sender test depended on live delivery, and its socket production-error test installed a throwing error callback while expecting no exception. The repaired tests exercise local delivery and failure behavior without live PostHog requests.

No previously covered production lines were lost, and no coverage exclusions were added. All 729 PHP tests also passed in randomized order. Six targeted mutations were detected by the strengthened assertions.

These figures measure SDK line/method coverage, not branch coverage or an exhaustive mutation score. They exclude manually launched PHP subprocesses and Python implementation coverage. Existing PHP/PHPUnit deprecation notices remain. Linux, PHP 8.2-8.4 and the external Docker compliance harness were not run locally.

@greptile-apps

greptile-apps Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Retrigger

[Medium risk] Refactors transport tests and fixes gzip payload handling.

The PR appears safe to merge; no actionable regression was identified.

Reviews (1) · Last reviewed commit: "fix: strengthen SDK tests and preserve f..."

@github-actions

Copy link
Copy Markdown
Contributor

posthog-php-fork_curl Compliance Report

Date: 2026-09-26T15:29:08.220292+00:00
Duration: 112389ms

⚠️ Some Tests Failed

36/47 tests passed, 11 failed


Capture Tests

⚠️ 19/30 tests passed, 11 failed

View Details
Test Status Duration
Format Validation.Event Has Required Fields ✅ 34ms
Format Validation.Event Has Uuid ✅ 529ms
Format Validation.Event Has Lib Properties ✅ 529ms
Format Validation.Distinct Id Is String ✅ 530ms
Format Validation.Token Is Present ✅ 531ms
Format Validation.Custom Properties Preserved ✅ 530ms
Format Validation.Event Has Timestamp ✅ 531ms
Format Validation.Non Utc Event Timestamp Is Converted To Utc ✅ 529ms
Retry Behavior.Retries On 503 ❌ 5533ms
Retry Behavior.Does Not Retry On 400 ✅ 2534ms
Retry Behavior.Does Not Retry On 401 ✅ 2532ms
Retry Behavior.Respects Retry After Header ❌ 5535ms
Retry Behavior.Implements Backoff ❌ 15547ms
Retry Behavior.Retries On 500 ❌ 5534ms
Retry Behavior.Retries On 502 ❌ 5540ms
Retry Behavior.Retries On 504 ❌ 5536ms
Retry Behavior.Max Retries Respected ❌ 15546ms
Deduplication.Generates Unique Uuids ✅ 537ms
Deduplication.Preserves Uuid On Retry ❌ 5537ms
Deduplication.Preserves Uuid And Timestamp On Retry ❌ 10533ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry ❌ 5044ms
Deduplication.No Duplicate Events In Batch ✅ 537ms
Deduplication.Different Events Have Different Uuids ✅ 531ms
Compression.Sends Gzip When Enabled ✅ 533ms
Batch Format.Uses Proper Batch Structure ✅ 530ms
Batch Format.Flush With No Events Sends Nothing ✅ 519ms
Batch Format.Multiple Events Batched Together ✅ 519ms
Error Handling.Does Not Retry On 403 ✅ 2531ms
Error Handling.Does Not Retry On 413 ✅ 2534ms
Error Handling.Retries On 408 ❌ 5535ms

Failures

retry_behavior.retries_on_503

Expected at least 3 requests, got 1

retry_behavior.respects_retry_after_header

Expected at least 2 requests, got 1

retry_behavior.implements_backoff

Expected at least 3 requests, got 1

retry_behavior.retries_on_500

Expected at least 2 requests, got 1

retry_behavior.retries_on_502

Expected at least 2 requests, got 1

retry_behavior.retries_on_504

Expected at least 2 requests, got 1

retry_behavior.max_retries_respected

Expected 4 requests, got 1

deduplication.preserves_uuid_on_retry

Need at least 2 requests to check retry

deduplication.preserves_uuid_and_timestamp_on_retry

Expected at least 3 requests, got 1

deduplication.preserves_uuid_and_timestamp_on_batch_retry

Expected at least 2 requests, got 1

error_handling.retries_on_408

Expected at least 2 requests, got 1

Feature_Flags Tests

✅ 17/17 tests passed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id ✅ 523ms
Request Payload.Flags Request Uses V2 Query Param ✅ 521ms
Request Payload.Flags Request Hits Flags Path Not Decide ✅ 522ms
Request Payload.Flags Request Omits Authorization Header ✅ 521ms
Request Payload.Token In Flags Body Matches Init ✅ 521ms
Request Payload.Groups Round Trip ✅ 521ms
Request Payload.Groups Default To Empty Object ✅ 521ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False ✅ 521ms
Request Payload.Disable Geoip Omitted Defaults To False ✅ 522ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key ✅ 521ms
Request Lifecycle.No Flags Request On Init Alone ✅ 516ms
Request Lifecycle.No Flags Request On Normal Capture ✅ 516ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests ✅ 525ms
Request Lifecycle.Mock Response Value Is Returned To Caller ✅ 522ms
Retry Behavior.Retries Flags On 502 ✅ 624ms
Retry Behavior.Retries Flags On 504 ✅ 624ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event ✅ 531ms

@github-actions

Copy link
Copy Markdown
Contributor

posthog-php-lib_curl Compliance Report

Date: 2026-09-26T15:29:18.146505+00:00
Duration: 117875ms

✅ All Tests Passed!

47/47 tests passed


Capture Tests

✅ 30/30 tests passed

View Details
Test Status Duration
Format Validation.Event Has Required Fields ✅ 28ms
Format Validation.Event Has Uuid ✅ 524ms
Format Validation.Event Has Lib Properties ✅ 524ms
Format Validation.Distinct Id Is String ✅ 526ms
Format Validation.Token Is Present ✅ 525ms
Format Validation.Custom Properties Preserved ✅ 524ms
Format Validation.Event Has Timestamp ✅ 527ms
Format Validation.Non Utc Event Timestamp Is Converted To Utc ✅ 525ms
Retry Behavior.Retries On 503 ✅ 5836ms
Retry Behavior.Does Not Retry On 400 ✅ 2527ms
Retry Behavior.Does Not Retry On 401 ✅ 2528ms
Retry Behavior.Respects Retry After Header ✅ 8535ms
Retry Behavior.Implements Backoff ✅ 16234ms
Retry Behavior.Retries On 500 ✅ 5148ms
Retry Behavior.Retries On 502 ✅ 5640ms
Retry Behavior.Retries On 504 ✅ 5631ms
Retry Behavior.Max Retries Respected ✅ 17050ms
Deduplication.Generates Unique Uuids ✅ 233ms
Deduplication.Preserves Uuid On Retry ✅ 5633ms
Deduplication.Preserves Uuid And Timestamp On Retry ✅ 10836ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry ✅ 5141ms
Deduplication.No Duplicate Events In Batch ✅ 531ms
Deduplication.Different Events Have Different Uuids ✅ 527ms
Compression.Sends Gzip When Enabled ✅ 526ms
Batch Format.Uses Proper Batch Structure ✅ 525ms
Batch Format.Flush With No Events Sends Nothing ✅ 521ms
Batch Format.Multiple Events Batched Together ✅ 513ms
Error Handling.Does Not Retry On 403 ✅ 2526ms
Error Handling.Does Not Retry On 413 ✅ 2530ms
Error Handling.Retries On 408 ✅ 5634ms

Feature_Flags Tests

✅ 17/17 tests passed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id ✅ 524ms
Request Payload.Flags Request Uses V2 Query Param ✅ 523ms
Request Payload.Flags Request Hits Flags Path Not Decide ✅ 524ms
Request Payload.Flags Request Omits Authorization Header ✅ 523ms
Request Payload.Token In Flags Body Matches Init ✅ 523ms
Request Payload.Groups Round Trip ✅ 523ms
Request Payload.Groups Default To Empty Object ✅ 523ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False ✅ 523ms
Request Payload.Disable Geoip Omitted Defaults To False ✅ 524ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key ✅ 523ms
Request Lifecycle.No Flags Request On Init Alone ✅ 518ms
Request Lifecycle.No Flags Request On Normal Capture ✅ 509ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests ✅ 527ms
Request Lifecycle.Mock Response Value Is Returned To Caller ✅ 525ms
Retry Behavior.Retries Flags On 502 ✅ 626ms
Retry Behavior.Retries Flags On 504 ✅ 625ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event ✅ 526ms

@github-actions

Copy link
Copy Markdown
Contributor

posthog-php-socket Compliance Report

Date: 2026-09-26T15:29:56.305985+00:00
Duration: 147247ms

⚠️ Some Tests Failed

36/47 tests passed, 11 failed


Capture Tests

⚠️ 19/30 tests passed, 11 failed

View Details
Test Status Duration
Format Validation.Event Has Required Fields ✅ 27ms
Format Validation.Event Has Uuid ✅ 522ms
Format Validation.Event Has Lib Properties ✅ 525ms
Format Validation.Distinct Id Is String ✅ 524ms
Format Validation.Token Is Present ✅ 523ms
Format Validation.Custom Properties Preserved ✅ 524ms
Format Validation.Event Has Timestamp ✅ 525ms
Format Validation.Non Utc Event Timestamp Is Converted To Utc ✅ 524ms
Retry Behavior.Retries On 503 ❌ 9232ms
Retry Behavior.Does Not Retry On 400 ✅ 2528ms
Retry Behavior.Does Not Retry On 401 ✅ 2526ms
Retry Behavior.Respects Retry After Header ❌ 9230ms
Retry Behavior.Implements Backoff ❌ 18734ms
Retry Behavior.Retries On 500 ❌ 8750ms
Retry Behavior.Retries On 502 ❌ 9236ms
Retry Behavior.Retries On 504 ❌ 9231ms
Retry Behavior.Max Retries Respected ❌ 18734ms
Deduplication.Generates Unique Uuids ✅ 47ms
Deduplication.Preserves Uuid On Retry ❌ 9233ms
Deduplication.Preserves Uuid And Timestamp On Retry ❌ 14242ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry ❌ 9233ms
Deduplication.No Duplicate Events In Batch ✅ 35ms
Deduplication.Different Events Have Different Uuids ✅ 525ms
Compression.Sends Gzip When Enabled ✅ 525ms
Batch Format.Uses Proper Batch Structure ✅ 524ms
Batch Format.Flush With No Events Sends Nothing ✅ 520ms
Batch Format.Multiple Events Batched Together ✅ 512ms
Error Handling.Does Not Retry On 403 ✅ 2526ms
Error Handling.Does Not Retry On 413 ✅ 2526ms
Error Handling.Retries On 408 ❌ 5528ms

Failures

retry_behavior.retries_on_503

Expected at least 3 requests, got 1

retry_behavior.respects_retry_after_header

Expected at least 2 requests, got 1

retry_behavior.implements_backoff

Expected at least 3 requests, got 1

retry_behavior.retries_on_500

Expected at least 2 requests, got 1

retry_behavior.retries_on_502

Expected at least 2 requests, got 1

retry_behavior.retries_on_504

Expected at least 2 requests, got 1

retry_behavior.max_retries_respected

Expected 4 requests, got 1

deduplication.preserves_uuid_on_retry

Need at least 2 requests to check retry

deduplication.preserves_uuid_and_timestamp_on_retry

Expected at least 3 requests, got 1

deduplication.preserves_uuid_and_timestamp_on_batch_retry

Expected at least 2 requests, got 1

error_handling.retries_on_408

Expected at least 2 requests, got 1

Feature_Flags Tests

✅ 17/17 tests passed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id ✅ 524ms
Request Payload.Flags Request Uses V2 Query Param ✅ 522ms
Request Payload.Flags Request Hits Flags Path Not Decide ✅ 522ms
Request Payload.Flags Request Omits Authorization Header ✅ 521ms
Request Payload.Token In Flags Body Matches Init ✅ 522ms
Request Payload.Groups Round Trip ✅ 523ms
Request Payload.Groups Default To Empty Object ✅ 522ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False ✅ 522ms
Request Payload.Disable Geoip Omitted Defaults To False ✅ 522ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key ✅ 521ms
Request Lifecycle.No Flags Request On Init Alone ✅ 517ms
Request Lifecycle.No Flags Request On Normal Capture ✅ 509ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests ✅ 525ms
Request Lifecycle.Mock Response Value Is Returned To Caller ✅ 523ms
Retry Behavior.Retries Flags On 502 ✅ 626ms
Retry Behavior.Retries Flags On 504 ✅ 623ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event ✅ 524ms

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant