docs(skills): test-skill cleanups and gotchas surfaced from supadata PR#40
Merged
Merged
Conversation
…tures (writing-unit-tests) Test files in autohive-integrations don't need the heavy `importlib.util.spec_from_file_location` boilerplate when the integration source can be imported as a normal module. With a 2-line `sys.path.insert` in `tests/conftest.py`, plain `from <integration> import ...` works fine. The repo-wide `conftest.py` also already provides three fixtures every test should reuse instead of redeclaring locally: - `mock_context` — minimal ExecutionContext - `make_context` — factory for arbitrary auth shapes - `env_credentials` — `.env`-aware env-var lookup with skip support Changes: - Promote plain imports as the default boilerplate; keep the `importlib` form as a documented fallback for unusual layouts. - Replace the 'every file declares its own mock_context' guidance with the override-in-conftest pattern, so credential shapes are set once per integration and inherited by all tests. - Add a parametrize example to 'Testing Helper Functions' for collapsing tabular helper tests (e.g. ms_to_timestamp boundaries). - New 'Common Gotchas' entry: PyPI package name collision. When the integration folder name matches a PyPI package the source imports (e.g. supadata, dropbox), the empty `__init__.py` shadows the real package. The fix is to drop `__init__.py` — the validator treats it as optional, the Lambda runtime is unaffected, and tests no longer need `site.getsitepackages()` / `importlib` shims. Surfaced while reviewing Autohive-AI/autohive-integrations#280 (Supadata SDK 2.0 upgrade).
…DK-call variant (writing-integration-tests) Three updates aligned with the writing-unit-tests skill changes: 1. Replace the heavy `importlib` boilerplate with plain imports (matching the unit-tests skill). Cross-reference the unit-tests skill for the `importlib` fallback rather than duplicating it. 2. Promote `env_credentials` from the repo-wide `conftest.py` as the recommended way to handle API-key / token env vars. The fixture auto-loads `.env` and integrates with `pytest.skip` when a key is missing. Module-level `os.environ.get(...)` is still useful for object IDs (which env_credentials doesn't model directly), so the require_* helpers stay — but credentials should be read via env_credentials. 3. Add Variant 4: 'External Python SDK (no `context.fetch`)'. Some integrations (e.g. supadata) call a third-party Python SDK directly rather than going through `context.fetch`. For these, the aiohttp-wrapping fixture is irrelevant — the SDK does its own networking. The new variant is a 4-line fixture that just injects credentials via `make_context(auth=...)`. Updated the 'How to choose' guidance to a decision table covering all four variants by auth shape and networking pattern. Surfaced while reviewing Autohive-AI/autohive-integrations#280.
…sdk-v2) Two new entries in the 'Common Gotchas' section, both surfaced while reviewing Autohive-AI/autohive-integrations#280 (Supadata 1.0.0 → 2.0.0 upgrade) and the underlying production crash report in Autohive-AI/autohive-integrations#316: 1. Audit auth lookup defaults during the upgrade. Several 1.0.x integrations shipped with the wrong default *type* in their auth lookup, e.g.: context.auth.get('credentials', {}).get('api_key', {}) The `{}` default returns a dict when the field is empty, which crashes the upstream SDK that expects a string with TypeError — surfacing as a Lambda 500 / Raygun crash instead of the user-facing auth error it actually is. The 2.0.0 upgrade is the right moment to fix this because the auth path is being touched anyway to convert error returns to ActionError. 2. PyPI package name collision. When the integration folder is named after a PyPI package the source imports (e.g. `from supadata import Supadata` inside `supadata/`), the empty `__init__.py` shadows the real PyPI package and every test fails with ImportError. Drop the `__init__.py` — the validator treats it as optional, the Lambda runtime is unaffected, and tests can use plain imports. Cross-references the writing-unit-tests skill where the matching test-side gotcha now lives.
Coverage —
|
| File | Stmts | Miss | Cover | Missing |
|---|---|---|---|---|
src/autohive_integrations_sdk/__init__.py |
2 | 0 | 100% | |
src/autohive_integrations_sdk/integration.py |
358 | 0 | 100% |
![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: 5e1346b185
ℹ️ 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".
The new file-header boilerplate dropped `import os` (it's no longer needed once the importlib path-juggling is gone), but the 'Module-level constants' snippet a few sections later still uses `os.environ.get(...)`. Readers who copy both sections verbatim would hit `NameError` at import time. Add `import os` directly to the constants snippet and call out in prose that it should join the file-header imports. Keeps each snippet self-contained and executable. Caught by the codex review bot on PR #40.
NinosMan
approved these changes
May 4, 2026
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
2 participants
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.
Summary
Three skills get updates based on lessons learned while reviewing autohive-ai/autohive-integrations#280 (Supadata SDK 2.0 upgrade) and the production crash that prompted it (autohive-ai/autohive-integrations#316).
Each commit is independently reviewable.
Commits
b02b631writing-unit-testsimportlib.util.spec_from_file_locationas a documented fallback for unusual layouts. Most integrations don't need the heavy form whentests/conftest.pydoes a 2-linesys.path.insert.mock_context" guidance with the override-in-conftest pattern. The repo-wideconftest.pyalready providesmock_context,make_context, andenv_credentials— tests should reuse them.@pytest.mark.parametrizeexample to "Testing Helper Functions" for collapsing tabular helper tests.__init__.pyshadows the real package; the fix is to drop it.641e54bwriting-integration-testsimportlibfallback rather than duplicating it).env_credentialsfrom the repo-wideconftest.pyas the recommended way to handle API-key / token env vars; module-levelos.environ.getstays for object IDs whichenv_credentialsdoesn't model.context.fetch) — for integrations that call a third-party Python SDK directly rather thancontext.fetch. The aiohttp wrapper is irrelevant for these; the variant is a 4-linemake_context(auth=...)fixture. The "How to choose" guidance is now a decision table covering all four variants.5e1346bupgrading-sdk-v2Two new "Common Gotchas" entries:
.get("api_key", {})— a dict default for a string field. Empty credentials then crash the upstream SDK withTypeError, which surfaces as a Lambda 500 / crash report instead of a user-facing auth error. The 2.0.0 upgrade is the right moment to fix because the auth path is being touched anyway to addActionError.Why now
The supadata PR review made it clear that:
importlibboilerplate is overkill for the common case and obscures cleaner alternatives.mock_context/ env-var lookups instead of using the repo-wide fixtures because the skills don't mention them.{}auth default — is exactly the kind of thing a 2.0 upgrade should catch in passing, but the skill didn't flag it.Out of scope
autohive-integrationsto the simpler boilerplate. The skill change applies forward to new tests; existing files (perplexity, hubspot, bitly, etc.) all use the heavy form and that's fine — no need to churn them.__init__.pyshould be optional when the folder name collides with a PyPI dependency) — filed separately as autohive-ai/autohive-integrations-tooling#39.Test plan
The skills are markdown documentation; no code or tests are affected. Sanity-checked:
git diff --statshows only the three skills touched.pdocregen required per AGENTS.md.