admin-api simulation tests #3863
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: geoberle The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
geoberle
force-pushed
the
admin-simulation-tests
branch
3 times, most recently
from
24ac82e to
f2d03a1
Compare
add integration tests similar to the backend and frontend ones in `/test-integration` * testsuit framework with cosmos and CS data loading * simplified hello-world demo endpoint so it can be served from cosmos and CS data * add support for cosmosdb emulator on OSX (there is no arm64 image in the latest tag and x86 emulation with qemu results in segfaults) Signed-off-by: Gerd Oberlechner <goberlec@redhat.com>
geoberle
force-pushed
the
admin-simulation-tests
branch
from
f2d03a1 to
adf8c40
Compare
Signed-off-by: Gerd Oberlechner <goberlec@redhat.com>
geoberle
force-pushed
the
admin-simulation-tests
branch
from
a027f75 to
a13133f
Compare
| return | ||
| } | ||
|
|
||
| // get first party application token credentials for the HCP |
There was a problem hiding this comment.
Help me understand what's going on here?
There was a problem hiding this comment.
Listing the LBs of the managed RG was just for showcasing the FPA creds in this demo handler. I did not want to spend time introducing fakes for the LB azure service at this point if we will not need it.
There was a problem hiding this comment.
Let's go in the other direction. Backend wants these creds in @miguelsorianod's PR. Let's actually start an endpoint to pull useful/important azure resources for debugging a cluster.
| } | ||
| } | ||
|
|
||
| func (tc *AdminTestCase) loadTestDefinition(t *testing.T) { |
There was a problem hiding this comment.
I'm surprised you needed to do this (and executeRequest). Why were the primitives in the existing simulation tests insufficient?
cc @deads2k do you recommend a different approach? I am not in the weeds enough to know
There was a problem hiding this comment.
i had another look and now saw that there is the concept of the HTTPTestAccessor with an implementation for frontend. i will check if an implementation for admin api would fit the approach
Signed-off-by: Gerd Oberlechner <goberlec@redhat.com>
Signed-off-by: Gerd Oberlechner <goberlec@redhat.com>
geoberle
force-pushed
the
admin-simulation-tests
branch
2 times, most recently
from
91451da to
c91ba4f
Compare
Signed-off-by: Gerd Oberlechner <goberlec@redhat.com>
geoberle
force-pushed
the
admin-simulation-tests
branch
from
c91ba4f to
188bcbd
Compare
to make the steps reusable across frontend and admin tests, i replaced the FrontendClient func in the stepInput with an HTTPAccessorFactory function encapsulating the differences in internal client initialization and usage
| func (l *httpCreateStep) RunTest(ctx context.Context, t *testing.T, stepInput StepInput) { | ||
| subscriptionID := api.Must(azcorearm.ParseResourceID(l.key.ResourceID)).SubscriptionID | ||
| accessor := newFrontendHTTPTestAccessor(stepInput.FrontendURL, stepInput.FrontendClient(subscriptionID)) | ||
| accessor := stepInput.HTTPAccessorFactory(l.key.ResourceID) |
There was a problem hiding this comment.
you made the keys different. Admin API keys start with /admin. Use that to determine to the correct client to use and don't require a factory. That will allow us to do "the right thing" in a combined test.
| # these are the default values of the emulator container. | ||
| DEFAULT_COSMOS_ENDPOINT="https://localhost:8081" | ||
| # Control whether to restart an existing emulator | ||
| RESTART_EXISTING_EMULATOR="${RESTART_EXISTING_EMULATOR:-true}" |
| }, nil | ||
| } | ||
|
|
||
| func (tt *AdminResourceMutationTest) RunTest(t *testing.T) { |
There was a problem hiding this comment.
now that we have the steps mutually compatible, what is the value of this function compared to the "normal" RunTest in this package.
| CosmosContainer *azcosmos.ContainerClient | ||
| DBClient database.DBClient | ||
| ClusterServiceMockInfo *integrationutils.ClusterServiceMock | ||
| ServiceURL string |
There was a problem hiding this comment.
I expected two URLS, one for frontend, one for admin API, and both optional
What
add integration tests similar to the backend and frontend ones in
/test-integrationthis is a preparational PR so followup PRs that introduce first real admin api endpoints can focus on their logic + tests
Why
Special notes for your reviewer