Skip to content

chore: add more tests for service bindings#118

Open
ryanking13 wants to merge 2 commits into
gyeongjae/do-bindings-testfrom
gyeongjae/service-bindings-test
Open

chore: add more tests for service bindings#118
ryanking13 wants to merge 2 commits into
gyeongjae/do-bindings-testfrom
gyeongjae/service-bindings-test

Conversation

@ryanking13
Copy link
Copy Markdown
Contributor

This on top of #113 (I want stacked PRs...)

Adds some service-bindings related tests. This PR purely updates test without changing SDK.

Some of them are duplicate of sdk_tests.py but I wanted to locate the bindings related tests in the same directory + run a full e2e test using pywrangler.

@ryanking13 ryanking13 requested review from dom96 and hoodmane June 1, 2026 08:25
Copy link
Copy Markdown

@ask-bonk ask-bonk Bot left a comment

Choose a reason for hiding this comment

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

I'm Bonk, and I've done a quick review of your PR.

This PR adds end-to-end service binding tests covering RPC calls (identity, multi-arg, defaults), unsupported types, and fetch forwarding.

  1. Minor: Misleading noqa comment — The ServiceBinding import comment says "side effect of registering the Durable Object" but ServiceBinding is a WorkerEntrypoint, not a DurableObject.

Comment thread packages/cli/tests/bindings-test/src/worker.py Outdated
@ask-bonk
Copy link
Copy Markdown

ask-bonk Bot commented Jun 1, 2026

Review posted on PR #118. The only actionable finding was a misleading copy-pasted noqa comment — ServiceBinding is a WorkerEntrypoint, not a DurableObject, so the comment should reflect that. A suggestion comment was posted inline with the fix. The rest of the PR is clean and follows established patterns well.

github run

Co-authored-by: ask-bonk[bot] <249159057+ask-bonk[bot]@users.noreply.github.com>
@dom96
Copy link
Copy Markdown
Contributor

dom96 commented Jun 1, 2026

GitHub supports stacked PRs now, doesn't it? https://github.github.com/gh-stack/

@ryanking13
Copy link
Copy Markdown
Contributor Author

Yeah, but I think it is still in the private beta that is not enabled for this org.

Copy link
Copy Markdown
Contributor

@dom96 dom96 left a comment

Choose a reason for hiding this comment

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

If we're duplicating tests, maybe we could deduplicate somehow or remove the other test cases if these give better test coverage?

Comment on lines +1 to +8
async def test_identity_primitives(env):
svc = env.SERVICE_BINDING
assert await svc.identity("hello") == "hello"
assert await svc.identity(42) == 42
assert await svc.identity(3.14) - 3.14 < 0.001
assert await svc.identity(True) is True
assert await svc.identity(False) is False
assert await svc.identity(None) is None
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How are these different to the tests we already do in python-rpc workerd tests?

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