Repository navigation
Conversation
Every test in this package mocked the transport, and that is how `forget` on the async client shipped without the archiving PATCH the protocol requires: the mock answered the DELETE and never modelled `archived -> deleted`, so a method that could not work against a real server passed its tests. A mock answers whatever the test tells it to. Only a real server disagrees. `sdk/python/tests/test_live_client.py` is ten cases against a running server: - the round trips, including that reading a cell bumps its access count - `forget` on both clients, which is the regression this file exists for - the access rule as a client sees it: an agent the memory was not shared with gets an empty result, not an error - the protocol error codes as the SDK parses them, including `403 ACCESS_DENIED` for an unknown id (never 404, per spec §8.4) and `422 VALIDATION_ERROR` for a rejected body - which also pins the previous fix from the client side - paging with `offset` **Proven to fail, not just to pass.** With the old broken `forget` planted back, the live test fails with the server's own words: `INVALID_TRANSITION: Cannot delete cell with status 'active'. Archive it first.` Restored, it passes. A test that has never failed is a test nobody has checked. CI runs it, and does not let it skip itself: - The `sdk` job installs and starts the reference server and runs the live file with `AMP_REQUIRE_LIVE_SERVER=1`, which turns "no server reachable" into a failure - the same guard the storage job uses for PostgreSQL. - Not on the 3.10 leg, and by an explicit `if` rather than a skip: the reference server's own floor is 3.11, so that leg has nothing to talk to. A skip would read as a pass. - Locally the file still skips with a message naming `AMP_TEST_URL`, so the package tests on a laptop with nothing running (verified: 10 skipped without a server, 10 failures with the flag set and no server).
Owner
Author
|
Landed on master in the v0.1.0 chain: the branch was fast-forward merged as part of |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
test(sdk): run the Python client against a real server
Every test in this package mocked the transport, and that is how
forgeton theasync client shipped without the archiving PATCH the protocol requires: the mock
answered the DELETE and never modelled
archived -> deleted, so a method thatcould not work against a real server passed its tests. A mock answers whatever the
test tells it to. Only a real server disagrees.
sdk/python/tests/test_live_client.pyis ten cases against a running server:forgeton both clients, which is the regression this file exists foran empty result, not an error
403 ACCESS_DENIEDfor an unknown id (never 404, per spec §8.4) and
422 VALIDATION_ERRORfor arejected body - which also pins the previous fix from the client side
offsetProven to fail, not just to pass. With the old broken
forgetplanted back,the live test fails with the server's own words:
INVALID_TRANSITION: Cannot delete cell with status 'active'. Archive it first.Restored, it passes. A test that hasnever failed is a test nobody has checked.
CI runs it, and does not let it skip itself:
sdkjob installs and starts the reference server and runs the live file withAMP_REQUIRE_LIVE_SERVER=1, which turns "no server reachable" into a failure -the same guard the storage job uses for PostgreSQL.
ifrather than a skip: the referenceserver's own floor is 3.11, so that leg has nothing to talk to. A skip would read
as a pass.
AMP_TEST_URL, so the packagetests on a laptop with nothing running (verified: 10 skipped without a server,
10 failures with the flag set and no server).