Skip to content

doc: add pyats developer guide - #929

Open
oboehmer wants to merge 8 commits into
mainfrom
pyats-docs
Open

doc: add pyats developer guide#929
oboehmer wants to merge 8 commits into
mainfrom
pyats-docs

Conversation

@oboehmer

@oboehmer oboehmer commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Description

To meet the ask of some architecture leads, I started to put together a write-up of nac-tests's pyats API to help people create test cases (when needed).
Once reviewed, I would move this to netascode.cisco.com docs site.

I reverse-engineered a lot of the contents, so definitely require review from Andrea and Chris who contributed most of it and can surely provide some more motivation on why certain things are done they way they are.

- context["api_context"] works for D2D tests the same as API tests;
  remove incorrect "framework limitation" claim
- Add context["api_context"] = api_context to D2D example in section 4.3
- Clarify test_context() role: labels output block; context["api_context"]
  links it to the result entry; both are needed
- Document multiple-command case: set context["api_context"] to primary
  command; secondary commands appear in Commands Without Matching Results
- Unify build_api_context() description to cover both API and D2D usage
- E: opening paragraph listing what the framework handles automatically
  (discovery, auth caching, parallelism, command caching, HTML reports, CI exit codes)
- A: section 3 opener — the three-method contract buys you parallel execution,
  caching, auth, retry, and structured reports at no extra cost
- B: 'Three things the report needs' table before display_context/api_context/
  test_context subsections — clarifies the three similar-sounding concepts
  at a glance before the detailed descriptions
…fy_group note

- Add subsection numbers (1.1–13.5) and expand TOC with anchor links
- Add §13 Static Analysis: mypy/ruff pre-commit setup for external repos,
  config matching nac-test project, async/await error detection examples,
  parse_output() 2.0→2.1 migration as concrete mypy use case
- Bidirectional cross-references between §7.3 (api_context) and §10.3 (test_context)
- QUICKSTART: reference verify_group() as alternative to verify_item() in contract
- steps parameter description in §3.1
- Motivation callouts: opening paragraph (E), §3 contract payoff (A),
  §7.3 three-context table (B)
@oboehmer
oboehmer marked this pull request as ready for review August 31, 2026 12:18
@oboehmer oboehmer added documentation Improvements or additions to documentation pyats PyATS framework related labels Aug 31, 2026

@aitestino aitestino left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @oboehmer, thank you for the PR — this is a significant documentation effort and the three-tier structure (quickstart → test case guide → API reference) is exactly right for the audience. Reverse-engineering all of this is impressive, and it's remarkably accurate for that approach. A few factual items to correct before this goes to netascode.cisco.com:

Things that need adjustment:

  1. APIC_URL should be ACI_URL — api-reference.md says controller_url comes from APIC_URL. The actual env var is ACI_URL (see nac_test/utils/controller.py:80, url_env_var="ACI_URL"). The framework uses ACI_URL, SDWAN_URL, and CC_URL consistently.

  2. self.hostname source is wrong — api-reference.md says "auto-set from env var HOSTNAME". The actual source is json.loads(os.environ["DEVICE_INFO"])["hostname"] — the orchestrator passes a JSON blob via DEVICE_INFO, and hostname is extracted from it (ssh_base_test.py:116,162).

  3. device_info/device_data alias direction is backwards — api-reference.md says self.device_info is "Alias for self.device_data". It's the other way around: device_info is the primary (set from the DEVICE_INFO env var at ssh_base_test.py:116), and device_data is the alias (self.device_data = self.device_info at ssh_base_test.py:246).

  4. Code examples pass a phantom api_duration positional arg — Several examples in the test-case-guide pass api_duration as an extra argument to format_mismatch(), format_not_found(), and format_api_error(). These methods don't accept it — they take context as the last param and extract api_duration from context.get("api_duration", 0) internally. Copy-pasting these examples would throw TypeError. For example, self.format_api_error(response.status_code, response.url, context, api_duration) (line ~1160) should drop the last arg — the method signature is format_api_error(status_code, url, context). Same for format_not_found and format_mismatch in the ACI end-to-end examples around lines 1311–1322 and 1562–1571. The api-reference correctly documents the 3/4-param signatures — just the code examples in the test-case-guide that need fixing.

  5. get_devices_from_data_model() references unmerged code — api-reference.md documents this as an existing method on SDWANManagerTestBase, but it's from nac-test-pyats-common PR #37 which is still open. Either merge #37 first, or note it as upcoming.

Things I verified and are correct:

  • All class definitions and import paths (both shorthand and full module paths) — APICTestBase, SDWANManagerTestBase, CatalystCenterTestBase, IOSXETestBase, SDWANTestBase all resolve correctly.
  • The parse_output sync/async versioning — correctly documents the sync form for v2.0.0 and the async form for 2.1+ (post PR #863). Good forward-looking call.
  • NACTestBase method signatures — format_verification_result, build_api_context, get_default_value, build_identifier, categorize_results, run_async_verification_test, test_context, wrap_client_for_tracking, api_call_with_retry all match the codebase.
  • SSHTestBase's execute_command behavior, command cache TTL (3600s), _track_ssh_command flow.
  • The test discovery via AST mechanism, the verify_item/verify_group dispatch pattern, and the TEST_CONFIG structure.
  • ResultStatus enum values.

What do you think?

P.S. — This comment was drafted using voice-to-text via Claude Code. If the tone comes across as overly direct or terse, please know that's just how it tends to phrase things. No offense or criticism is intended — this is purely an objective technical review of the PR. Thanks for understanding! 🙂

- Fix APIC_URL -> ACI_URL environment variable name (actual var is ACI_URL)
- Fix hostname source: extracted from DEVICE_INFO JSON blob, not HOSTNAME env var
- Fix device_info/device_data alias direction: device_info is primary, device_data is the alias
- Remove phantom api_duration positional args from format_mismatch/format_not_found/format_api_error examples
- Add note that get_devices_from_data_model() is from unmerged nac-test-pyats-common PR #37
@oboehmer

oboehmer commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator Author

thanks for the speedy reply, @aitestino .. addressed all points

@oboehmer
oboehmer requested a review from aitestino September 2, 2026 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation pyats PyATS framework related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants