Add smoke tests for allocation, allocation summary, and assets endpoints#97
Open
e-300 wants to merge 3 commits into
Open
Add smoke tests for allocation, allocation summary, and assets endpoints#97e-300 wants to merge 3 commits into
e-300 wants to merge 3 commits into
Conversation
Signed-off-by: Ebad Shahid <ebad@ebad.dev>
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR adds smoke tests for the allocation and asset API endpoints, along with a minor shell quoting fix in the bats setup functions.
Changes:
- Added smoke tests for
/allocation,/allocation/summary, and/assetsendpoints that verify HTTP 200 responses with a 60-second timeout. - Quoted
$DIRin thecdcommand within the batssetup()functions to handle paths with spaces.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/integration/api/asset/test.bats | Added smoke test entry and quoted $DIR in setup |
| test/integration/api/asset/assets_smoke_test.go | New Go test verifying /assets?window=1d returns 200 |
| test/integration/api/allocation/test.bats | Added smoke test entries and quoted $DIR in setup |
| test/integration/api/allocation/allocation_smoke_test.go | New Go test verifying /allocation returns 200 |
| test/integration/api/allocation/allocation_summary_smoke_test.go | New Go test verifying /allocation/summary returns 200 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ) | ||
|
|
||
| func TestAllocationSmoke(t *testing.T) { | ||
| apiObj := api.NewAPI() |
|
|
||
| apiClient := api.NewAPI() | ||
|
|
||
| // logging incase we can trace potential env issues with go test -v |
| func TestAllocationSmoke(t *testing.T) { | ||
| apiObj := api.NewAPI() | ||
|
|
||
| // logging incase we can trace potential env issues with go test -v |
Signed-off-by: Ebad Shahid <ebad@ebad.dev>
ameijer
reviewed
Jun 12, 2026
| window string | ||
| }{ | ||
| { | ||
| name: "ValidQueryTest", |
Member
There was a problem hiding this comment.
nit - make naming consistent with alloc test
| ) | ||
|
|
||
| func TestAllocationSmoke(t *testing.T) { | ||
| apiObj := api.NewAPI() |
| func TestAllocationSmoke(t *testing.T) { | ||
| apiObj := api.NewAPI() | ||
|
|
||
| // logging incase we can trace potential env issues with go test -v |
|
|
||
| apiClient := api.NewAPI() | ||
|
|
||
| // logging incase we can trace potential env issues with go test -v |
|
|
||
| apiClient := api.NewAPI() | ||
|
|
||
| // logging incase we can trace potential env issues with go test -v |
This reverts commit 8946b9e. Signed-off-by: Ebad Shahid <ebad@ebad.dev>
|
|
||
| apiClient := api.NewAPI() | ||
|
|
||
| // logging incase we can trace potential env issues with go test -v |
|
|
||
| apiClient := api.NewAPI() | ||
|
|
||
| // logging incase we can trace potential env issues with go test -v |
|
|
||
| apiClient := api.NewAPI() | ||
|
|
||
| // logging incase we can trace potential env issues with go test -v |
Comment on lines
+11
to
+12
| #60sec cap bc SDK http.Get has no timeout, avoids go test's default 10 min | ||
| go test -timeout 60s assets_smoke_test.go |
| } | ||
|
|
||
| @test "allocation: smoke test allocation" { | ||
| #60sec cap bc SDK http.Get has no timeout, avoids go test's default 10 min |
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.
Description
Adds smoke tests for the
/allocation,/allocation/summary, and/assetsendpoints.Each test issues a request with
window=1dand verifies the endpoint responds with a 200 status code, providing a fast first-line check that the API is alive before the deeper integration tests run.Cloud cost endpoint smoke coverage was intentionally left out of this PR: implementing it would require adding a cloud cost client to
pkg/apifirst, I've left it for a follow up PR to keep this one focused, but happy to include it here if reviewers prefer.Changes
allocation_smoke_test.goandallocation_summary_smoke_test.goundertest/integration/api/allocation/assets_smoke_test.goundertest/integration/api/asset/test/integration/api/allocation/test.batsandtest/integration/api/asset/test.batsto register the new smoke tests, with a-timeout 60scap on eachgo testinvocation (the SDK's HTTP client has no timeout, so this prevents a hung endpoint from stalling untilgo test's 10-minute default)Related Issues
#74
Testing
Ran the new smoke tests against a local k3d cluster running OpenCost and the demo cluster; all pass via both
go test -run Smoke ./...and the bats suite.