E2E: Provider dimensions for artifact upload #3176
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the Flink Artifacts E2E tests to parameterize provider/region combinations, enabling testing across multiple cloud providers (AWS and Azure). The changes improve test coverage by iterating over provider dimensions while streamlining the artifact upload flow logic.
Key Changes:
- Introduced nested test structure to iterate over multiple provider/region combinations ("AWS/us-east-2" and "AZURE/eastus")
- Refactored upload flow to accept explicit provider/region parameters instead of deriving them from cluster labels
- Improved resource selection logic to match clusters by provider/region rather than using first-available items
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
tests/e2e/specs/flinkArtifact.spec.ts |
Added nested test loops for provider dimensions; refactored upload flow to use explicit provider/region parameters; improved cluster selection logic |
tests/e2e/objects/views/FlinkDatabaseView.ts |
Changed clickUploadFromComputePool to accept provider/region parameters; added getDatabaseResourceByLabel helper method; removed return of provider/region string |
shouples
left a comment
shouples
left a comment
There was a problem hiding this comment.
Good stuff! 🎉 Only a few minor suggestions, nothing that would block merging this though
| const computePools = resourcesView.ccloudFlinkComputePools; | ||
| await expect(computePools).not.toHaveCount(0); | ||
|
|
||
| const clusterLabel = `${provider}/${region}`; |
There was a problem hiding this comment.
Minor rename for clarity
| const clusterLabel = `${provider}/${region}`; | |
| const poolDescription = `${provider}/${region}`; |
| ); | ||
| test.describe(config.testName, () => { | ||
| for (const providerRegion of providersWithRegions) { | ||
| test(`with ${providerRegion}`, async ({ page, electronApp }) => { |
There was a problem hiding this comment.
Minor: I'd put the outer dimension in the describe title while keeping the test names in the test() titles like:
for (const providerRegion of providersWithRegions) {
test.describe(`with ${providerRegion}`, () => {
test(config.testName, async ({ page, electronApp }) => {| test(`with ${providerRegion}`, async ({ page, electronApp }) => { | ||
| await setupTestEnvironment(config.entrypoint, page, electronApp); | ||
|
|
||
| const [provider, region] = providerRegion.split("/"); |
There was a problem hiding this comment.
Minor: Would it be easier to set up providersWithRegions as an array of { provider: string; region: string } objects instead of strings? Looks like most functions take them as separate arguments
|
cool, will do a quick follow-on with these changes and a couple other renames for deceptively-named funcs |
Summary of Changes
Additions
Internal
for...loop added to test the provider dimension in our Artifact E2E CUJs using the provider/region enum.Improvements
Removes the call to
clickUploadFromComputePoolfromloadArtifacts, since it doesn't load artifacts but initiates the upload flow. We now callclickUploadFromComputePoolfromcompleteUploadFlowForComputePool, and pass the provider/region, matching on that instead of using theclusterLabel.Also, we are now using
getDatabaseResourceByLabelto retrieve theartifactViewItem, as this gets the item at the db resource level.Lastly, we make a minor update to how we pass the provider and region info so we're not parsing
providerRegionall over the place.Click-testing instructions
gulp e2e "Flink Artifacts"should pass.Pull request checklist
Please check if your PR fulfills the following (if applicable):
Tests
Release notes