Skip to content

test: strengthen SDK assertions and run all test suites - #816

Draft
marandaneto wants to merge 2 commits into
mainfrom
audit/all-tests-20260627
Draft

marandaneto wants to merge 2 commits into
mainfrom
audit/all-tests-20260627

Conversation

@marandaneto

@marandaneto marandaneto commented Sep 26, 2026 •

Copy link
Copy Markdown
Member

💡 Motivation and Context

Several tests could pass without exercising the behavior named in the test. Some checked an unattached queue, stopped before the last batch was sent, used fixtures that skipped the intended request, or tested a private copy of a survey algorithm. make test also omitted core, server and plugin suites.

This PR strengthens those tests without changing production SDK code:

  • Assert complete request streams, exact payloads, persisted state and independent guard conditions.
  • Replace simulated survey matching and branching tests with tests of the Android survey integration. Keep core model compatibility tests.
  • Propagate background test failures, clear queues before shutting down their executor, close owned resources and restore shared state after tests.
  • Make fake preferences stage edits until commit or apply.
  • Add a runtime test with Compose removed from the classpath, and run it from check and make test.
  • Run core, server, Android, Compose and plugin suites through make test, including the plugin's two-AGP functional matrix. Enable library debug tests with -PenableDebugTests=true instead of overriding CI, so signing and sample-plugin settings keep their CI behavior.

No changeset or package release is needed because this changes tests and test configuration only. Local audit logs and coverage reports are not part of the commit. The coverage comparison is posted as a separate PR comment.

💚 How did you test it?

Validated locally with Java 17.0.18-amzn:

  • CI=true make test, CI=true make testSurveyUI and make checkFormat: passed. Module XML reports contain 3,154 passing invocations and six existing benchmark skips, counting debug and release separately.
  • Fresh Android debug, Compose debug and Compose-free executions with CI=true: passed.
  • A Gradle configuration check confirmed that CI=true is preserved, signing stays required, the sample upload plugin stays disabled, and debug tests are included.
  • Added a queue-cleanup assertion that failed before the fix. The complete queue and session suites pass after the fix.
  • The original audit commit also passed make compile, make testReport, koverXmlReport and supplemental Compose/plugin coverage reports. Coverage in the separate comment is measured at that original commit.
  • Targeted production mutations proved that representative repaired tests detect missing guards, duplicate events, stale push markers, lost callbacks and incorrect survey branching. All mutations were restored.
  • Server concurrency cases passed three repeat runs. Android focused tests and the Compose-free runtime task passed.
  • Isolated autoreview of 1923463f21629a52d826771e2f6a60f4f90ed958 against origin/main: no findings.

No device/emulator tests, live-service acceptance tests or manual benchmarks were run. Plugin coverage does not measure execution inside nested TestKit Gradle processes.

📝 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 changeset to generate a changeset file (not applicable to this test-only PR)

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Prepared with Pi, delegated module audits, shell/Gradle tools and isolated autoreview. The audit prioritized tests that observe actual SDK behavior over test counts. Survey algorithm checks were moved to the Android implementation rather than preserving private simulations in core tests. No production behavior or public API was changed. Human review is required.

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

Copy link
Copy Markdown
Member Author

Test coverage before and after

Compared baseline 3bb72588df94f27cd6d667d48fc8a712ed597dd6 with the changes in 0923306f6f8d9272b4a55bee70ba06f3c993103e, using Java 17.0.18-amzn and Kover 0.9.9 for both measurements.

Module Line before Line after Branch before Branch after
Core 85.07% 85.00% 70.09% 70.09%
Android 84.94% 86.20% 67.26% 68.80%
Server 91.00% 91.04% 80.88% 81.26%
Compose surveys 59.84% 59.84% 57.91% 57.91%
Gradle plugin 11.31% 11.92% 10.19% 10.83%

Across the reported modules, covered lines increased from 11,837/14,632 (80.90%) to 11,890/14,632 (81.26%). Covered branches increased from 6,495/9,579 (67.80%) to 6,554/9,579 (68.42%). These totals sum module-local counters, not a combined measurement of every process and dependency.

Measurement details

  • Both sides exclude the test-only PostHogFake and NativeSymbolsUploadFunctionalTest classes, including nested classes. Default Kover reports include these custom source sets. Their class counters were subtracted from both saved XML reports so adding test code does not inflate production coverage. Generated and inlined production bytecode remains included.
  • The unadjusted Kover line results were 84.02% to 83.95% for core and 29.03% to 29.86% for the plugin. The table above removes the test-only sources consistently on both sides.
  • Core's small decrease is associated with replacing private survey simulations with tests of the Android survey integration. Those model constructors are now exercised by the Android tests rather than the removed core cases. The replacement branching and selection tests also detected controlled production defects.
  • Plugin coverage measures the instrumented test JVM, not plugin execution inside nested Gradle TestKit processes. All 14 functional cases passed, but their nested plugin execution is not reflected in this percentage.
  • Compose and plugin coverage used a temporary init script applying the same pinned Kover version. No coverage dependency was added to published artifacts.

Test results

Module Passing invocations Skipped
Core 1,008 0
Android, including the Compose-free task 1,492 6
Server 559 0
Compose surveys 75 0
Plugin unit and functional tests 20 0
Total 3,154 6

Debug and release variants count separately. The six skips are three existing manual benchmarks in each Android variant. There were no failures or errors. make test, make compile and make checkFormat passed. No device/emulator, external compliance or live-service tests were run.

Raw XML, HTML where generated, logs and the counter-comparison script are retained locally outside the commit. Stronger failure detection is the main outcome of this audit; these coverage percentages alone do not prove test quality.

@github-actions

github-actions Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor
⚠️ 5 packages modified but this PR has no changeset

This is informational — the PR is not blocked. Click the triangle above to collapse, or push a fix and this comment will auto-delete.

Modified in this PR but no changeset added:

  • posthog
  • posthog-android
  • posthog-android-gradle-plugin
  • posthog-android-surveys-compose
  • posthog-server

If this change should ship, run pnpm changeset and select a bump level.
If it isn't user-facing (refactor with no behavior change, internal tooling, generated files), no action needed.

@greptile-apps

greptile-apps Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Retrigger

[Low risk] Test suite improvements and assertion strengthening.

The PR appears safe to merge, with non-blocking test-configuration and cleanup issues to address.

Reviews (1) · Last reviewed commit: "test: strengthen SDK assertions and run ..."

Comment thread Makefile Outdated
Comment thread posthog/src/test/java/com/posthog/internal/PostHogQueueTest.kt
@github-actions

github-actions Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

posthog-android Compliance Report

Date: 2026-09-27 14:41:43 UTC
Duration: 118541ms

✅ All Tests Passed!

46/46 tests passed


Capture Tests

✅ 29/29 tests passed

View Details
Test Status Duration
Format Validation.Event Has Required Fields ✅ 413ms
Format Validation.Event Has Uuid ✅ 51ms
Format Validation.Event Has Lib Properties ✅ 28ms
Format Validation.Distinct Id Is String ✅ 31ms
Format Validation.Token Is Present ✅ 27ms
Format Validation.Custom Properties Preserved ✅ 29ms
Format Validation.Event Has Timestamp ✅ 28ms
Retry Behavior.Retries On 503 ✅ 7027ms
Retry Behavior.Does Not Retry On 400 ✅ 4024ms
Retry Behavior.Does Not Retry On 401 ✅ 4028ms
Retry Behavior.Respects Retry After Header ✅ 7028ms
Retry Behavior.Implements Backoff ✅ 17034ms
Retry Behavior.Retries On 500 ✅ 7020ms
Retry Behavior.Retries On 502 ✅ 7020ms
Retry Behavior.Retries On 504 ✅ 7019ms
Retry Behavior.Max Retries Respected ✅ 17038ms
Deduplication.Generates Unique Uuids ✅ 50ms
Deduplication.Preserves Uuid On Retry ✅ 7017ms
Deduplication.Preserves Uuid And Timestamp On Retry ✅ 12025ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry ✅ 7019ms
Deduplication.No Duplicate Events In Batch ✅ 40ms
Deduplication.Different Events Have Different Uuids ✅ 28ms
Compression.Sends Gzip When Enabled ✅ 21ms
Batch Format.Uses Proper Batch Structure ✅ 20ms
Batch Format.Flush With No Events Sends Nothing ✅ 12ms
Batch Format.Multiple Events Batched Together ✅ 38ms
Error Handling.Does Not Retry On 403 ✅ 4022ms
Error Handling.Does Not Retry On 413 ✅ 4021ms
Error Handling.Retries On 408 ✅ 5027ms

Feature_Flags Tests

✅ 17/17 tests passed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id ✅ 36ms
Request Payload.Flags Request Uses V2 Query Param ✅ 29ms
Request Payload.Flags Request Hits Flags Path Not Decide ✅ 23ms
Request Payload.Flags Request Omits Authorization Header ✅ 22ms
Request Payload.Token In Flags Body Matches Init ✅ 35ms
Request Payload.Groups Round Trip ✅ 44ms
Request Payload.Groups Default To Empty Object ✅ 22ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False ✅ 26ms
Request Payload.Disable Geoip Omitted Defaults To False ✅ 19ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key ✅ 21ms
Request Lifecycle.No Flags Request On Init Alone ✅ 10ms
Request Lifecycle.No Flags Request On Normal Capture ✅ 20ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests ✅ 39ms
Request Lifecycle.Mock Response Value Is Returned To Caller ✅ 27ms
Retry Behavior.Retries Flags On 502 ✅ 325ms
Retry Behavior.Retries Flags On 504 ✅ 323ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event ✅ 23ms

@marandaneto

Copy link
Copy Markdown
Member Author

Removed the empty changeset in 1923463. This PR only changes tests and test configuration, so it should not bump or release any SDK package. The changeset-hygiene warning does not require adding package releases. The PR description now reflects this and the CI-preserving test commands. Autoreview of 1923463 against origin/main reported no findings.

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.

2 participants