docs: expand TEA documentation with cheat sheets, MCP enhancements, a… #1289
Conversation
…nd API testing patterns
📝 WalkthroughWalkthroughThis PR significantly expands test architecture documentation to emphasize API-first and backend-first testing approaches. It introduces comprehensive API testing patterns, enhances utility documentation with additional examples and API references, updates the TEA agent persona to include API testing scope, and adds Playwright MCP enhancement guidance throughout. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/modules/bmm/testarch/knowledge/intercept-network-call.md (1)
318-325: Fix glob pattern example for query parameter matching.Line 324 shows
'**/api/users?id=*', but in picomatch?matches exactly one character (not a literal question mark). This pattern would match/api/usersx id=*, not/api/users?id=123. Either escape the question mark as'**/api/users\?id=*'or use a simpler pattern like'**/api/users*'to match any query parameters.src/modules/bmm/testarch/knowledge/file-utils.md (1)
126-139: PDF reader return value structure is inconsistent across examples and undocumented.Example 3 (line 136-138) accesses:
pdfResult.pagesCountandpdfResult.fileNameat root levelpdfResult.contentas a nested propertyBut the Vector-based PDFs example (lines 161-162) accesses:
pdfResult.pagesCountat root levelpdfResult.info.extractionNotesunder aninfoobjectAdditionally, lines 169-171 list properties (
textExtractionSuccess,isVectorBased,extractionNotes) without specifying whether they're at the root level, under.info, or under.content.The API Reference section (lines 298-305) only documents input options—it lacks a Return Value table documenting the complete structure. Add a return value table for
readPDFspecifying the full object shape, or update the examples to use consistent property paths.
🤖 Fix all issues with AI agents
In @docs/explanation/features/tea-overview.md:
- Line 202: Update the fragment count under the "**Extensive domain knowledge**"
bullet: either change the hardcoded "32 fragments" to "33 fragments" to match
the current knowledge base, or replace it with a general "30+ fragments" phrase
to future-proof the text; edit the line containing "**Extensive domain
knowledge**: 32 fragments..." accordingly in the tea-overview content.
- Line 426: Update the heading "Utilities available (11 total)" to reflect the
actual list length: change "11" to "10" in that line, or if a utility was
accidentally omitted, add the missing utility name to the comma-separated list
(the line containing "Utilities available (11 total): api-request,
network-recorder, auth-session, intercept-network-call, recurse, log,
file-utils, burn-in, network-error-monitor, fixtures-composition") so the
numeric count matches the listed items.
In @src/modules/bmm/testarch/knowledge/api-request.md:
- Around line 5-6: Example 2 has invalid TypeScript because zod is imported
inside a test and `const response` is redeclared; fix by moving `import { z }
from 'zod'` to the module level (top of the file), split the single test into
two separate tests (e.g. "should validate response schema (JSON Schema)" and
"should validate response schema (Zod)") so each uses its own scoped variables,
and remove the duplicate `const response` declarations by ensuring each test
declares its own uniquely scoped response variable or omits it if not used; keep
using the existing `test` fixture and `apiRequest` call signatures
(`validateSchema` with JSON Schema in one test and with a Zod `UserSchema` in
the other).
In @src/modules/bmm/testarch/knowledge/auth-session.md:
- Line 5: Examples 6 and 7 do not demonstrate auth-session disk persistence;
either refactor them to use the auth-session pattern or move them to
api-testing-patterns.md. Replace the in-memory beforeAll authToken flow in
Example 6 with calls to the auth-session helper that reads/writes tokens to disk
(so tokens survive across runs), and update Example 7 to obtain API tokens via
auth-session rather than raw env vars or move it to the API-only doc. Also make
fixtures consistent by using a single request fixture (either request or
apiRequest) across login and subsequent calls (update references to { request }
and { apiRequest } accordingly), and add explicit null/undefined checks for
process.env.TEST_USER_EMAIL, TEST_USER_PASSWORD and SERVICE_API_KEY with clear
error messages so missing env vars fail fast.
In @src/modules/bmm/testarch/knowledge/file-utils.md:
- Around line 278-313: Add a "Return Value" subsection for each reader in
file-utils.md documenting the exact result object shapes returned by the
functions: for readCSV describe that it returns { content: { data:
Array<Array<string|number>>, headers: string[] } } (or null headers when
parseHeaders:false) and include any metadata fields; for readXLSX describe {
content: { worksheets: Array<{ name: string, rows: Array<Array<any>>, headers?:
string[] }> } }; for readPDF describe { content: string, pagesCount: number,
fileName?: string, info?: Record<string, any> } and note mergePages/maxPages
effects; and for readZIP describe { content: { entries: Array<{ name: string,
size: number, isDirectory: boolean }>, extractedFiles: Record<string, Buffer |
string> } so callers know exact keys and types.
In @src/modules/bmm/testarch/knowledge/log.md:
- Around line 199-209: The example uses an undefined TODO_ITEMS in the
createDefaultTodos test step; fix by defining TODO_ITEMS or importing it into
the same scope so the for-loop in createDefaultTodos can iterate over it — e.g.,
add a local const TODO_ITEMS = ['item1', 'item2', ...] before createDefaultTodos
or import { TODO_ITEMS } from its module, and ensure the constant’s type matches
expectations for TodoPage.addTodo.
In @src/modules/bmm/testarch/knowledge/network-error-monitor.md:
- Around line 24-25: The merged-fixtures example uses base.extend() but never
imports base (and also references mergeTests and
createNetworkErrorMonitorFixture), so add the missing imports at the top of the
example: import the named test as base from '@playwright/test', import
mergeTests from '@playwright/test', and import createNetworkErrorMonitorFixture
from the network-error-monitor fixtures module so base.extend(), mergeTests and
createNetworkErrorMonitorFixture resolve at runtime.
In @src/modules/bmm/testarch/knowledge/recurse.md:
- Around line 251-282: The test "kafka event processed" uses a hard-coded
assertion expect(res.body.available).toBeLessThanOrEqual(98) without
establishing initial inventory; update the test to either fetch and record the
baseline inventory before posting the order (call apiRequest GET for
/api/inventory/ABC123 prior to the POST) and then assert the available count
decreased by 2 using that baseline, or replace the magic number with a relative
comparison (e.g., expect(res.body.available).toBe(baseline - 2)); adjust the
usage of recurse, apiRequest, and the assertion accordingly so the expectation
is deterministic.
🧹 Nitpick comments (5)
src/modules/bmm/testarch/knowledge/api-testing-patterns.md (1)
682-682: Replace JWT token placeholder to avoid security scanner false positives.Static analysis (gitleaks) flags the partial JWT token on this line as a potential API key exposure. While the token is incomplete and clearly in a documentation example context, security scanners will continue to flag it. Consider using an obviously placeholder string to avoid false positives in CI/CD pipelines.
🔒 Proposed alternative
- const expiredToken = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9...'; // Expired token + const expiredToken = 'expired.jwt.token.example'; // Expired tokenAlternatively, use a variable name that makes the example intent clearer:
- const expiredToken = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9...'; // Expired token + const expiredToken = 'EXPIRED_TOKEN_PLACEHOLDER'; // Expired tokensrc/modules/bmm/testarch/knowledge/intercept-network-call.md (1)
391-391: Clarify "Reduction" note reference.The "~5-7 lines -> ~2-3 lines per interception" reduction notation is helpful, but the phrasing could be clearer. Consider rephrasing to: "Reduction: Typical boilerplate reduced from ~5-7 lines to ~2-3 lines per interception" for consistency with similar patterns in related docs.
docs/explanation/features/tea-overview.md (1)
349-349: Soften wordy phrasing: "Prior to release".Line 349 uses "Prior to release, rerun coverage..." which could be more concise. Consider: "Before release, rerun coverage..." or "At release, rerun coverage..."
Before release, rerun coverage (`*trace`, `*automate`), perform final quality audit with `*test-review`, and formalize the decision with `*trace` Phase 2 (gate decision); archive artifacts for compliance audits.src/modules/bmm/testarch/knowledge/file-utils.md (2)
92-115: XLSX example type assertion may hide runtime errors.Example 2 shows:
- Line 96:
const worksheet = xlsxResult.content.worksheets[0];- Line 101:
const sheetData = worksheet?.data;- Line 105:
const firstRow = sheetData![0] as Record<string, unknown>;The optional chaining (
worksheet?.data) is good defensive coding. However, line 105 uses a non-null assertion (!) onsheetDataafter a potentially falsy check. Ifworksheet.datais undefined, line 105 will throw a runtime error. Either:
- Move the type assertion inside a proper null check, or
- Add a validation guard before accessing
sheetData[0]Consider this refactor:
const worksheet = xlsxResult.content.worksheets[0]; expect(worksheet).toBeDefined(); const sheetData = worksheet?.data; if (!Array.isArray(sheetData) || sheetData.length === 0) { throw new Error('Sheet data is empty or not an array'); } const firstRow = sheetData[0] as Record<string, unknown>; expect(firstRow).toHaveProperty('id');
241-244: Verify readCSV destructuring pattern in Example 5.Line 241 shows:
const { content } = await readCSV({ filePath: downloadPath });This destructures only
contentfrom the result. Later line 243 accessescontent.data. This is correct and matches Example 1, but the brief destructuring pattern is different from the pattern in Example 1 (line 59) which destructures bothdataandheadersdirectly. For consistency and clarity, consider using the same destructuring pattern across both examples, or explain the difference.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/modules/bmm/testarch/tea-index.csvis excluded by!**/*.csv
📒 Files selected for processing (13)
docs/explanation/features/tea-overview.mdsrc/modules/bmm/agents/tea.agent.yamlsrc/modules/bmm/module.yamlsrc/modules/bmm/testarch/knowledge/api-request.mdsrc/modules/bmm/testarch/knowledge/api-testing-patterns.mdsrc/modules/bmm/testarch/knowledge/auth-session.mdsrc/modules/bmm/testarch/knowledge/file-utils.mdsrc/modules/bmm/testarch/knowledge/intercept-network-call.mdsrc/modules/bmm/testarch/knowledge/log.mdsrc/modules/bmm/testarch/knowledge/network-error-monitor.mdsrc/modules/bmm/testarch/knowledge/network-recorder.mdsrc/modules/bmm/testarch/knowledge/overview.mdsrc/modules/bmm/testarch/knowledge/recurse.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
**/*: Focus on inconsistencies, contradictions, edge cases and serious issues.
Avoid commenting on minor issues such as linting, formatting and style issues.
When providing code suggestions, use GitHub's suggestion format:<code changes>
Files:
src/modules/bmm/module.yamlsrc/modules/bmm/testarch/knowledge/auth-session.mdsrc/modules/bmm/testarch/knowledge/log.mdsrc/modules/bmm/testarch/knowledge/file-utils.mdsrc/modules/bmm/testarch/knowledge/api-testing-patterns.mdsrc/modules/bmm/agents/tea.agent.yamlsrc/modules/bmm/testarch/knowledge/network-recorder.mdsrc/modules/bmm/testarch/knowledge/overview.mdsrc/modules/bmm/testarch/knowledge/network-error-monitor.mddocs/explanation/features/tea-overview.mdsrc/modules/bmm/testarch/knowledge/api-request.mdsrc/modules/bmm/testarch/knowledge/recurse.mdsrc/modules/bmm/testarch/knowledge/intercept-network-call.md
🪛 Gitleaks (8.30.0)
src/modules/bmm/testarch/knowledge/api-testing-patterns.md
[high] 682-682: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 LanguageTool
docs/explanation/features/tea-overview.md
[style] ~349-~349: ‘Prior to’ might be wordy. Consider a shorter alternative.
Context: ...mpliance with testing best practices. - Prior to release, rerun coverage (*trace, `*au...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
🔇 Additional comments (9)
src/modules/bmm/agents/tea.agent.yaml (1)
15-15: Persona/principles update aligns with API-first TEA positioning.
The test-level preference and “API tests are first-class citizens” principle read consistently with the PR’s documentation direction.Also applies to: 20-26, 44-44
src/modules/bmm/module.yaml (1)
47-50: Doc reference path is correct, but suggested fix assumes unavailable template variable.The referenced file
docs/explanation/features/tea-overview.mdexists and the current path is accurate. However, the suggestion to use{project-root}is not applicable—this template variable is not used anywhere inmodule.yaml. Other artifact paths in the file use{output_folder}placeholders, and the simplerdocsreference on line 44 shows that relative paths are acceptable in this configuration.The path should work correctly when the module is run from the repository root, which is the expected context.
Likely an incorrect or invalid review comment.
src/modules/bmm/testarch/knowledge/network-recorder.md (1)
24-62: Well-structured Quick Start and comprehensive examples.The documentation effectively guides users from basic record/playback (Quick Start) through advanced patterns (URL mapping, content storage) with clear, executable code samples. The emphasis on stateful CRUD detection as the key differentiator from native Playwright is articulated clearly.
Also applies to: 158-196, 326-356
src/modules/bmm/testarch/knowledge/overview.md (1)
5-5: Clear repositioning toward API-first testing approach.The reorganization effectively emphasizes backend-first mentality and explicitly identifies which utilities support pure API testing without a browser. The note clarifying that 6 of 9 utilities work browserless is a valuable signal for users planning test architecture.
Also applies to: 23-23, 41-53
src/modules/bmm/testarch/knowledge/api-testing-patterns.md (2)
1-38: Comprehensive new API testing guide with clear rationale.This new documentation effectively establishes API testing as a first-class citizen in the testing pyramid. The rationale section (lines 7–23) clearly articulates when API tests are preferable to E2E, and the when-to-use table provides pragmatic guidance for practitioners.
41-193: Seven well-crafted examples spanning common API testing scenarios.The examples progress logically from pure API tests to advanced patterns (microservices, GraphQL, background jobs). Each includes clear context, implementation code, and key points. The microservice example (Example 3, lines 203–290) and background job polling example (Example 6, lines 533–621) particularly demonstrate the
apiRequestandrecurseutilities effectively.Also applies to: 203-290, 300-425
docs/explanation/features/tea-overview.md (2)
226-266: Clarify when "exploratory mode" is available for*test-design.The Command Catalog (line 373) mentions
*test-designincludes "+ Exploratory: Interactive UI discovery with browser automation (uncover actual functionality)" when MCP is enabled. However, the Greenfield cheat sheet does not mention exploratory mode as an option during Phase 3 (Solutioning). Clarify whether exploratory mode applies only to Phase 4 (epic-level test design) or also to Phase 3 (system-level test design), and add a note to the relevant cheat sheet rows.
369-378: MCP enhancement descriptions are accurate and properly aligned with the MCP Enhancements section.The command catalog's simplified terms ("Exploratory," "Recording," "Healing") are intentionally abstracted labels for how Playwright MCP tools are applied in TEA workflows. The MCP Enhancements section (lines 432–486) provides the required technical detail—mapping each label to specific MCP tools (
browser_navigate,browser_snapshot,browser_generate_locator, etc.). All capabilities are properly documented, aligned with the config flagtea_use_mcp_enhancements: true(line 461), and match actual Playwright MCP server functionality. The two-level abstraction (conceptual names in catalog + detailed tools in MCP section) is intentional and appropriate for a feature overview.src/modules/bmm/testarch/knowledge/recurse.md (1)
119-144: Example 2: Truthiness explanation needs clarification or verification.The explanation of predicate return behavior (lines 119-144) claims the utility succeeds if assertions pass OR if the return is falsy (undefined), but this seems contradictory to polling semantics. Clarify whether:
- Assertions-only predicates (implicit undefined return) are intended to succeed after
expect()calls pass- Explicit
return falsefrom a predicate (when no assertion is thrown) should fail and retry- How the library distinguishes between "no explicit return" (success) vs "return false" (failure)
The current explanation could confuse users about whether their predicates need explicit returns. If the behavior truly allows assertion-only predicates, this is a valuable pattern but needs very clear documentation of how the library achieves this.
bmad-code-org#1289) * docs: expand TEA documentation with cheat sheets, MCP enhancements, and API testing patterns * docs: update TEA fragment counts and fix playwright-utils code examples * docs: addressed PR review concerns * docs: update TEA MCP configuration link to point to documentation site
Summary
Addresses feedback that TEA appeared too web/UI focused. Updates positioning, knowledge base, and documentation to establish TEA as equally proficient in API/backend testing and UI testing.
Changes
api-testing-patterns.md- comprehensive guide for pure API testing (microservices, GraphQL, service auth, no browser)api,backend,service-testing,microservicesfor better discoverabilitytest-architecture.mdreference → now points totea-overview.md