Move requireGrpcCode test helper into shared servertest package - #5047
Open
dejanzele wants to merge 1 commit into
Open
Move requireGrpcCode test helper into shared servertest package#5047dejanzele wants to merge 1 commit into
dejanzele wants to merge 1 commit into
Conversation
This was referenced Jul 24, 2026
Contributor
Greptile SummaryConsolidates duplicated gRPC status assertion helpers into the shared
Confidence Score: 5/5The PR appears safe to merge. No blocking failures remain.
|
| Filename | Overview |
|---|---|
| internal/server/servertest/grpc.go | Adds the shared gRPC status-code assertion helper with fatal assertions for invalid status errors and mismatched codes. |
| internal/server/executor/executor_server_test.go | Replaces the local helper with the shared servertest implementation. |
| internal/server/node/node_test.go | Replaces the local helper with the shared servertest implementation. |
| internal/server/queue/queue_service_test.go | Replaces the local helper with the shared servertest implementation. |
| internal/server/retrypolicy/service_test.go | Replaces the local helper with the shared servertest implementation. |
Reviews (6): Last reviewed commit: "Move requireGrpcCode test helper into sh..." | Re-trigger Greptile
Member
Author
dejanzele
force-pushed
the
share-grpc-status-test-helper
branch
3 times, most recently
from
July 30, 2026 15:37
9d67642 to
cae4b3e
Compare
dejanzele
added a commit
that referenced
this pull request
Jul 30, 2026
Implements the `RetryPolicyService` whose proto was defined in #4998: create, get, list, update, and delete for retry policies, backed by a Postgres repository. Adds `create_retry_policy`, `update_retry_policy`, and `delete_retry_policy` permissions and maps them in the local development server configs. Creating a policy that already exists replaces it, the same as `create queue`, so a pipeline that creates policies can be re-run. Updating one that does not exist still returns `NotFound`. Queue attachments are stored relationally rather than inside the serialized queue definition. Migration 035 adds the `retry_policy` table together with a `queue_retry_policy` join table, where each row records the position its policy held in the submitted list so that a queue reads its policies back in the order they were given. The queue repository rewrites those rows on every queue write and composes `RetryPolicies` from them on every read, so the serialized definition never carries the names. A foreign key on `policy_name` is what prevents a queue referencing a policy that does not exist. A queue write also looks the names up first, so the error can say which ones are missing, returned as `InvalidArgument`. In the narrow case where a policy is deleted between that lookup and the write, the foreign key rejects the write with a less specific message. Deleting a policy removes its attachment rows and then the policy itself, returning the queues it detached for the service to log. The migration names that constraint explicitly because the repository matches on it. Both tables are queried through sqlc, generated into `internal/server/queryapi/database`, matching the pattern the scheduler already uses. The queue table's own statements are unchanged. Independent of #5047 and #5048, any merge order works. Two notes on the seams: the `RetryPolicies` field on the client queue type also appears in #5048 as an identical hunk, since both PRs need it to compile on their own, and it merges cleanly from either direction. The retry policy service tests define their own gRPC status helper, which can switch to the shared one from #5047 once both are in. Replaces #5000, which carried all of this plus the client and armadactl work in one PR. Signed-off-by: Dejan Zele Pejchev <pejcev.dejan@gmail.com>
Signed-off-by: Dejan Zele Pejchev <pejcev.dejan@gmail.com>
dejanzele
force-pushed
the
share-grpc-status-test-helper
branch
from
July 30, 2026 16:28
cae4b3e to
160066f
Compare
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
1 participant
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.
The executor, node, queue, and retry policy server tests each defined their own private
requireGrpcCodehelper for asserting that an error is a gRPC status with a given code. This moves the helper into the sharedservertestpackage asRequireGrpcCodeand switches all four to it.The private copies used
assert.Equalfor the code comparison and the shared helper usesrequire.Equal, so a mismatched code now stops the test instead of letting it carry on.The retry policy copy came in with #5049, which noted it could switch to the shared helper once both were in. That is done here, so no private copies remain.