Skip to content

test: align test suite with SDK baseline, and honor the retries option - #598

Merged
razor-x merged 2 commits into
mainfrom
claude/ruby-python-sdk-testing-2tqetp
Aug 6, 2026
Merged

test: align test suite with SDK baseline, and honor the retries option#598
razor-x merged 2 commits into
mainfrom
claude/ruby-python-sdk-testing-2tqetp

Conversation

@razor-x

@razor-x razor-x commented Aug 6, 2026

Copy link
Copy Markdown
Member

Brings the Python test suite in line with the scope and strategy used by seamapi/javascript-http, then fixes the one SDK bug that work uncovered.

Two commits, split so the fix is reviewable on its own:

Commit Touches
test: align test suite with the JavaScript SDK baseline tests only
fix: honor the retries option seam/client.py + drops two xfail markers

The baseline this follows

The JavaScript SDK has one fixture, getTestServer, that gives every test a freshly seeded fake. Its test files are named after SDK concerns — auth, env, headers, http errors, pagination, retry, serialization, waiting on action attempts — and they reach for routes like devices.get only as a means of exercising the client. There are no per-route tests, because routes are generated. nock appears only where the fake cannot help.

Fixture

  • Start the fake directly from node_modules/.bin instead of npm run start, so teardown signals the server rather than an npm wrapper that may leave the server behind.
  • Poll /health until the server is ready and fail loudly if it exits early. Previously the only wait was a connect-retry on /_fake/default_seed with a 0.1s backoff, which gave up after roughly 1.5s — that is what made startup flaky.
  • Scope PORT to the subprocess instead of mutating os.environ.
  • Add a recording_server fixture for the two things the fake cannot do: asserting what the SDK puts on the wire, and driving retry responses.

Scope

  • Drop test/workspaces, which covered generated route methods rather than SDK behavior.
  • Move deep_attr_dict_test.py out of seam/ so all tests live under test/.

Coverage

  • Add serialization_test.py, retry_test.py, and client_test.py, and cover SeamMultiWorkspace against the fake.
  • Replace the mocked niquests.Session in the headers test with assertions on the request the server actually received, including the seam-workspace header.
  • Assert against seed ids rather than len(devices) > 0, so the tests would notice a client talking to the wrong workspace.
  • Cover paginator construction, cursor validation, and the last page.
  • Cover the publishable key rejection in the token format checks.

The personal access token xfail is gone

test/env_test.py carried xfail(reason="Fake does not support personal access token."). The fake does support it — it authorizes a personal access token on /devices/get and rejects it only on /devices/list. The JavaScript SDK's equivalent test uses devices.get, which is why it passes there. Switching the route turns that skipped test into a passing one and adds real coverage for Seam.from_personal_access_token.

The fix: retries were silently ignored

SeamHttpClient assigned self.retries after calling niquests.Session.__init__. By that point the session had already mounted its adapters with the default max_retries, so the assignment had no effect on anything that actually sends a request. A caller passing Retry(total=5, status_forcelist=[503]) got exactly one attempt.

The fix passes retries through to niquests.Session so the mounted adapters are built with it. Seam and SeamMultiWorkspace both default the option to None, which previously meant "drop it entirely" and now falls back to DEFAULT_RETRIES.

Worth knowing when reviewing: this changes runtime behavior. Requests that never retried will now retry according to whatever policy is configured. DEFAULT_RETRIES is urllib3.Retry(), whose allowed_methods covers idempotent verbs only — and the SDK sends POST — so the default path retries connection errors but not status codes, and callers who never passed retries should see no change in retry behavior on HTTP responses.

Verification

57 passed in 30.07s

Down from 55s, and the run that produced the previous baseline also hit one flaky startup error. black --check, pylint ./seam ./test (10.00/10), and rstcheck README.rst are all clean.

Bring the Python test suite in line with the scope and strategy used by
seamapi/javascript-http: tests are organized by SDK concern, exercise the
SDK against fake-seam-connect, and assert against seeded records rather
than hand-written stubs.

Fixture:

- Start the fake directly from node_modules/.bin instead of `npm run
  start`, so teardown signals the server rather than an npm wrapper.
- Poll /health until the server is ready and fail loudly if it exits
  early, replacing the connect-retry on /_fake/default_seed that made
  startup flaky.
- Scope PORT to the subprocess instead of mutating os.environ.
- Add a recording_server fixture for the two things the fake cannot do:
  asserting what the SDK puts on the wire, and driving retry responses.

Scope:

- Drop test/workspaces, which covered generated route methods rather
  than SDK behavior.
- Move deep_attr_dict_test.py under test/ so all tests live together.

Coverage:

- Add serialization, retry, and client tests, and cover the multi
  workspace client against the fake.
- Replace the mocked niquests.Session in the headers test with
  assertions on the request the server actually received.
- Assert against seed ids instead of `len(devices) > 0`.
- Cover paginator construction, cursor validation, and last page.
- Replace the personal access token xfail with passing tests. The fake
  rejects that token on /devices/list but authorizes it on
  /devices/get, which is the route the JavaScript SDK tests use.

Two retry tests are marked xfail(strict=True): SeamHttpClient sets
self.retries after calling niquests.Session.__init__ without forwarding
it, so the mounted HTTPAdapter keeps its default and the retries option
is silently ignored.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011DzapiU8A9NMdyoTybL9xB
@razor-x
razor-x requested a review from a team as a code owner August 6, 2026 02:58
SeamHttpClient assigned self.retries after calling
niquests.Session.__init__, by which point the session had already mounted
its adapters with the default max_retries. The retries option was
therefore silently ignored: a caller passing Retry(total=5,
status_forcelist=[503]) still got exactly one attempt.

Pass retries through to niquests.Session so the mounted adapters are
built with it. Seam and SeamMultiWorkspace default the option to None,
which now falls back to DEFAULT_RETRIES rather than being dropped.

Drops the xfail markers from the two retry tests that covered this.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011DzapiU8A9NMdyoTybL9xB
@razor-x razor-x changed the title test: align test suite with the JavaScript SDK baseline test: align test suite with the JavaScript SDK baseline, and honor the retries option Aug 6, 2026
@razor-x razor-x changed the title test: align test suite with the JavaScript SDK baseline, and honor the retries option test: align test suite with SDK baseline, and honor the retries option Aug 6, 2026
@razor-x
razor-x merged commit 1dea7be into main Aug 6, 2026
18 checks passed
@razor-x
razor-x deleted the claude/ruby-python-sdk-testing-2tqetp branch August 6, 2026 04:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants