Skip to content

docs: add TESTING.md for python - #2961

Merged
opieter-aws merged 3 commits into
strands-agents:mainfrom
opieter-aws:opieter-aws/docs-testing
Jun 25, 2026
Merged

opieter-aws merged 3 commits into
strands-agents:mainfrom
opieter-aws:opieter-aws/docs-testing

Conversation

@opieter-aws

@opieter-aws opieter-aws commented Jun 25, 2026 •

Copy link
Copy Markdown
Contributor

Description

The TypeScript SDK has a thorough docs/TESTING.md that strands-ts/AGENTS.md hard-delegates to ("you MUST follow"). The Python SDK had no equivalent — its testing guidance was ~40 lines inside AGENTS.md and nothing in docs/. The practical cost is that real, reusable test infrastructure stays invisible: the shared fixtures (MockedModelProvider, MockHookProvider, MockAgentTool, MockedSessionRepository) and the conftest.py async/AWS helpers (agenerator, alist, generate, moto_*) aren't documented anywhere, so contributors hand-roll their own mocks instead of reusing them. A few sharp edges also went unwritten — strict-asyncio mode silently skips an unmarked coroutine test, and asserting a whole Message dict when a test only cares about role/text is a recurring source of brittle breakage in tests_integ/.

This adds strands-py/docs/TESTING.md as the authoritative Python testing reference, mirroring the TypeScript guide's depth and topic coverage so the two SDKs stay aligned. It documents the fixtures quick-reference (with "reuse these, don't hand-roll"), the tru_/exp_ assertion-pair naming, whole-object assertions with unittest.mock.ANY for volatile fields, the strict-asyncio marker requirement, and the flat-vs-class test-organization preference — all reflecting conventions the suite already follows rather than inventing new ones.

It also corrects a contradiction in strands-ts/docs/TESTING.md, which referenced a tests_integ/ directory that doesn't exist; the integration tests live in test/integ/ (as the same file states a few lines later). An agent following the authoritative doc would otherwise create files in the wrong place.

The companion AGENTS.md restructure that links to this new file is a separate PR; this one is split out so the doc it points at lands first.

Related Issues

None — maintenance change surfaced by a convention audit of both SDKs.

Documentation PR

Not applicable — these are contributor-facing docs/ files in the SDK packages, not user-facing documentation under site/.

Type of Change

Documentation update

Testing

Documentation-only. Verified that every fixture, helper, and path named in the new guide exists in the tree (tests/fixtures/*, the conftest.py session-scoped helpers, strict-asyncio being the default), and that the corrected test/integ/ path matches the real TypeScript integration-test directory.

  • I ran hatch run prepare

Checklist

  • I have read the CONTRIBUTING document
  • I have reviewed and understand every line of code in this PR, including any generated by AI tools, and I can explain why it works
  • My change is focused and reasonably small; I have split unrelated work into separate PRs
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@github-actions github-actions Bot added size/s chore Maintenance tasks, dependency updates, CI changes, refactoring with no user-facing impact area-community Related to community and contributor health labels Jun 25, 2026
@opieter-aws opieter-aws changed the title opieter aws/docs testing docs: add TESTING.md for python Jun 25, 2026
@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@opieter-aws
opieter-aws marked this pull request as ready for review June 25, 2026 13:39
Comment thread strands-py/docs/TESTING.md Resolved
@github-actions

Copy link
Copy Markdown
Contributor

Assessment: Approve

Documentation-only change that adds an authoritative Python TESTING.md mirroring the TypeScript guide, plus a one-line path correction in the TS doc. I verified every factual claim against the tree and they all hold up.

Verification details
  • Layout & paths: tests_integ/ and the src/strands/ ↔ tests/strands/ mirror both exist as described.
  • Fixtures table: all five classes (MockedModelProvider, MockHookProvider, MockMultiAgentHookProvider, MockAgentTool, MockedSessionRepository) exist at the listed paths with the stated names; MockedModelProvider does accept per-response Usage.
  • conftest helpers: agenerator, alist, generate, moto_env, moto_mock_aws all present; generate returns (events, return_value) as documented.
  • Strict asyncio: asyncio_mode is indeed unset in pyproject.toml, so the strict default holds.
  • Python 3.10–3.14: matches requires-python, classifiers, and the hatch matrix.
  • TS fix: test/integ/ is the real directory (the old tests_integ/ reference was stale), consistent with strands-ts/AGENTS.md.
  • Conventions: tru_/exp_, whole-object assertions, and unittest.mock.ANY masking are all genuinely used in the suite — the doc reflects existing practice rather than inventing it.

Only one non-blocking clarity nit inline (the two assertion rules can read as contradictory). Clear, accurate, and tightly scoped — nice work splitting the AGENTS.md restructure into a separate PR.

Comment thread strands-py/docs/TESTING.md Resolved
Comment thread strands-py/docs/TESTING.md Resolved
Comment thread strands-py/docs/TESTING.md Resolved
@github-actions

Copy link
Copy Markdown
Contributor

Assessment: Approve

Re-reviewed after the "Clarify wording" commit (2d7a743). The Assertions section now opens with "The right granularity depends on who controls the expected shape" and recasts the two rules as a primary rule plus a natural exception — which fully resolves the only point from my earlier review. The two directives no longer read as contradictory.

No other changes since the last pass; the TS path correction is unchanged and still accurate. Nothing outstanding — good to merge.

@github-actions

Copy link
Copy Markdown
Contributor

Assessment: Approve

Re-reviewed after the "Address comments" commit (95ef186), which responds to all three of @pgrayy's review comments:

How each comment was addressed
  • Drop the TS cross-reference / keep this Python-specific → The intro paragraph no longer mentions the TypeScript SDK; it now reads as a standalone Python reference. ✅
  • Test naming convention → A new bullet codifies test_<method>_<description> (with the test_<subject>_<method>_<description> variant for ambiguous modules) and the class-scope exception. I fact-checked both cited examples — test__init__default_model_id (in test_bedrock.py) and test_update_config_validation_warns_on_unknown_keys (across the model tests) — and they exist verbatim. The "most of the suite is flat" claim also holds: ~1856 flat test functions vs ~246 Test* classes. ✅
  • Centralize test rules / link from AGENTS.md → Author confirmed this is the follow-up PR docs: improve agent guidance #2959, deliberately split to keep this one small. ✅

One thing worth confirming with the reviewer: this Python doc intentionally keeps tests_integ/ (the real Python integ path), while the TS-doc fix in this same PR changes the TS path to test/integ/. Both are correct for their respective SDKs — just flagging since the two paths now differ across the two files.

Nothing blocking. Accurate, tightly scoped, and all reviewer feedback addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-community Related to community and contributor health chore Maintenance tasks, dependency updates, CI changes, refactoring with no user-facing impact size/s

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants