Draft
Conversation
hanneshayashi
changed the title
TestAccReport_Attributions if not ru…
hanneshayashi
force-pushed
the
improve-tests
branch
from
18c9ae4 to
ab13c0b
Compare
…rces - Allocation: Tests pass (API returns 200 on delete) - Budget: Tests pass - Report: Tests pass - Label: Pre-skipped with CMP-37040 reference (API returns 500 for non-existent resources) Partial cherry-pick from feat/alert-resource, excluding alert-related files.
hanneshayashi
force-pushed
the
improve-tests
branch
from
ab13c0b to
695af5e
Compare
The Alerts API DELETE endpoint now correctly returns 404 for non-existent resources (CMP-37342 has been deployed), so we can enable this test. Labels API still returns 500, so TestAccLabel_Disappears remains skipped until CMP-37040 is resolved.
The Report resource was the only resource missing an import test, causing 0% coverage on the ImportState method.
…e error handling Add 5 new unit tests for error handling paths: - TestLabelResourceRead_NotFound (improves label_resource.go:Read 52.6% -> 73.7%) - TestAnnotationResourceDelete_NotFound (improves annotation_resource.go:Delete 54.5% -> 72.7%) - TestAnnotationResourceRead_NotFound (improves annotation_resource.go:Read 63.6% -> 72.7%) - TestAlertResourceDelete_NotFound (improves alert_resource.go:Delete 54.5% -> 72.7%) - TestAlertResourceRead_NotFound (improves alert_resource.go:Read 81.8% -> 90.9%) These tests follow the existing pattern in delete_notfound_test.go using httptest mock servers to verify 404/500 error handling behavior.
Fix undefined 'testAccClient' in 5 test files after merge from main: - budget_resource_test.go - label_resource_test.go - report_resource_test.go - annotation_resource_test.go - allocation_resource_test.go The correct function name is 'getAPIClient' defined in provider_test.go.
- TestAccAlertDataSource_WithScopes: Covers mapScopeToModel (0%→~70%) - TestAccAnnotationDataSource_WithLabelsAndReports: Covers optional field mapping (46.8%→~80%)
- TestAnnotationDataSource_Read_ErrorHandling: 404, 500, empty body - TestAlertDataSource_Read_ErrorHandling: 404, 500 These cover the error handling branches in data source Read methods.
- TestAccAllocation_ComponentFlags: include_null, inverse_selection - TestAccReport_WithTargets: targets configuration (write-only field) Note: Report config isn't returned by API GET endpoint - filed CMP-38157
Reference CMP-38157 - targets field is write-only and causes perpetual drift. The step is commented out until the API is fixed to return targets.
Add ExpectEmptyPlan() second step to verify no drift on re-apply for: - TestAccReport_Minimal, TestAccReport_Attributions, TestAccReport_Full - TestAccAllocation_ComponentFlags - TestAccAlert_WithScopes, TestAccAlert_AllConfigAttributes - TestAccBudget_Attributes_Coverage This ensures that API responses match configured values and catches write-only fields or other API bugs that could cause perpetual drift.
… environment variables for IDs.
- Add empty list validation to budgetAlertsLengthValidator - Add budgetRecipientsMinLengthValidator to block empty recipients - Add empty rules check to allocationRulesValidator - Simplify mapping functions to reflect API reality (nil vs empty slice) - Remove 'preserve user intent' logic that was hiding drift - Remove all suppressed diagnostics - Update tests to expect validation errors for empty lists
- Remove t.Skip from TestAccReport_WithSplits (API bug CMP-38160 fixed) - Add required 'group' field to splits test config - Fix targets mapping to return empty list instead of null - Update test env vars with correct attribution group/member IDs
allocation.go: - Distinguish between nil (null) and empty slice [] for rules - Added documentation explaining the pattern for imports report.go: - Removed redundant log.Println calls (diagnostics already surface errors) - Added len() > 0 checks to skip empty slice iteration - Added comments explaining that API treats empty list and nil as equivalent for user-configurable list fields (dimensions, filters, group, splits)
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
1 participant
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.
No description provided.