docs(testing): align validation guide with current frameworks#520
docs(testing): align validation guide with current frameworks#520omribz156 wants to merge 2 commits into
Conversation
|
@microsoft-github-policy-service agree |
rezatnoMsirhC
left a comment
There was a problem hiding this comment.
Thank you for cleaning up this guide and replacing the stale Terratest examples with native terraform test guidance.
The PR description does not follow .github/PULL_REQUEST_TEMPLATE.md. Please update it to use the required sections: Description, Related Issue (with Fixes #144), Type of Change, Implementation Details, Testing Performed, Validation Steps, Checklist, and Security Review.
| |:---------------|:------------------------|:------------------------------------------------------| | ||
| | **Terraform** | native `terraform test` | One `.tftest.hcl` per component with `command = plan` | | ||
| | **Rust** | `cargo test` | `#[cfg(test)]` module covering core logic | | ||
| | **Rust** | `cargo test` | Run from each service crate with a local `Cargo.toml` | |
There was a problem hiding this comment.
The replacement text Run from each service crate with a local Cargo.toml describes how to invoke tests rather than what to write. The "Minimum Requirement" column specifies authoring requirements. The original #[cfg(test)] module covering core logic was clearer in this respect. The invocation guidance belongs in the Rust Testing section added later in this PR.
There was a problem hiding this comment.
Fixed in b2b687d: restored the Rust minimum requirement to the authoring requirement (#[cfg(test)] module covering core logic) and left invocation guidance in the Rust Testing section.
| keyVaultName := terraform.Output(t, terraformOptions, "key_vault_name") | ||
| assert.NotEmpty(t, keyVaultName) | ||
| variables { | ||
| resource_prefix = "edge-ai" |
There was a problem hiding this comment.
The example hardcodes resource_prefix = "edge-ai" and asserts azurerm_resource_group.new[0].name == "rg-edge-ai-dev-001", but the Test Data Management section directly below this example says to use setup modules for generated prefixes. The actual naming-convention.tftest.hcl uses resource_prefix = run.setup_tests.resource_prefix from a setup module run, not a hardcoded value.
The example should either use the setup module pattern to stay consistent with the actual file and the surrounding guidance, or include a note that it is a simplified illustration that intentionally deviates from the recommended pattern.
There was a problem hiding this comment.
Fixed in b2b687d: updated the example to use the setup_tests module and run.setup_tests.resource_prefix, matching the surrounding Test Data Management guidance.
| location = "East US" | ||
| enable_monitoring = true | ||
| # terraform/tests/setup/main.tf | ||
| resource "random_string" "resource_prefix" { |
There was a problem hiding this comment.
The setup module code example does not match the actual file at src/000-cloud/000-resource-group/terraform/tests/setup/main.tf. Differences:
| Field | This example | Actual file |
|---|---|---|
| Resource name | random_string.resource_prefix |
random_string.prefix |
| Length | 8 |
4 |
| Output value | random_string.resource_prefix.result |
"a${random_string.prefix.id}" |
terraform {} block |
absent | present with required_providers |
Since the PR points readers directly to this component as the reference implementation, the example should reflect the actual file contents.
There was a problem hiding this comment.
Fixed in b2b687d: replaced the setup module snippet with the actual tests/setup/main.tf structure, including the terraform block, random_string.prefix, length 4, and a${random_string.prefix.id} output value.
| - Output normalization across frameworks | ||
|
|
||
| **Reference Implementation:** [blueprints/full-single-node-cluster/tests/](https://github.com/microsoft/edge-ai/tree/main/blueprints/full-single-node-cluster/tests/) | ||
| **Reference Implementation:** [src/000-cloud/000-resource-group/terraform/tests/](https://github.com/microsoft/edge-ai/tree/main/src/000-cloud/000-resource-group/terraform/tests/) |
There was a problem hiding this comment.
This reference implementation link now points to a component (src/000-cloud/000-resource-group/), but the surrounding section is titled "Blueprint Test Architecture" and describes blueprint test patterns. Pointing readers from a blueprint architecture section to a component test directory is likely to cause confusion. Consider retaining the original blueprint reference, or updating the section heading and introductory text to reflect the component-level scope.
There was a problem hiding this comment.
Fixed in b2b687d: retitled this section to Terraform Test Reference and updated the intro/reference label so the component-level test directory is presented as a Terraform reference, not a blueprint reference implementation.
| # Terraform deployment check | ||
| terraform init | ||
| terraform plan -var-file="test.tfvars" | ||
| terraform apply -var-file="test.tfvars" -auto-approve |
There was a problem hiding this comment.
-auto-approve bypasses the plan review step and immediately provisions real Azure resources. A brief inline comment noting the cost and credential requirements before running this command would help, for example: # requires ARM_* credentials; provisions billable Azure resources.
There was a problem hiding this comment.
Fixed in b2b687d: added an inline warning before terraform apply -auto-approve that ARM_* credentials are required and the command provisions billable Azure resources.
|
Thanks, that makes sense. I updated the PR description to follow No code/docs diff changed in this follow-up; only the PR body was updated. This follow-up was Codex-assisted, with the final text reviewed before posting. |
Pull Request
Description
Updates
docs/contributing/testing-validation.mdso the testing guide reflects the current repository layout and tooling. The previous guide still referenced Terratest /go testexamples, while the repo now uses native Terraform.tftest.hcltests, Rust crate tests, and Docusaurus docs checks.Related Issue
Fixes #144
Type of Change
Implementation Details
go testguidance with nativeterraform testexamples..tftest.hcllayout undersrc/000-cloud/000-resource-group/terraform/tests/.cargo testguidance.Testing Performed
Validation Steps
Reviewers can validate this documentation-only change with:
Checklist
terraform fmton all Terraform codeterraform validateon all Terraform codeaz bicep formaton all Bicep codeaz bicep buildto validate all Bicep codeSecurity Review
Additional Notes
This was prepared with Codex assistance, with the final diff reviewed before opening.
Screenshots (if applicable)
N/A; documentation-only change.