[AV-127654] Add comprehensive acceptance tests for app_endpoint resource#578
Conversation
|
No PR template selections were detected. Please make sure to fill out the PR template properly by selecting the appropriate checkboxes. |
|
Droid finished @nimiyajoseph's task —— View job |
There was a problem hiding this comment.
Pull request overview
Adds broad acceptance test coverage around the couchbase-capella_app_endpoint ecosystem (CORS, OIDC, import, lifecycle edge-cases), alongside provider-side fixes to reduce import drift and improve deletion stability.
Changes:
- Extend
app_endpointacceptance coverage with multiple new scenario-focused tests (including import verification vianameidentifier). - Adjust provider
Readbehavior to avoid suppressing remote CORS during import. - Add post-
DELETEpolling for App Endpoint removal to reduce downstream 412/“still in use” failures (notably bucket deletion races).
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/resources/app_endpoint.go | Detect import to avoid CORS suppression; add post-delete polling wait for 404. |
| acceptance_tests/bucket.go | Whitespace-only change. |
| acceptance_tests/app_endpoint.go | Prevent nil-body unmarshal panic by retrying on transient GET errors. |
| acceptance_tests/app_endpoint_acceptance_test.go | Adds many new app_endpoint acceptance scenarios; adjusts some tests to run serially. |
| acceptance_tests/app_endpoint_datasource_acceptance_test.go | New acceptance tests for app_endpoint-related data sources (single, list, filtered). |
| acceptance_tests/app_endpoint_cors_acceptance_test.go | New acceptance tests for the app_endpoint_cors resource (CRUD + import). |
| acceptance_tests/app_endpoint_oidc_provider_acceptance_test.go | New acceptance tests for OIDC provider + default provider resources (CRUD + import). |
| acceptance_tests/app_endpoint_import_filter_acceptance_test.go | New acceptance tests for app_endpoint_import_filter resource (CRUD + import). |
| acceptance_tests/app_endpoint_access_control_function_acceptance_test.go | New acceptance tests for app_endpoint_access_control_function resource (CRUD + import). |
Comments suppressed due to low confidence (1)
acceptance_tests/app_endpoint_acceptance_test.go:96
- This test creates its own bucket/endpoint with randomized names and only validates an expected error; there’s no apparent ordering dependency. Per acceptance test guidelines, prefer resource.ParallelTest here to avoid serializing the suite unnecessarily.
resource.Test(t, resource.TestCase{
ProtoV6ProviderFactories: globalProtoV6ProviderFactory,
Steps: []resource.TestStep{
{
Config: cfg,
ExpectError: re.MustCompile("Collection Not Found"),
},
},
There was a problem hiding this comment.
Validated 5 candidates: 3 are actionable and correctly anchored, and 2 were rejected because their anchors are outside changed diff hunks. The approved items cover concrete issues in waitForEndpointDeletion error/cancellation handling and a real acceptance-test flake risk from parallel writes to a shared global endpoint.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
3d0dcb4 to
dfd1361
Compare
dfd1361 to
492e36b
Compare
492e36b to
bd6d54f
Compare
…er/app service/bucket/endpoint under the existing project
…er/app service/bucket/endpoint under the existing project
Jira
Description
Acceptance tests — infrastructure (acceptance_tests/)
globals.go
Added named global bucket constants for all fixture endpoints and app_endpoint resource tests, replacing random-named buckets
Each test gets a stable, pre-created bucket — eliminates 500 errors on bucket creation and 412 errors on bucket cleanup
endpoint_setup.go (new file)
ensureFixtureBucketByName — creates a named bucket exactly once per test process using sync.Once, safe for parallel tests
ensure{ACF,IF,CORS,CORSOriginOnly,OIDC,DefaultOIDC}Endpoint — lazily provisions shared sub-resource test endpoints sequentially before parallel tests run
app_endpoint.go
Added bucket parameter to createAppEndpoint so each caller specifies its own bucket (prevents 409 conflicts)
Added retry loop for transient 500 errors on endpoint creation (App Service overloaded during warmup)
setup_test.go
Updated createAppEndpoint call sites to pass bucket name
Acceptance tests — test files
app_endpoint_acceptance_test.go
Full CRUD and update coverage: NoCors, CorsFullConfig, CorsSpecificOrigins, CorsMaxAgeZeroSilentDrift, OIDCFullFields, OIDCDiscoveryURL, UpdateCorsExpand, UpdateCorsOriginWildcardToSpecific, UpdateAddOIDC, UpdateACF, UpdateCorsMaxAgeToZero, UpdateCorsMaxAgeFromZero
Each test calls ensureFixtureBucketByName before resource.ParallelTest; Terraform HCL no longer owns bucket lifecycle
Known-broken cases skipped with Jira references: CorsDisabledFalseNoOrigin (AV-128217), MultipleOIDC (AV-128222), UpdateRemoveCors (AV-128229), UpdateCorsDisableToggle (AV-128229), UpdateRemoveOIDC (AV-128167)
app_endpoint_access_control_function_acceptance_test.go, app_endpoint_import_filter_acceptance_test.go, app_endpoint_cors_acceptance_test.go, app_endpoint_oidc_provider_acceptance_test.go (new files)
Sub-resource tests using pre-created shared endpoints via ensure*Endpoint
app_endpoint_datasource_acceptance_test.go (new file)
Single-endpoint, activation-status, and filtered-list data source tests
Unfiltered list test (TestAccAppEndpointsDataSource) skipped: returns 500 under parallel load (AV-128XXX)
Type of Change
Manual Testing Approach
How was this change tested and do you have evidence? (REQUIRED: Select at least 1)
Testing
Further comments