test: Strengthen assertions and isolate external HTTP calls - #985
Draft
marandaneto wants to merge 1 commit into
Draft
marandaneto wants to merge 1 commit into
marandaneto wants to merge 1 commit into
Conversation
Member
Author
Test coverage comparisonMeasured on Python 3.13.13 with the same branch-coverage command before and after. Coverage includes runtime code under
The six baseline failures involved external HTTP dependencies under the network guard. The repaired tests use controlled transport responses instead. The main improvement is stronger assertions and test isolation, not broader execution coverage. As one check, changing the tracing default flush interval to 50 seconds in memory passed the old defaults test but failed the repaired test. These numbers cover the root suite only. Separate validation passed for OpenFeature (68 tests), Django middleware and exception capture (7 tests), and the compliance adapter (23 tests). The comprehensive semantic audit remains incomplete. Live credential-dependent tests, MCP v2, and the full Python-version matrix were not run. |
Contributor
posthog-python Compliance ReportDate: 2026-09-26T15:30:39.173191+00:00 ✅ All Tests Passed!121/121 tests passed Capture_V1 Tests✅ 95/95 tests passed View Details
Capture_Ai Tests✅ 5/5 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
Feature_Flags_Local_Evaluation Tests✅ 4/4 tests passed View Details
|
Contributor
|
[Low risk] Test improvements and assertion strengthening across test files. The PR appears safe to merge, with non-blocking gaps in tests intended to protect timing and registry coverage. Reviews (1) · Last reviewed commit: "test: Strengthen assertions and isolate ..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| delivered.clear() | ||
| event = _track_event("python event %d" % i) | ||
| q.put(event) | ||
| self.assertTrue(delivered.wait(5)) |
Contributor
There was a problem hiding this comment.
This test waits up to five seconds for each event but never checks that delivery waited for the configured 0.3-second interval. It would still pass if partial batches flushed immediately, so it cannot catch an early-flush regression. Use a controlled clock or check that no request occurs before the interval.
Knowledge Base Used: Event capture and delivery
Prompt To Fix With AI
This is a comment left during a code review.
Path: posthog/test/test_consumer.py
Line: 259
Comment:
**Flush timing goes unchecked**
This test waits up to five seconds for each event but never checks that delivery waited for the configured 0.3-second interval. It would still pass if partial batches flushed immediately, so it cannot catch an early-flush regression. Use a controlled clock or check that no request occurs before the interval.
**Knowledge Base Used:** [Event capture and delivery](https://app.greptile.com/posthog-org-19734/-/custom-context/knowledge-base/posthog/posthog-python/-/docs/event-capture-and-delivery.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+61
to
+62
| for resource_name in resource_types: | ||
| wrapper_type = wrappers[resource_name] |
Contributor
There was a problem hiding this comment.
The test now iterates over the separate resource_types map rather than the wrapper registry used by production code. If a wrapper is added without a matching test-resource entry, this test silently skips it. Iterate over wrappers.items() and use resource_types to check each original type, or assert that the maps have the same keys.
Knowledge Base Used: AI provider integrations
Prompt To Fix With AI
This is a comment left during a code review.
Path: posthog/test/ai/openai/test_resource_wrapping.py
Line: 61-62
Comment:
**New wrappers can go untested**
The test now iterates over the separate `resource_types` map rather than the wrapper registry used by production code. If a wrapper is added without a matching test-resource entry, this test silently skips it. Iterate over `wrappers.items()` and use `resource_types` to check each original type, or assert that the maps have the same keys.
**Knowledge Base Used:** [AI provider integrations](https://app.greptile.com/posthog-org-19734/-/custom-context/knowledge-base/posthog/posthog-python/-/docs/ai-provider-integrations.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+27
to
+33
| obj = SimpleNamespace( | ||
| path="posthog.version.VERSION", annotation=None, value='"7.19.1"' | ||
| ) | ||
| assert ( | ||
| check_public_api._attribute_details(obj) | ||
| == "posthog.version.VERSION = <version>" | ||
| ) |
Contributor
There was a problem hiding this comment.
The hardcoded VERSION case replaces a loop over every entry in ATTRIBUTE_VALUE_PLACEHOLDERS. A future placeholder could stop working without this test noticing. Keep the per-entry check and add the non-placeholder case separately.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/scripts/test_check_public_api.py
Line: 27-33
Comment:
**Placeholder coverage no longer scales**
The hardcoded `VERSION` case replaces a loop over every entry in `ATTRIBUTE_VALUE_PLACEHOLDERS`. A future placeholder could stop working without this test noticing. Keep the per-entry check and add the non-placeholder case separately.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
💡 Motivation and Context
Several tests could pass without exercising the behavior they claimed to protect. Some also depended on external HTTP services, scheduler timing, or assertions raised inside worker callbacks that catch exceptions.
This test-only change replaces those HTTP calls with controlled responses and strengthens checks for retry counts, event payloads, exception propagation, truncation, copy isolation, and lifecycle behavior. Consumer tests now wait for delivery and stop their workers. LangChain error tests still exercise the real callback lifecycle through a mocked HTTP transport.
These are the validated repairs from a partial test audit. This PR does not claim that every repository test has been reviewed. No production code or public API changes are included.
💚 How did you test it?
origin/mainreported no actionable findings for107b3fb.Validation used Python 3.13.13. Live credential-dependent tests, MCP v2, and the full Python-version matrix were not run.
📝 Checklist
If releasing new changes
sampo addto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Pi used read-only subagents for test inventories and review, then applied and validated focused test repairs in a dedicated worktree. Tools included Git, pytest, coverage.py, Ruff, mypy, and the autoreview helper. The audit remains incomplete, and unresolved review scope was not treated as approved. Local audit notes are kept outside the PR. No shared session link is available.