test(logs): route /logs/search stubs by content + document hand-marshal bypass (issue #21)#28
Open
tkkhq wants to merge 1 commit into
Open
test(logs): route /logs/search stubs by content + document hand-marshal bypass (issue #21)#28tkkhq wants to merge 1 commit into
tkkhq wants to merge 1 commit into
Conversation
…tentional hand-marshal bypass
4 tasks
There was a problem hiding this comment.
Pull request overview
This PR makes the /logs/search API test stubs more robust by routing responses based on request content (instead of call order), and documents why internal/api/logs.go intentionally bypasses the generated typed request model for log search due to the generated oneOf union ergonomics.
Changes:
- Route
/projects/{id}/logs/searchtest responses by presence of aresource.deploymentsselector via a sharedlogSearchIsBuildhelper. - Add an explanatory comment documenting the intentional hand-marshalled
logSearchRequest+SearchProjectLogsWithBodyWithResponsepath inlogs.go.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/api/logs.go | Documents the intentional hand-marshalling approach for log-search requests. |
| internal/api/frontends_test.go | Uses content-based routing for /logs/search stub responses (build vs runtime). |
| internal/api/client_test.go | Uses content-based routing and introduces the shared logSearchIsBuild helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+20
to
+26
| // logSearchRequest is a hand-marshalled body for POST /projects/{id}/logs/search, | ||
| // sent via SearchProjectLogsWithBodyWithResponse rather than the typed | ||
| // SearchProjectLogsWithResponse. The generated apiclient models the request's | ||
| // `resource` selector as an oapi-codegen oneOf union (apiclient.LogRequestResource), | ||
| // built through From.../As... accessors; this flat struct produces the identical | ||
| // wire format while keeping the call sites readable. Intentional — do not | ||
| // "simplify" it back to the typed union call. |
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.
Addresses the low-risk, verified parts of #21 after re-scoping (the stale apiclient concern is already resolved on main —
LogSearchRequest.Resourceis the nested union).Changes
client_test.go,frontends_test.go): the/logs/searchstub now returns the build vs runtime response based on whether the request body carries aresource.deploymentsselector, via a sharedlogSearchIsBuildhelper — instead of the brittlelen(logSearchBodies)==1call-order check. Matches the approach already used in command-level tests.logs.go): comment explaining why we hand-marshal a flatlogSearchRequestand callSearchProjectLogsWithBodyWithResponserather than the typedSearchProjectLogsWithResponse. The generatedLogRequestResourceis an oapi-codegenoneOfunion (From.../As... constructors); the flat struct yields the identical wire format but keeps call sites readable. This is issue tech-debt(apiclient): regenerate log-search client; remove hand-marshalled bypass #21's checkbox feat(cli): add environment contexts #3, and resolves its checkbox chore(repo): add pull request template #2 as a deliberate 'keep' (union constructors would make the call site less clear, and the typed path now serializes correctly, so the only remaining argument was consistency).Not included (by design)
Tests:
go test ./internal/apigreen;go tool golangci-lint run ./internal/apiclean.