ldraney/godaddy-tofu
1
0
Fork 0
Code Issues 2 Pull requests 1 Projects Releases Packages Wiki Activity Actions

Integration tests for pkg/godaddy/ client #4

New issue
Closed
opened 2026-06-13 15:34:10 +00:00 by ldraney · 3 comments
ldraney commented 2026-06-13 15:34:10 +00:00
Owner
Copy link

Type

Feature

Lineage

Depends on #3 (Go scaffold + DNS client, merged as PR #5). Tests prove the client works against the live GoDaddy API before provider wiring. Should land after #6 (QA nits: DomainID type fix, io.ReadAll limit, url naming) to avoid writing tests against stale types.

Repo

ldraney/godaddy-tofu

User Story

As a provider developer
I want integration tests that prove pkg/godaddy/ methods work against the live API
So that I can trust the client layer before building provider resources on top of it

Context

Integration tests run against the live GoDaddy API using production credentials from ~/secrets/godaddy/.env. Tests should be gated behind a build tag or env var so they don't run in CI without credentials.

DNS tests are P0 — must prove read, write, and delete of A records on a real domain. Domain tests are P1 — must prove list and get. All tests should be non-destructive where possible (read existing records, create test records, clean up after).

Rate limit: API returns 429 with retryAfterSec. Tests should implement a test-only retry helper that sleeps and retries on 429 — this is NOT client-level retry logic (that's a separate concern), just a test utility to avoid flaky failures.

File Targets

  • pkg/godaddy/client_test.go — test helpers (env var client instantiation, 429 retry helper), client instantiation tests
  • pkg/godaddy/dns_test.go — DNS integration tests covering all 6 methods:
    • TestGetRecords — read existing A records for a known domain
    • TestAddAndDeleteRecords — create test TXT records via AddRecords, verify they exist via GetRecords, delete via DeleteRecords
    • TestReplaceRecords — replace a test TXT record by type+name, verify, clean up
    • TestReplaceRecordsByType — replace all TXT records for domain, verify, clean up
    • TestReplaceAllRecords — snapshot existing records, replace all, restore original (use with caution, test on safe subdomain)
  • pkg/godaddy/domains_test.go — domain integration tests:
    • TestListDomains — list all domains, verify non-empty
    • TestGetDomain — get a specific domain, verify fields populated

Feature Flag

None.

Acceptance Criteria

  • GODADDY_API_KEY=... GODADDY_API_SECRET=... go test ./pkg/godaddy/ -v passes
  • All 6 DNS client methods have at least one test exercising the live API
  • DNS tests create, read, and delete test records without leaving state
  • Domain tests read existing domain data and verify field types (DomainID is int64 after #6)
  • Tests skip gracefully when credentials are not set (t.Skip)
  • Tests handle 429 rate limiting via test-only retry helper with backoff

Test Expectations

  • All integration tests pass against live API
  • No test leaves orphaned DNS records
  • Tests are idempotent (can run multiple times)

Constraints

  • Use testing.Short() or env var gate — tests must not run without credentials
  • Test against a real domain Lucas owns (e.g., palinks.app)
  • Use TXT records for write tests (safest — won't affect DNS resolution)
  • Clean up all created records in test teardown (t.Cleanup)
  • 429 retry is a test-only helper, NOT changes to the production client

Checklist

  • client_test.go with env var helpers and 429 retry utility
  • dns_test.go with tests for all 6 methods
  • domains_test.go with list/get tests
  • All tests pass locally
  • No orphaned test state

Related

  • Depends on #3 (merged)
  • Should land after #6 (DomainID type fix)
  • project-godaddy-tofu
  • docs/dns-endpoints.md — endpoint reference
  • docs/auth.md — credential setup
### Type Feature ### Lineage Depends on #3 (Go scaffold + DNS client, merged as PR #5). Tests prove the client works against the live GoDaddy API before provider wiring. Should land after #6 (QA nits: DomainID type fix, io.ReadAll limit, url naming) to avoid writing tests against stale types. ### Repo `ldraney/godaddy-tofu` ### User Story As a provider developer I want integration tests that prove pkg/godaddy/ methods work against the live API So that I can trust the client layer before building provider resources on top of it ### Context Integration tests run against the live GoDaddy API using production credentials from `~/secrets/godaddy/.env`. Tests should be gated behind a build tag or env var so they don't run in CI without credentials. DNS tests are P0 — must prove read, write, and delete of A records on a real domain. Domain tests are P1 — must prove list and get. All tests should be non-destructive where possible (read existing records, create test records, clean up after). Rate limit: API returns 429 with `retryAfterSec`. Tests should implement a test-only retry helper that sleeps and retries on 429 — this is NOT client-level retry logic (that's a separate concern), just a test utility to avoid flaky failures. ### File Targets - `pkg/godaddy/client_test.go` — test helpers (env var client instantiation, 429 retry helper), client instantiation tests - `pkg/godaddy/dns_test.go` — DNS integration tests covering all 6 methods: - `TestGetRecords` — read existing A records for a known domain - `TestAddAndDeleteRecords` — create test TXT records via `AddRecords`, verify they exist via `GetRecords`, delete via `DeleteRecords` - `TestReplaceRecords` — replace a test TXT record by type+name, verify, clean up - `TestReplaceRecordsByType` — replace all TXT records for domain, verify, clean up - `TestReplaceAllRecords` — snapshot existing records, replace all, restore original (use with caution, test on safe subdomain) - `pkg/godaddy/domains_test.go` — domain integration tests: - `TestListDomains` — list all domains, verify non-empty - `TestGetDomain` — get a specific domain, verify fields populated ### Feature Flag None. ### Acceptance Criteria - [ ] `GODADDY_API_KEY=... GODADDY_API_SECRET=... go test ./pkg/godaddy/ -v` passes - [ ] All 6 DNS client methods have at least one test exercising the live API - [ ] DNS tests create, read, and delete test records without leaving state - [ ] Domain tests read existing domain data and verify field types (DomainID is int64 after #6) - [ ] Tests skip gracefully when credentials are not set (`t.Skip`) - [ ] Tests handle 429 rate limiting via test-only retry helper with backoff ### Test Expectations - [ ] All integration tests pass against live API - [ ] No test leaves orphaned DNS records - [ ] Tests are idempotent (can run multiple times) ### Constraints - Use `testing.Short()` or env var gate — tests must not run without credentials - Test against a real domain Lucas owns (e.g., `palinks.app`) - Use TXT records for write tests (safest — won't affect DNS resolution) - Clean up all created records in test teardown (`t.Cleanup`) - 429 retry is a test-only helper, NOT changes to the production client ### Checklist - [ ] client_test.go with env var helpers and 429 retry utility - [ ] dns_test.go with tests for all 6 methods - [ ] domains_test.go with list/get tests - [ ] All tests pass locally - [ ] No orphaned test state ### Related - Depends on #3 (merged) - Should land after #6 (DomainID type fix) - `project-godaddy-tofu` - `docs/dns-endpoints.md` — endpoint reference - `docs/auth.md` — credential setup
ldraney commented 2026-06-14 16:45:05 +00:00
Author
Owner
Copy link

Scope Review: NEEDS_REFINEMENT

Review note: review-1425-2026-06-14

Ticket is well-structured with all template sections present, but has 5 [BODY] fixes and 2 [SCOPE] gaps to resolve before moving to next_up.

Issues found:

  • Wrong dependency -- Lineage says "Depends on #2" (docs PR) but should say "Depends on #3" (Go scaffold)
  • Test name mismatch -- TestAddAndDeleteRecord (singular) but method is AddRecords (plural, takes []DNSRecord slice)
  • #6 ordering -- Issue #6 (DomainID float64->int64) should land first or be acknowledged, otherwise domain tests will break when #6 merges
  • 429 AC is ambiguous -- Client has no retry/backoff logic. Tests can't "handle 429 with backoff" without it. Either descope this AC or expand scope to include client retry
  • Incomplete DNS coverage -- Test plan covers 4 of 6 DNS methods; ReplaceAllRecords and ReplaceRecordsByType are unaddressed
  • Missing backing notes -- story-godaddy-tofu-dns-iac and arch-godaddy-tofu notes don't exist in pal-e-docs (project page links to the story but note was never created)
## Scope Review: NEEDS_REFINEMENT Review note: `review-1425-2026-06-14` Ticket is well-structured with all template sections present, but has 5 `[BODY]` fixes and 2 `[SCOPE]` gaps to resolve before moving to next_up. **Issues found:** - **Wrong dependency** -- Lineage says "Depends on #2" (docs PR) but should say "Depends on #3" (Go scaffold) - **Test name mismatch** -- `TestAddAndDeleteRecord` (singular) but method is `AddRecords` (plural, takes `[]DNSRecord` slice) - **#6 ordering** -- Issue #6 (DomainID float64->int64) should land first or be acknowledged, otherwise domain tests will break when #6 merges - **429 AC is ambiguous** -- Client has no retry/backoff logic. Tests can't "handle 429 with backoff" without it. Either descope this AC or expand scope to include client retry - **Incomplete DNS coverage** -- Test plan covers 4 of 6 DNS methods; `ReplaceAllRecords` and `ReplaceRecordsByType` are unaddressed - **Missing backing notes** -- `story-godaddy-tofu-dns-iac` and `arch-godaddy-tofu` notes don't exist in pal-e-docs (project page links to the story but note was never created)
ldraney commented 2026-06-14 16:46:10 +00:00
Author
Owner
Copy link

Scope refinement applied (review-1425-2026-06-14):

  1. Lineage — fixed from "Depends on #2" to "Depends on #3", added #6 as soft dependency
  2. Test naming — TestAddAndDeleteRecord → TestAddAndDeleteRecords to match AddRecords method signature
  3. Test coverage — added TestReplaceRecordsByType and TestReplaceAllRecords to cover all 6 DNS methods (was 4 of 6)
  4. 429 handling — clarified as test-only retry helper, not client-level retry logic
  5. #6 dependency — noted explicitly in lineage and domain test AC to avoid writing tests against stale float64 DomainID
**Scope refinement applied (review-1425-2026-06-14):** 1. **Lineage** — fixed from "Depends on #2" to "Depends on #3", added #6 as soft dependency 2. **Test naming** — `TestAddAndDeleteRecord` → `TestAddAndDeleteRecords` to match `AddRecords` method signature 3. **Test coverage** — added `TestReplaceRecordsByType` and `TestReplaceAllRecords` to cover all 6 DNS methods (was 4 of 6) 4. **429 handling** — clarified as test-only retry helper, not client-level retry logic 5. **#6 dependency** — noted explicitly in lineage and domain test AC to avoid writing tests against stale `float64` DomainID
ldraney commented 2026-06-14 16:48:55 +00:00
Author
Owner
Copy link

Scope Review: APPROVED

Review note: review-1425-2026-06-14-r2

Re-review after refinement. All 5 body fixes from the prior review have been applied and verified against the codebase. Ticket is ready for next_up.

Verified:

  • All 12 template sections present and complete
  • All 6 DNS methods covered by test plan (verified against dns.go signatures)
  • Both domain methods covered (ListDomains, GetDomain)
  • File targets correct (3 new _test.go files in pkg/godaddy/)
  • Dependencies documented (#3 merged/done, #6 soft dependency noted in lineage and AC)
  • 6 acceptance criteria, all agent-verifiable
  • No decomposition needed (3 files, 1 repo, under 5 minutes)

Remaining [SCOPE] items (docs-level, not ticket blockers):

  • Create story note story-godaddy-tofu-dns-iac in pal-e-docs
  • Create architecture note arch-godaddy-tofu in pal-e-docs
## Scope Review: APPROVED Review note: `review-1425-2026-06-14-r2` Re-review after refinement. All 5 body fixes from the prior review have been applied and verified against the codebase. Ticket is ready for next_up. **Verified:** - All 12 template sections present and complete - All 6 DNS methods covered by test plan (verified against `dns.go` signatures) - Both domain methods covered (ListDomains, GetDomain) - File targets correct (3 new `_test.go` files in `pkg/godaddy/`) - Dependencies documented (#3 merged/done, #6 soft dependency noted in lineage and AC) - 6 acceptance criteria, all agent-verifiable - No decomposition needed (3 files, 1 repo, under 5 minutes) **Remaining [SCOPE] items (docs-level, not ticket blockers):** - Create story note `story-godaddy-tofu-dns-iac` in pal-e-docs - Create architecture note `arch-godaddy-tofu` in pal-e-docs
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
docs/keycloak-public-ingress
No results found.
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
ldraney/godaddy-tofu#4
No description provided.