Skip to content

Add missing API validations tests#708

Open
mandre wants to merge 19 commits intok-orc:mainfrom
shiftstack:add-api-validations-tests
Open

Add missing API validations tests#708
mandre wants to merge 19 commits intok-orc:mainfrom
shiftstack:add-api-validations-tests

Conversation

@mandre
Copy link
Collaborator

@mandre mandre commented Mar 12, 2026

This add tests for all existing resources.

Assisted-By: claude-opus-4-6

@github-actions github-actions bot added the semver:patch No API change label Mar 12, 2026
@mandre mandre marked this pull request as ready for review March 13, 2026 13:26
@winiciusallan
Copy link
Contributor

winiciusallan commented Mar 14, 2026

I've been getting familiar with the apivalidations package. Very cool.

What do you think about adding some instructions to the documentation as a new step in "Post-scaffolding" section, so we encourage (and require) newcomers to add apivalidations specs to their resources as long as they need?

@mandre
Copy link
Collaborator Author

mandre commented Mar 19, 2026

What do you think about adding some instructions to the documentation as a new step in "Post-scaffolding" section, so we encourage (and require) newcomers to add apivalidations specs to their resources as long as they need?

Good call. I'll add it do the docs and skills definition.

@github-actions github-actions bot added semver:major Breaking change and removed semver:patch No API change labels Mar 19, 2026
mandre added 19 commits March 19, 2026 15:07
Extract duplicated management policy validation tests from all 22
resource test files into a generic runManagementPolicyTests helper in
common_test.go. This reduces ~100 lines of duplicated test code per
file to ~13 lines of resource-specific callbacks.

The shared helper covers all standard management policy validations:
- Default management policy is managed
- Require import for unmanaged
- Not permit unmanaged with resource
- Not permit empty import
- Not permit empty import filter
- Permit valid import filter
- Require resource for managed
- Not permit managed with import
- Not permit managedOptions for unmanaged
- Permit managedOptions for managed

For network, port, and subnet, this also adds previously missing
management policy test coverage.

Disable the dupl linter for test/ since the remaining per-resource
callback boilerplate is inherently similar.
Add a template that generates test/apivalidations/<resource>_test.go when
scaffolding a new controller. The generated test file includes management
policy tests via the shared runManagementPolicyTests helper, required field
validation, and immutability tests for all dependency refs.
Update scaffolding.md to list the generated API validation test file,
expand the API validation tests section in writing-tests.md with guidance
on what to test and how, and update the new-controller skill with Step 7
instructions and a checklist item for API validation tests.
@mandre mandre force-pushed the add-api-validations-tests branch from 48ee7b4 to 250c4d8 Compare March 19, 2026 14:07
@github-actions github-actions bot added semver:patch No API change and removed semver:major Breaking change labels Mar 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver:patch No API change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants