fix(humanitix): upgrade SDK to v2.0.0, add unit and integration tests (#304)#306
Conversation
…n.load()
- Fix humanitix.py to use Integration.load("config.json") so the config
is resolved relative to cwd (required for multi-file integrations with
an actions/ sub-package, per SDK documentation)
- Delete the old manual test script (tests/test_humanitix.py)
- Add tests/conftest.py for pytest path resolution
- Add tests/test_humanitix_unit.py: 33 unit tests covering all 6 actions
(get_events, get_orders, get_tickets, get_tags, check_in, check_out)
using AsyncMock for context.fetch
- Add tests/test_humanitix_integration.py: live integration tests for
read-only actions (get_events, get_tags) that skip if HUMANITIX_API_KEY
is not set
… ActionError for errors
… use Integration.load()
🔍 Integration Validation ResultsCommit: Changed directories:
✅ Structure Check output✅ Code Check output✅ Tests Check output✅ README Check output✅ Version Check output |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e3a923360
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…to avoid AttributeError
|
@TheRealAgentK integration tests are blocked locally by Cloudflare, Humanitix's API returns a 403 from any server-side or datacenter IP. Unit tests all pass (32/32) and validation is clean, but we can't run live integration tests in CI or locally without a residential/browser IP. Wanted to flag this and get your thoughts on how you want to handle it before we move forward. |
|
Just to clarify - you're saying we couldn't run e-2-e CI from GH for instance, correct? That'd be ok because the integration tests currently don't run in CI. I don't understand what you mean with them being being "blocked locally"? You should be able to run them from your desktop computer, right? That's not a server-side or datacenter IP. |
|
Hey @TheRealAgentK — got to the bottom of it. Cloudflare checks the full TLS fingerprint on api.humanitix.com, not just headers. aiohttp uses a Python TLS stack which Cloudflare flags, regardless of what User-Agent you send. Switched the test fixture to use curl_cffi with Chrome impersonation — it replicates the full Chrome TLS handshake so Cloudflare lets it through. All 5 integration tests now pass locally:
No changes to the integration code itself, just the test fixture. |
…heck_in, check_out
|
Hey @TheRealAgentK, expanded integration tests to cover all 6 actions (was 2 before). Also switched the fixture from Integration test results, 5 passed, 6 skipped, 0 failed11 tests across all 6 actions. Skips are chained tests that need pre-existing events, the test account has no events so they skip gracefully. Run against an account with actual events and all 11 will pass.
|
TheRealAgentK
left a comment
TheRealAgentK
left a comment
There was a problem hiding this comment.
Local validation: 32/32 unit pass (95% cov), validate_integration.py clean, check_code.py clean. Couldn't run integration tests without a Humanitix key, but the curl_cffi switch looks correct.
A few items to address before merge:
1. os.chdir(_parent) at module level without restore — tests/test_humanitix_integration.py:25
os.chdir(_parent)
_spec = importlib.util.spec_from_file_location(...)Process-global side effect — leaves cwd inside humanitix/ for any test file collected after this one. Same pattern I flagged on #284. The unit test file at least save-and-restores; the integration file doesn't.
2. tests/conftest.py is a no-op — sys.path.insert(0, os.path.dirname(__file__))
That puts tests/ on sys.path, not the integration root. The actual import wiring is the importlib block inside each test file.
Pattern from #280 (supadata): conftest does the path setup once, test files import normally:
# tests/conftest.py
import os, sys
sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), "..")))# test_humanitix_unit.py
from humanitix import humanitixThe folder/entry-point name collision (humanitix/humanitix.py) doesn't break this — from humanitix import humanitix inside actions/*.py resolves via the partially-loaded module in sys.modules because humanitix = Integration.load() runs before import actions. This drops both the importlib block and the os.chdir calls.
3. Stale tests/context.py — delete
Pre-SDK-2.0 testbed shim (from humanitix import humanitix, dependencies/ path). With conftest fixed per #2 above, this file is unused. Same cleanup as supadata #280.
4. Output schemas still declare "result" (boolean) as required
"required": ["result"]Inconsistent with the established SDK 2.0 pattern (#280 supadata, #289 google-sheets, #307 coda all dropped "result" and "error" from output schemas). Errors now flow through ActionError, so the boolean is dead weight. Either drop it everywhere (preferred) or be explicit about why it's kept.
5. build_error_result is misleadingly named — helpers.py:58
Returns ActionError, not an ActionResult. Minor: rename to build_action_error or inline at the four call sites.
#1 is the only one that risks breaking other test runs; the rest are cleanup that brings this in line with the other SDK 2.0 PRs.
The previous conftest.py inserted the tests directory itself onto sys.path, which is a no-op for the intended purpose of making the integration's modules importable from test files. Insert the parent (integration root) instead, matching the pattern from PR #280 (supadata) and PR #289 (google-sheets).
With tests/conftest.py now putting the integration root on sys.path and the workspace conftest.py patching Integration.load to find config.json from the caller's directory, tests can import the integration with a plain 'from humanitix import humanitix'. Removes: - The importlib.util.spec_from_file_location dance in both test files. - The process-global os.chdir(_parent) calls (the integration test file did not restore cwd, leaving it inside humanitix/ for any test file collected after it). - The legacy tests/context.py shim from the pre-SDK-2.0 testbed. Matches the simpler pattern used in PR #280 (supadata), PR #289 (google-sheets), and PR #307 (coda).
Errors now flow through ActionError, so the boolean 'result' flag and the 'statusCode'/'error'/'message' error fields are dead weight in output schemas. Removes them from config.json and from the data dictionaries built by fetch_single_resource and build_paginated_result in helpers.py. Drops the corresponding 'assert data["result"] is True' lines from the unit tests. Brings humanitix in line with PR #280 (supadata), PR #289 (google-sheets), and PR #307 (coda).
The function returns an ActionError, not an ActionResult, so the '_result' suffix is misleading. Renames the helper and all four call sites in actions/.
…list error path Adds tests for previously untested optional params: - get_orders: override_location, page_size, since - get_tickets: override_location, event_date_id, page_size, since - get_tickets: api_error path on list endpoint (not just single) Also drops the dead 'if not hasattr(response, "status")' defensive branch in build_action_error — every caller passes a FetchResponse, so the branch is unreachable. Coverage: 95% -> 100% (32 -> 40 tests).
TheRealAgentK
left a comment
TheRealAgentK
left a comment
There was a problem hiding this comment.
Self-approved to remove the change request. Will need 3rd person reviews and then e2e testing.
With tests/conftest.py putting the integration root on sys.path and the workspace conftest.py patching Integration.load to find config.json, test files can now import the integration with a plain 'from app_business_reviews import app_business_reviews'. Removes the importlib.util.spec_from_file_location dance from both test files and the now-unused dependencies/ sys.path entry. Also deletes the legacy tests/context.py shim from the pre-SDK-2.0 testbed. Matches the simpler pattern used in PRs #280, #289, #306, #307.
The Humanitix API returns the event list shape directly (events, total,
page, pageSize) without a wrapping 'result' key. Drop the bogus
'result' assertion in test_returns_events_list and remove the
unreachable 'if data.get("result") is True:' guard in
test_has_pagination_fields so the pagination fields are actually
verified.
The Humanitix API returns the tag list shape directly (tags, total,
page, pageSize) without a wrapping 'result' key. Drop the bogus
'result' assertion in test_returns_tags_list and remove the
unreachable 'if data.get("result") is True:' guards in
test_tags_structure and test_pagination_params_respected so the
structural and pagination checks are actually executed.
…ts (#285) * feat(app-business-reviews): upgrade SDK to 2.0.0, add pytest unit tests - Pin autohive-integrations-sdk~=2.0.0 in requirements.txt - Add .data to all context.fetch() return accesses - Convert raise ValueError to return ActionError(message=...) - Return ActionResult(data=..., cost_usd=0.0) from all action handlers - Bump config.json version to 2.0.0 - Add tests/conftest.py and 40-test unit suite covering all 6 actions * fix(app-business-reviews): fix auth, optional params, add proper unit and integration tests - Fix api_key default from {} to "" in all action handlers - Delete old manual asyncio test file - Add test_app_business_reviews_integration.py with pytest.mark.integration, APP_BUSINESS_REVIEWS_API_KEY env var - Add APP_BUSINESS_REVIEWS_API_KEY to .env.example * fix(app-business-reviews): fix integration tests per skill guidelines * fix(app-business-reviews): fix ruff formatting issues * fix(app-business-reviews): fix ruff formatting * fix(app-business-reviews): reformat with line-length 120 to match tooling ruff config * fix(app-business-reviews): correct mock_context.auth shape in unit tests The source code reads context.auth.get("credentials", {}).get("api_key", ""), but the test fixture set ctx.auth = {"api_key": "..."}. As a result every action received api_key="" in unit tests. The mismatch was silent because no test asserted on params["api_key"]. Adds a regression assertion in TestSearchAppsIOS::test_request_params_include_term that verifies api_key is forwarded from the credentials block, so any future auth-shape drift will fail the suite immediately. * fix(app-business-reviews): point tests/conftest.py at integration root The previous conftest inserted the tests directory itself onto sys.path, which is a no-op for the intended purpose of making the integration's modules importable from test files. Insert the parent (integration root) instead, matching the pattern from PRs #280, #289, #306, and #307. * refactor(app-business-reviews): drop importlib boilerplate from tests With tests/conftest.py putting the integration root on sys.path and the workspace conftest.py patching Integration.load to find config.json, test files can now import the integration with a plain 'from app_business_reviews import app_business_reviews'. Removes the importlib.util.spec_from_file_location dance from both test files and the now-unused dependencies/ sys.path entry. Also deletes the legacy tests/context.py shim from the pre-SDK-2.0 testbed. Matches the simpler pattern used in PRs #280, #289, #306, #307. * refactor(app-business-reviews): remove response = None / 'is not None' guards Both get_reviews_google_play and get_reviews_google_maps tracked the last response object across the pagination loop just so the post-loop code could call response.data.get(...) for product/place info, with a defensive 'if response is not None' guard for the max_pages=0 case. Capture app_info / place_info inside the loop instead, initialized to an empty dict before the loop. Same behavior, no defensive guard, no loop-leaked variable. * refactor(app-business-reviews): restore post-loop app/place_info extraction Reverts the in-loop assignment introduced in c1455c7. The original code extracted product_info / place_info from the response variable once after the pagination loop, which is the more idiomatic shape (one read, one write). Keeps the sentinel-and-guard pattern that c1455c7 was meant to clean up, but adds a comment explaining why the guard is there (max_pages=0 edge case where the loop never executes). --------- Co-authored-by: Shubhank <72601061+Sagsgit@users.noreply.github.com> Co-authored-by: Kai Koenig <kai@ventego-creative.co.nz>
Live Integration Test Results5 passed, 5 skipped against the real Humanitix API. Full results (5 passed, 5 skipped)
Notes:
|
…nt in integration tests
3 participants
Summary
context.auth.get("credentials", {}).get("api_key", "")inputs.get(), required params useinputs["param"]response.data/response.statusfromFetchResponseActionError(message=...)on errorsTest Results
Unit Tests — 32/32 passed
32 passed ✅
Integration Tests (live Humanitix API) — 5 passed, 6 skipped, 0 failed
11 tests covering all 6 actions. Skipped tests are chained tests that require pre-existing events in the account — they skip gracefully when the account has no events, not a failure.
The test fixture uses
curl_cffiwith Chrome impersonation to bypass Cloudflare's TLS fingerprint check — plainaiohttpgets blocked with a 403 regardless of IP.test_returns_events_listget_eventstest_has_pagination_fieldsget_eventstest_returns_tags_listget_tagstest_tags_structureget_tagstest_pagination_params_respectedget_tagsTestGetOrders(2 tests)get_ordersTestGetTickets(3 tests)get_ticketsTestCheckInOutcheck_in/check_outValidation
validate_integration.pycheck_code.pyHow to run