diff --git a/skills/upgrading-sdk-v2/SKILL.md b/skills/upgrading-sdk-v2/SKILL.md index 23081b1..61b2d84 100644 --- a/skills/upgrading-sdk-v2/SKILL.md +++ b/skills/upgrading-sdk-v2/SKILL.md @@ -285,3 +285,17 @@ Before considering an integration upgraded, verify: 7. **CI fetch pattern linter false positives**: The linter flags any variable named `response` accessed with `.get()` or `["..."]`. If a helper already unwraps `.data` and returns a plain dict, rename the variable in callers to avoid the match (e.g. `gql_result`, `body`, `api_data`). 8. **Ruff config mismatch**: CI uses `../autohive-integrations-tooling/ruff.toml` with `line-length = 120`. Always pass `--config` when formatting or local results will differ from CI. + +9. **Audit auth lookup defaults during the upgrade**: 1.0.x integrations sometimes 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 unset, which then crashes the upstream API client that expects a string — surfacing as a Lambda 500 / Raygun crash rather than the user-facing auth error it actually is. Fix to a string default that matches the field type: + + ```python + # Before — returns {} on missing field, crashes upstream SDK with TypeError + api_key = context.auth.get("credentials", {}).get("api_key", {}) + + # After — returns "" on missing field; upstream auth error becomes ActionError cleanly + api_key = context.auth.get("credentials", {}).get("api_key", "") + ``` + + The 2.0.0 upgrade is the right time to catch this because you're already touching the auth path to convert error returns to `ActionError`. + +10. **PyPI package name collision**: If your integration folder name matches a PyPI package the source imports (e.g. an integration in `supadata/` that does `from supadata import Supadata`), an empty `/__init__.py` will shadow the real PyPI package and every test fails with `ImportError`. Drop `/__init__.py` — the validator's "missing __init__.py" warning is correct to ignore in this case, and the Lambda runtime is unaffected (the entry point is the action source file, not the package). See the `writing-unit-tests` skill for the matching test-side guidance. diff --git a/skills/writing-integration-tests/SKILL.md b/skills/writing-integration-tests/SKILL.md index 80186f2..9bc1bcb 100644 --- a/skills/writing-integration-tests/SKILL.md +++ b/skills/writing-integration-tests/SKILL.md @@ -49,7 +49,7 @@ This double exclusion ensures integration tests never run accidentally. ### File Header (boilerplate) -Every integration test file must start with this exact boilerplate. Replace `myintegration` with the actual integration name: +With `sys.path` set up in `tests/conftest.py` (see the unit tests skill for the standard shape), an integration test file can use plain imports: ```python """ @@ -65,42 +65,49 @@ Never runs in CI — the default pytest marker filter (-m unit) excludes these, and the file naming (test_*_integration.py) is not matched by python_files. """ -import os -import sys -import importlib +import pytest +from unittest.mock import AsyncMock, MagicMock +from autohive_integrations_sdk import FetchResponse +from autohive_integrations_sdk.integration import ResultType + +from myintegration import myintegration -_parent = os.path.abspath(os.path.join(os.path.dirname(__file__), "..")) -_deps = os.path.abspath(os.path.join(os.path.dirname(__file__), "../dependencies")) -sys.path.insert(0, _parent) -sys.path.insert(0, _deps) +pytestmark = pytest.mark.integration +``` -import pytest # noqa: E402 -from unittest.mock import MagicMock, AsyncMock # noqa: E402 -from autohive_integrations_sdk import FetchResponse # noqa: E402 +Use the `importlib` fallback boilerplate from the unit tests skill only when plain imports won't work for the integration's layout. -_spec = importlib.util.spec_from_file_location("myintegration_mod", os.path.join(_parent, "myintegration.py")) -_mod = importlib.util.module_from_spec(_spec) -_spec.loader.exec_module(_mod) +## Environment Variables -myintegration = _mod.myintegration # the Integration instance +### env_credentials Fixture (preferred) -pytestmark = pytest.mark.integration +The repo-wide [`conftest.py`](https://github.com/Autohive-AI/autohive-integrations/blob/master/conftest.py) provides an `env_credentials` fixture that auto-loads the project `.env` and reads variables on demand: + +```python +@pytest.fixture +def live_context(env_credentials, make_context): + api_key = env_credentials("MYINTEGRATION_API_KEY") + if not api_key: + pytest.skip("MYINTEGRATION_API_KEY not set — skipping integration tests") + return make_context(auth={"credentials": {"api_key": api_key}}) ``` -## Environment Variables +This is the recommended pattern for any test that needs API credentials: it skips automatically when the env var is missing, integrates with the project `.env`, and avoids per-test boilerplate. -### Token and ID Setup +### Module-level constants (when you need them everywhere) -Define environment variables at module level. Use `os.environ.get` with an empty string default: +If multiple fixtures or helpers need the same env var, define them at module level — but still drive skip behaviour off `env_credentials` inside the fixture, not at import time. Add `import os` to the file header alongside the other top-level imports: ```python -ACCESS_TOKEN = os.environ.get("MYINTEGRATION_ACCESS_TOKEN", "") +import os + TEST_ITEM_ID = os.environ.get("MYINTEGRATION_TEST_ITEM_ID", "") +TEST_PROJECT_ID = os.environ.get("MYINTEGRATION_TEST_PROJECT_ID", "") ``` ### require_* Skip Helpers -For tests that need specific object IDs, create `require_*` helpers that skip gracefully: +For tests that need specific object IDs (which `env_credentials` doesn't model directly), create `require_*` helpers that skip gracefully: ```python def require_item_id(): @@ -215,7 +222,29 @@ def live_context(): return ctx ``` -**How to choose**: Check the integration's `config.json` — if `auth.type` is `"platform"`, use Variant 3. If the action handler reads an API key from `context.auth` or env vars and sets headers manually, use Variant 2. If no auth is needed, use Variant 1. +### Variant 4: External Python SDK (no `context.fetch`) + +Some integrations don't go through `context.fetch` at all — instead they instantiate a third-party Python SDK and let it make the HTTP calls (e.g. `from supadata import Supadata`). For these, the `aiohttp` wrapper is irrelevant: the SDK does its own networking. Just inject the credentials via `make_context` and the upstream library handles the rest: + +```python +@pytest.fixture +def live_context(env_credentials, make_context): + api_key = env_credentials("MYINTEGRATION_API_KEY") + if not api_key: + pytest.skip("MYINTEGRATION_API_KEY not set — skipping integration tests") + return make_context(auth={"credentials": {"api_key": api_key}}) +``` + +This variant is the simplest of the four — no `real_fetch` definition needed. Use it whenever the integration's source imports a vendor SDK and calls it directly rather than calling `context.fetch`. + +**How to choose**: + +| Auth shape | Networking | Variant | +|---|---|---| +| None (public API) | `context.fetch` | 1 — No auth | +| API key in `context.auth` or env | `context.fetch` | 2 — API key | +| Platform OAuth (`config.auth.type == "platform"`) | `context.fetch` | 3 — Platform OAuth | +| Any | External Python SDK call | 4 — External SDK | ## The Destructive Marker diff --git a/skills/writing-unit-tests/SKILL.md b/skills/writing-unit-tests/SKILL.md index 035ead2..214ab64 100644 --- a/skills/writing-unit-tests/SKILL.md +++ b/skills/writing-unit-tests/SKILL.md @@ -42,21 +42,55 @@ hubspot/tests/ ### conftest.py -Every `tests/` directory should include a `conftest.py` with this standard content: +Every `tests/` directory should include a `conftest.py`. At minimum it puts the integration source on `sys.path` so test files can use plain imports: ```python -import sys import os +import sys + +# Make .py importable as a top-level module. +sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))) +``` + +This lets test files do `from myintegration import myintegration` (or `from myintegration_module import ...`) without per-file boilerplate. -# Allow 'from context import ...' to work when pytest runs from repo root -sys.path.insert(0, os.path.dirname(__file__)) +If the integration reads credentials from `context.auth`, also override the repo-wide `mock_context` fixture here so every test in this directory inherits credentials of the right shape: + +```python +from unittest.mock import AsyncMock, MagicMock +import pytest + +@pytest.fixture +def mock_context(): + """Mock ExecutionContext pre-loaded with this integration's credentials.""" + ctx = MagicMock(name="ExecutionContext") + ctx.fetch = AsyncMock(name="fetch") + ctx.auth = {"credentials": {"api_key": "test_api_key"}} # nosec B105 + return ctx ``` The `_unit.py` suffix is required — CI uses it to discover unit tests. ### File Header (boilerplate) -Every test file must start with this exact boilerplate. Replace `myintegration` with the actual integration name and `myintegration.py` with the actual entry point file: +Every test file starts with the same shape: imports, an `pytestmark = pytest.mark.unit` line, and the integration imports. With `sys.path` set up in `tests/conftest.py` (above), test files can use plain imports: + +```python +import pytest +from unittest.mock import AsyncMock, MagicMock, patch +from autohive_integrations_sdk import FetchResponse +from autohive_integrations_sdk.integration import ResultType + +from myintegration import myintegration +# Also import any helpers you want to test directly: +# from myintegration import parse_response, my_helper + +pytestmark = pytest.mark.unit +``` + +#### Fallback boilerplate (only when plain imports won't work) + +If the integration source can't be imported as a normal module — for example when the file lives at an unusual path or has been renamed away from the integration's folder name — use the explicit `importlib` loader: ```python import os @@ -77,31 +111,31 @@ _spec = importlib.util.spec_from_file_location("myintegration_mod", os.path.join _mod = importlib.util.module_from_spec(_spec) _spec.loader.exec_module(_mod) -myintegration = _mod.myintegration # the Integration instance -# Also import any helper functions you need to test directly: -# parse_response = _mod.parse_response -# my_helper = _mod.my_helper +myintegration = _mod.myintegration pytestmark = pytest.mark.unit ``` -Add `from unittest.mock import patch` if you need to patch `asyncio.sleep` or environment variables. +Prefer plain imports — reach for the `importlib` form only when there's a concrete reason it's needed. ### mock_context Fixture -Every test file needs this fixture: +The repo-wide [`conftest.py`](https://github.com/Autohive-AI/autohive-integrations/blob/master/conftest.py) already provides three fixtures every test can use: + +- `mock_context` — minimal `ExecutionContext` with `ctx.auth = {}` and `ctx.fetch` as an `AsyncMock` +- `make_context` — factory for building a context with arbitrary `auth=...` +- `env_credentials` — helper that reads env vars (with `.env` autoloaded), returns `None` if missing + +You don't need to redeclare these. Override `mock_context` only when the integration reads credentials from `context.auth` and you want every test to inherit the shape — see the snippet in the conftest.py section above. + +For one-off credential shapes inside a single test, use `make_context`: ```python -@pytest.fixture -def mock_context(): - ctx = MagicMock(name="ExecutionContext") - ctx.fetch = AsyncMock(name="fetch") - ctx.auth = {} - return ctx +async def test_with_custom_auth(make_context): + ctx = make_context(auth={"credentials": {"api_key": "different_key"}}) # nosec B105 + ... ``` -If the integration reads credentials from `context.auth`, populate it to match the auth shape in `config.json`. - **Platform OAuth** (`config.auth.type == "platform"`): The SDK wraps OAuth tokens in a standard envelope: ```python @@ -361,6 +395,23 @@ class TestParseResponse: assert result == {"key": "value"} ``` +For helpers with several near-identical input/output cases, prefer `@pytest.mark.parametrize` over a separate test method per case — same coverage, easier to extend with new boundary rows: + +```python +class TestMsToTimestamp: + @pytest.mark.parametrize( + "milliseconds, expected", + [ + (0, "00:00:00,000"), + (1000, "00:00:01,000"), + (60_000, "00:01:00,000"), + (3_600_000, "01:00:00,000"), + ], + ) + def test_ms_to_timestamp(self, milliseconds: int, expected: str): + assert ms_to_timestamp(milliseconds) == expected +``` + ## Test Organization ### One class per action @@ -448,6 +499,8 @@ Every action should have at minimum: 7. **Unused variables**: If you call `execute_action` only to verify `mock_context.fetch.call_args`, don't assign the result to a variable — ruff will flag it as unused. Use `await integration.execute_action(...)` without assignment. +8. **PyPI package name collision**: If your integration folder is named after a PyPI package the source imports (e.g. an integration in `supadata/` that does `from supadata import Supadata`), an empty `/__init__.py` will shadow the real PyPI package — every test fails with `ImportError`. The fix is to **delete `/__init__.py`**: the validator treats it as optional, the Lambda runtime is unaffected, and `from import ...` then resolves cleanly to site-packages. Don't paper over the shadow with `site.getsitepackages()` / `importlib` shims in the test files. + ## Reference Implementations Look at these integrations for well-tested examples: