Skip to content

test: strengthen assertions and cover Mix tasks - #218

Merged
marandaneto merged 2 commits into
mainfrom
test-audit
Sep 29, 2026
Merged

marandaneto merged 2 commits into
mainfrom
test-audit

Conversation

@marandaneto

@marandaneto marandaneto commented Sep 26, 2026 •

Copy link
Copy Markdown
Member

💡 Motivation and Context

Several tests could pass without proving the behavior named in the test. Partial map matches allowed context or metadata leaks. Feature-flag deduplication hid whether send_event: false worked. Loader tests did not verify that blocked request workers stopped, and a short polling interval could consume responses intended for manual refreshes.

This PR strengthens those assertions, checks exact variant/payload pairs, and uses message-processing barriers where a sleep or negative assertion was not enough. It also adds eight tests for the public API snapshot and source-packaging Mix tasks, fixes the manual capture example, and restores the original Logger configuration after manual tests.

The loader test explicitly waits for request-timeout cleanup before testing bounded shutdown and provider cleanup. It does not claim that shutdown immediately cancels an in-flight request. A 1-second request-timeout experiment confirmed that shutdown waits for that timeout.

There are no SDK implementation changes or deleted tests.

💚 How did you test it?

On Elixir 1.20.4 and OTP 29:

Suite Before After
SDK tests 600 passed 608 passed
SDK line coverage 85.94% 90.55%
Compliance adapter tests 6 passed 6 passed
Adapter line coverage 71.54% 71.54%

Coverage uses the unchanged Mix defaults, including Mix tasks and compiled test support. The coverage increase comes from the new Mix-task tests. Stronger assertions improve regression detection without changing runtime-module line coverage. Adapter coverage remains below Mix's default 90% threshold.

  • The full SDK suite passed with seeds 0 and 424242. All 18 live integration cases remained excluded. The changed manual integration examples were not run against live services.
  • Seven deliberate production mutations caused the intended tests to fail, covering conflicting task options, multiple source roots, variant overrides, silent flag reads, metadata filtering, source exclusions, and request-worker cleanup on timeout. All mutations were restored before final validation.
  • mix format --check-formatted, mix credo --strict, mix compile --warnings-as-errors, mix posthog.public_api --check, mix hex.build, and git diff --check passed.
  • Branch autoreview against origin/main reported no actionable findings at 363433584828dd1dec8167334ec3a3e1a94a431d.
  • Other Elixir/OTP versions and the external Docker compliance harness were not run locally. CI's external harness reported 40/47 passing. All seven failures match the limitations already documented in sdk_compliance_adapter/README.md.

📝 Checklist

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

If releasing new changes

  • Ran sampo add to generate a changeset file. Not needed for test-only changes.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Pi audited the tests in a dedicated worktree, using three read-only subagent audits, local shell tools, and the autoreview helper. The changes retain existing tests and strengthen their assertions rather than removing coverage. Local coverage and controlled mutations were used to check the result. No public session link is available.

The work was directed by @marandaneto and requires human review. No production behavior or coverage thresholds were changed. Live integration validation and some broader concurrency coverage remain follow-ups.

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

Copy link
Copy Markdown
Member Author

Test coverage comparison

Suite Before After Change
SDK line coverage 85.94% 90.55% +4.61 percentage points
SDK passing tests 600 608 +8
Adapter line coverage 71.54% 71.54% Unchanged
Adapter passing tests 6 6 Unchanged

Measured on Elixir 1.20.4 / OTP 29 with seed 0, comparing baseline 251e93d with the changes in 9b145cb. Coverage uses unchanged Mix defaults, including Mix tasks and compiled test support. No coverage exclusions or thresholds were changed.

The SDK coverage increase comes from eight new Mix-task tests. Strengthened existing assertions also detect regressions without increasing runtime-module line coverage. Seven deliberate production mutations were detected by the new or repaired tests, then fully restored.

The SDK now exceeds Mix’s default 90% coverage threshold. Adapter coverage remains below that threshold despite all six tests passing. The 18 live integration tests remained excluded before and after. Other Elixir/OTP versions were not measured locally.

@greptile-apps

greptile-apps Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Retrigger

[Low risk] Test suite improvements and assertion strengthening.

The PR appears safe to merge; no new actionable issue or outstanding finding was identified.

Reviews (2) · Last reviewed commit: "test: distinguish request timeout cleanu..."

@github-actions

github-actions Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

posthog-elixir Compliance Report

Date: 2026-09-27T14:26:11.947467+00:00
Duration: 108429ms

⚠️ Some Tests Failed

40/47 tests passed, 7 failed


Capture Tests

⚠️ 29/30 tests passed, 1 failed

View Details
Test Status Duration
Format Validation.Event Has Required Fields ✅ 608ms
Format Validation.Event Has Uuid ✅ 609ms
Format Validation.Event Has Lib Properties ✅ 609ms
Format Validation.Distinct Id Is String ✅ 609ms
Format Validation.Token Is Present ✅ 608ms
Format Validation.Custom Properties Preserved ✅ 608ms
Format Validation.Event Has Timestamp ✅ 608ms
Format Validation.Non Utc Event Timestamp Is Converted To Utc ❌ 609ms
Retry Behavior.Retries On 503 ✅ 5614ms
Retry Behavior.Does Not Retry On 400 ✅ 2611ms
Retry Behavior.Does Not Retry On 401 ✅ 2610ms
Retry Behavior.Respects Retry After Header ✅ 5614ms
Retry Behavior.Implements Backoff ✅ 15624ms
Retry Behavior.Retries On 500 ✅ 5615ms
Retry Behavior.Retries On 502 ✅ 5613ms
Retry Behavior.Retries On 504 ✅ 5615ms
Retry Behavior.Max Retries Respected ✅ 15624ms
Deduplication.Generates Unique Uuids ✅ 615ms
Deduplication.Preserves Uuid On Retry ✅ 5613ms
Deduplication.Preserves Uuid And Timestamp On Retry ✅ 10620ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry ✅ 5614ms
Deduplication.No Duplicate Events In Batch ✅ 613ms
Deduplication.Different Events Have Different Uuids ✅ 609ms
Compression.Sends Gzip When Enabled ✅ 607ms
Batch Format.Uses Proper Batch Structure ✅ 608ms
Batch Format.Flush With No Events Sends Nothing ✅ 606ms
Batch Format.Multiple Events Batched Together ✅ 611ms
Error Handling.Does Not Retry On 403 ✅ 2609ms
Error Handling.Does Not Retry On 413 ✅ 2611ms
Error Handling.Retries On 408 ✅ 5615ms

Failures

format_validation.non_utc_event_timestamp_is_converted_to_utc

Event 0 field 'timestamp' instant '2026-09-27T14:24:27.955157Z' != expected '2025-01-02T03:04:05Z'

Feature_Flags Tests

⚠️ 11/17 tests passed, 6 failed

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

Failures

request_payload.groups_default_to_empty_object

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

request_payload.disable_geoip_omitted_defaults_to_false

Field 'geoip_disable' not found in /flags request body at path 'geoip_disable'. Available keys: ['api_key', 'distinct_id', 'flag_keys_to_evaluate']

request_lifecycle.mock_response_value_is_returned_to_caller

Last action result missing field 'value'. Keys: ['error', 'success']

retry_behavior.retries_flags_on_502

Last action result missing field 'value'. Keys: ['error', 'success']

retry_behavior.retries_flags_on_504

Last action result missing field 'value'. Keys: ['error', 'success']

side_effect_events.get_feature_flag_captures_feature_flag_called_event

Expected 1 events with name '$feature_flag_called', got 0

Comment thread test/posthog/feature_flags/definition_loader_test.exs Outdated
@marandaneto
marandaneto marked this pull request as ready for review September 28, 2026 13:01
@marandaneto
marandaneto requested a review from a team as a code owner September 28, 2026 13:01
@marandaneto
marandaneto merged commit ed1f5bc into main Sep 29, 2026
32 of 33 checks passed
@marandaneto
marandaneto deleted the test-audit branch September 29, 2026 06:09
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.

2 participants