Skip to content

Commit 3dfda9f

Browse files
authored
chore: updated ai instruction set and readme (#789)
# Pull Request Description ## Summary [Provide a brief description of the changes in this PR] ### Issue Reference Fixes #[Issue Number] ### Motivation and Context - Why is this change needed? - What problem does it solve? - If it fixes an open issue, please link to the issue here ### Dependencies - List any dependencies that are required for this change - Include any configuration changes needed - Note any version updates required ## Type of Change Please mark the relevant option with an `x`: - [ ] 🐛 Bug fix (non-breaking change which fixes an issue) - [ ] ✨ New feature (non-breaking change which adds functionality) - [ ] 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] 📝 Documentation update (Wiki/README/Code comments) - [ ] ♻️ Refactor (code improvement without functional changes) - [ ] 🎨 Style update (formatting, renaming) - [ ] 🔧 Configuration change - [ ] 📦 Dependency update ## Testing - [ ] I have added unit tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] I have tested this code in the following browsers/environments: [list environments] ## Quality Checklist - [ ] I have reviewed my own code before requesting review - [ ] I have verified there are no other open Pull Requests for the same update/change - [ ] All CI/CD pipelines pass without errors or warnings - [ ] My code follows the established style guidelines of this project - [ ] I have added necessary documentation (if appropriate) - [ ] I have commented my code, particularly in complex areas - [ ] I have made corresponding changes to the README and other relevant documentation - [ ] My changes generate no new warnings - [ ] I have performed a self-review of my own code - [ ] My code is properly formatted according to project standards ## Screenshots/Recordings (if appropriate) [Add screenshots or recordings that demonstrate the changes] ## Additional Notes [Add any additional information that might be helpful for reviewers]
2 parents 8cd9c41 + 82abbf9 commit 3dfda9f

File tree

2 files changed

+108
-50
lines changed

2 files changed

+108
-50
lines changed

.github/ai-instructions.md

Lines changed: 107 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -339,27 +339,56 @@ Use the Terraform plugin logger (`tflog`) for logging within resource implementa
339339

340340
## Testing Best Practices
341341

342-
- **Unit Tests:** For each new resource or data source, write unit tests covering all operations and edge cases. Use the `jarcoal/httpmock` library (already in the project) to simulate HTTP API responses.
343-
- Register **mock responders** for every HTTP call that the Create, Read, Update, or Delete functions will make. Each test step should set up the expected API responses (e.g. mock the POST response for Create, GET for Read, etc.).
344-
- **Test Steps Lifecycle:** Structure unit tests in sequential steps to simulate resource lifecycle transitions:
345-
- **Step 1 (Create):** Call the resource's Create, then Read. Verify that after Create, the state read back includes all the created fields/attributes.
346-
- **Step 2 (Update):** Call Read (to get current state), then Update, then Read again. Ensure the first Read in this step matches the final state from the previous step, and the final Read reflects the updates applied.
347-
- **Step 3 (Delete):** Call Delete, then Read. After deletion, the final Read should return a "not found" error (e.g. 404) indicating the resource is gone.
348-
- If the resource supports import, write a dedicated test (single step) that calls the Read (or Import) with a given `ImportStateId` and verifies Terraform state import logic.
349-
- Include negative test cases: simulate API errors (like 403 Forbidden or 500 Internal Server Error) and ensure the provider surfaces appropriate errors. Also test validation logic (e.g., providing an invalid parameter returns an error).
350-
- Place JSON fixtures for mock responses in the appropriate test data directory (e.g. `internal/resources/<service>/test/<resource>/<scenario>/response.json`). **Do not use real customer data** in tests – anonymize any IDs or personal info in your dummy data.
351-
- Name unit test functions with the `TestUnit` prefix as mentioned, and keep them in a `_test.go` file using the `<package>_test` package name.
352-
- All the JSON response for unit tests should be stored in .json files:
353-
- Files should be placed in a folder with a name corresponding to the Unit Test that is being used. Folder name should omit `UnitTest` in its name.
354-
- Each Unit Test folder with .json files should be stored at `resources\{service_name}\test\resource` or `services\{service_name}\test\datasource` with all other resource and/or datasource .go files.
355-
- The .json file name should consist of the mock request method (`get`, `post`, `delete`) followed by `_` and name of the returned mock object name or action.
356-
- The file names have to be sensible without empty spaces and special characters.
357-
358-
- **Acceptance Tests:** Add acceptance tests for any new resource covering the same scenarios as unit tests, but against real Microsoft365 resources. These tests live in files with the `TestAcc...` prefix and require real credentials.
359-
- **IMPORTANT: If you don't have access to a test tenant, DO NOT modify, rename, or remove existing acceptance tests.** Focus exclusively on writing unit tests instead. Existing acceptance tests have been verified to work correctly and modifying them without the ability to test against a real Microsoft 365 environment can break the test suite.
360-
- Wrap any acceptance test with appropriate pre-check functions and environment variable checks so it skips if not configured.
361-
- Ensure each acceptance test cleans up after itself. Use `CheckDestroy` functions to verify that resources are actually deleted in Azure/Microsoft365 after the test run.
362-
- Keep acceptance tests focused and isolated (use separate environment or resource names to avoid conflicts).
342+
- **Unit Tests:** For each new resource or data source, write unit tests in `resource_test.go` covering all operations and edge cases. Use the `jarcoal/httpmock` library to simulate HTTP API responses with resource-specific mock responders.
343+
- **Test File Organization:**
344+
- **Unit Tests:** Place in `resource_test.go` with `TestUnit*` naming pattern
345+
- **Package Naming:** Use `<package>_test` suffix (e.g., `graphBetaWindowsAutopilotDeploymentProfile_test`)
346+
- **Mock Setup:** Create `setupMockEnvironment()` and `setupErrorMockEnvironment()` functions
347+
- **Mock Responders:** Store in `mocks/responders.go` file within resource directory
348+
- **Mock Infrastructure:**
349+
- Register **mock responders** for every HTTP call using resource-specific mock structs
350+
- Use `httpmock.Activate()` and `defer httpmock.DeactivateAndReset()` pattern
351+
- Call `mocks.SetupUnitTestEnvironment(t)` for consistent test environment setup
352+
- Use `mocks.TestUnitTestProtoV6ProviderFactories` for provider factories
353+
- **Test Steps Lifecycle:** Structure unit tests using `resource.UnitTest()` with sequential steps:
354+
- **Step 1 (Create):** Verify resource creation and initial state population
355+
- **Step 2 (Update):** Test resource modification and state updates
356+
- **Step 3 (Delete):** Confirm resource deletion and state cleanup
357+
- **Import Step:** Test import functionality with `ImportState: true` and `ImportStateId`
358+
- **Test Data Organization:**
359+
- Store JSON mock responses in `tests/responses/[operation]/[method]_[object]_[scenario].json`
360+
- Example: `tests/responses/validate_create/post_windows_autopilot_deployment_profile_success.json`
361+
- Operations: `validate_create`, `validate_get`, `validate_update`, `validate_delete`
362+
- **Do not use real customer data** – anonymize all IDs and personal information
363+
- Include negative test cases with error mock responses and validation testing
364+
- Use `testCheckExists()` helper function for resource existence verification
365+
366+
- **Acceptance Tests:** Add acceptance tests in `resource_acceptance_test.go` for new resources using real Microsoft365 API calls. These require valid authentication credentials and test environment.
367+
- **Test File Organization:**
368+
- **Acceptance Tests:** Place in `resource_acceptance_test.go` with `TestAcc*` naming pattern
369+
- **Package Naming:** Use same `<package>_test` pattern as unit tests
370+
- **Environment Setup:** Use `mocks.TestAccPreCheck(t)` for pre-flight checks
371+
- **Provider Setup:** Use `mocks.TestAccProtoV6ProviderFactories` for provider factories
372+
- **Centralized Test Infrastructure:**
373+
- Import and use helpers from `internal/acceptance/` package
374+
- Use `acceptance.TestGraphClient()` for direct Graph API client access when needed
375+
- Leverage `acceptance.ProviderConfigBuilder` for dynamic provider configuration
376+
- Use centralized environment variable validation and setup functions
377+
- **Test Structure:**
378+
- Use `resource.Test()` with real API calls against Microsoft 365
379+
- Include `PreCheck: func() { mocks.TestAccPreCheck(t) }` for environment validation
380+
- Add `CheckDestroy` functions to verify resource cleanup: `testAccCheck[ResourceName]Destroy`
381+
- Use `ExternalProviders` map for dependencies like `azuread` and `random` providers
382+
- **Test Scenarios:**
383+
- Test minimal and maximal resource configurations
384+
- Verify resource lifecycle (create, read, update, delete operations)
385+
- Test import functionality and state consistency
386+
- Include assignment testing for resources with assignment capabilities
387+
- **Environment Requirements:**
388+
- **IMPORTANT: If you don't have access to a test tenant, DO NOT modify existing acceptance tests**
389+
- Focus on unit tests only when lacking test environment access
390+
- Acceptance tests require valid `M365_*` environment variables for authentication
391+
- Use resource naming with random suffixes to avoid conflicts between parallel test runs
363392

364393
- **Test Coverage:** Aim for **at least 80%** code coverage for unit tests on new code. `make unittest` will return a coverage score by service and overall. Focus on the service that is currently being worked on when adding tests to improve coverage.
365394

@@ -409,24 +438,36 @@ Use the Terraform plugin logger (`tflog`) for logging within resource implementa
409438

410439
### Test Infrastructure Components
411440

412-
- **Mock System (`/internal/mocks/`):**
413-
- `AuthenticationMocks`: Mock authentication endpoints
414-
- `MockGraphClients`: Mock Microsoft Graph API clients
415-
- Resource-specific mock responders in `mocks/responders.go`
416-
- Terraform configuration files in `mocks/terraform/`
417-
- Sophisticated state management across CRUD operations
418-
419-
- **Test Configurations:**
420-
- **Minimal:** `resource_minimal.tf` - Tests basic required fields
421-
- **Maximal:** `resource_maximal.tf` - Tests all optional fields
422-
- **Error:** Test configurations for error scenarios
423-
424-
- **Test Helpers:**
425-
- `setupTestEnvironment()`: Configure test environment variables
426-
- `setupMockEnvironment()`: Activate HTTP mocking
427-
- `testCheckExists()`: Verify resource existence in state
428-
- `testAccPreCheck()`: Validate required environment variables
429-
- `testAccCheckResourceDestroy()`: Verify resource cleanup
441+
- **Global Mock System (`/internal/mocks/`):**
442+
- `AuthenticationMocks`: Mock authentication endpoints and token exchanges
443+
- `MockGraphClients`: Mock Microsoft Graph API clients for unit testing
444+
- `TestUnitTestProtoV6ProviderFactories`: Provider factories for unit tests
445+
- `TestAccProtoV6ProviderFactories`: Provider factories for acceptance tests
446+
- `SetupUnitTestEnvironment(t)`: Standardized unit test environment setup
447+
- `TestAccPreCheck(t)`: Validate acceptance test environment and credentials
448+
449+
- **Resource-Specific Mock System (per resource directory):**
450+
- **Mock Responders:** `mocks/responders.go` - HTTP mock registration for specific resource endpoints
451+
- **Mock State Management:** Sophisticated state tracking across CRUD operations within mock structs
452+
- **Setup Functions:** `setupMockEnvironment()` and `setupErrorMockEnvironment()` in resource test files
453+
454+
- **Test Data Organization (per resource directory):**
455+
- **JSON Response Files:** `tests/responses/[operation]/[method]_[object]_[scenario].json`
456+
- **Operation Directories:** `validate_create/`, `validate_get/`, `validate_update/`, `validate_delete/`
457+
- **File Naming:** `[method]_[object]_[scenario].json` (e.g., `post_windows_autopilot_deployment_profile_success.json`)
458+
- **Scenario Coverage:** Success, error, update, and hybrid configurations
459+
460+
- **Centralized Acceptance Test Infrastructure (`/internal/acceptance/`):**
461+
- **Client Factory:** `TestGraphClient()` - Creates authenticated Graph clients for acceptance tests
462+
- **Provider Configuration:** `ProviderConfigBuilder` - Dynamic provider configuration generation
463+
- **Environment Helpers:** Provider configuration and environment variable management
464+
- **Terraform Dependencies:** Centralized external provider dependency management
465+
466+
- **Test Helper Functions (per resource):**
467+
- `setupMockEnvironment()`: Activate HTTP mocking with resource-specific responders
468+
- `testCheckExists(resourceName)`: Verify resource existence using `TestCheckResourceAttrSet`
469+
- `testAccCheck[ResourceName]Destroy`: Verify resource cleanup after acceptance tests
470+
- `testAccConfig[Scenario]()`: Generate Terraform configurations for acceptance test scenarios
430471

431472
### Required Environment Variables for Acceptance Tests
432473

@@ -440,16 +481,33 @@ M365_CLIENT_ID # Application client ID
440481

441482
### Testing Best Practices Specific to This Provider
442483

443-
- **Mock Data Organization:** Store mock responses in `tests/` subdirectory within resource directory
444-
- Folder structure: `tests/[TestScenario]/[method]_[object].json`
445-
- Example: `tests/Validate_Create/post_device_shell_script.json`
446-
447-
- **State Management Testing:** Verify Terraform state correctly reflects API responses
448-
- **Drift Detection:** Test that changes made outside Terraform are detected
449-
- **Import Testing:** Verify resource import functionality works correctly
450-
- **Edge Case Testing:** Test minimal configurations, maximal configurations, and error scenarios
451-
- **Eventual Consistency:** Use `ReadWithRetry` after create/update operations
452-
- **Permission Testing:** Verify appropriate error messages for insufficient permissions
484+
- **Mock Data Organization:** Store mock responses in resource-specific directory structure
485+
- **Directory Structure:** `tests/responses/[operation]/[method]_[object]_[scenario].json`
486+
- **Operations:** `validate_create`, `validate_get`, `validate_update`, `validate_delete`
487+
- **Example:** `tests/responses/validate_create/post_windows_autopilot_deployment_profile_success.json`
488+
- **Scenario Coverage:** Include success, error, minimal, maximal, and hybrid scenarios
489+
490+
- **Separated Test Files:** Use distinct files for different test types
491+
- **Unit Tests:** `resource_test.go` with `TestUnit*` functions and httpmock
492+
- **Acceptance Tests:** `resource_acceptance_test.go` with `TestAcc*` functions and real API calls
493+
- **Package Naming:** Both use `<package>_test` suffix for external interface testing
494+
495+
- **Mock Infrastructure:** Implement resource-specific mock systems
496+
- **Mock Responders:** Create `mocks/responders.go` with resource-specific mock struct
497+
- **State Tracking:** Implement sophisticated state management within mock responders for CRUD operations
498+
- **Error Scenarios:** Include `RegisterErrorMocks()` methods for testing error conditions
499+
500+
- **Centralized Acceptance Testing:** Leverage shared acceptance test infrastructure
501+
- **Client Access:** Use `acceptance.TestGraphClient()` for direct Graph API operations when needed
502+
- **Provider Configuration:** Use `acceptance.ProviderConfigBuilder` for dynamic test configurations
503+
- **External Dependencies:** Use centralized `ExternalProviders` map for azuread, random, etc.
504+
505+
- **State Management Testing:** Verify Terraform state correctly reflects API responses across operations
506+
- **Assignment Testing:** Include assignment validation for resources with assignment capabilities
507+
- **Import Testing:** Verify resource import functionality with `ImportState: true` and `ImportStateId`
508+
- **Edge Case Testing:** Test minimal/maximal configurations, validation errors, and API error scenarios
509+
- **Eventual Consistency:** Use `ReadWithRetry` patterns after create/update operations for Microsoft Graph consistency
510+
- **Permission Testing:** Verify appropriate error messages and hints for insufficient permissions using error handling package
453511

454512
### Network Debugging
455513

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ Please refer to the [Getting Started](https://registry.terraform.io/providers/de
6969

7070
## Provider Comparison
7171

72-
For information and a comparison between this provider in relation to the msft official terraform-provider-msgraph provider, see the [Provider Comparison](./docs/development/provider_comparison.md) documentation.
72+
For information and a comparison between this provider in relation to the msft official `terraform-provider-msgraph provider`, see the [Provider Comparison](./docs/development/provider_comparison.md) documentation.
7373

7474
## Community Contributions
7575

0 commit comments

Comments
 (0)