Skip to content

[DO NOT MERGE] Add FileShare TypeSpec setup, tests & tooling#29256

Open
ankushbindlish2 wants to merge 2 commits intoAzure:mainfrom
ankushbindlish2:add-fileshares-test
Open

[DO NOT MERGE] Add FileShare TypeSpec setup, tests & tooling#29256
ankushbindlish2 wants to merge 2 commits intoAzure:mainfrom
ankushbindlish2:add-fileshares-test

Conversation

@ankushbindlish2
Copy link
Member

Add prerequisites documentation and installer script for TypeSpec-based FileShare development (PREREQUISITES-STATUS.md, check-and-install-prerequisites.ps1) and add eng package manifest to pull TypeSpec packages. Include generated Pester module files and related test assets for FileShare, plus test environment templates, test helpers, sample implementation, and a test plan/status. Update FileShare project metadata and docs (AssemblyInfo, Az.FileShare.psd1, Az.FileShare.md) and adjust several FileShare test scripts. Also remove obsolete EventGrid generate-info.json.

Description

Mandatory Checklist

  • SHOULD update ChangeLog.md file(s) appropriately
    • Update src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md.
      • A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header in the past tense.
    • Should not change ChangeLog.md if no new release is required, such as fixing test case only.
  • SHOULD regenerate markdown help files if there is cmdlet API change. Instruction
  • SHOULD have proper test coverage for changes in pull request.
  • SHOULD NOT adjust version of module manually in pull request

Add prerequisites documentation and installer script for TypeSpec-based FileShare development (PREREQUISITES-STATUS.md, check-and-install-prerequisites.ps1) and add eng package manifest to pull TypeSpec packages. Include generated Pester module files and related test assets for FileShare, plus test environment templates, test helpers, sample implementation, and a test plan/status. Update FileShare project metadata and docs (AssemblyInfo, Az.FileShare.psd1, Az.FileShare.md) and adjust several FileShare test scripts. Also remove obsolete EventGrid generate-info.json.
Copilot AI review requested due to automatic review settings March 10, 2026 08:35
@azure-client-tools-bot-prd
Copy link

Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status.

@NoriZC
Copy link
Contributor

NoriZC commented Mar 10, 2026

/azp run

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds/activates FileShare Autorest Pester tests and introduces JSON-based test environment assets to support TypeSpec-based FileShare development and validation.

Changes:

  • Load test environment variables from JSON and optionally create the test resource group in setupEnv.
  • Un-skip multiple FileShare cmdlet Pester tests and implement live assertions.
  • Add JSON request/asset files for tests and include local/env templates.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 15 comments.

Show a summary per file
File Description
src/FileShare/FileShare.Autorest/test/utils.ps1 Loads env values from JSON and creates RG if needed; writes env back to disk
src/FileShare/FileShare.Autorest/test/test-snapshot-update.json Adds snapshot update request payload asset
src/FileShare/FileShare.Autorest/test/test-recommendation.json Adds provisioning recommendation request payload asset
src/FileShare/FileShare.Autorest/test/test-availability.json Adds name availability request payload asset
src/FileShare/FileShare.Autorest/test/localEnv.json Adds local live-test environment values
src/FileShare/FileShare.Autorest/test/env.json.template Adds template env file for non-local scenarios
src/FileShare/FileShare.Autorest/test/env.json Adds env file used by tests when not in live mode
src/FileShare/FileShare.Autorest/test/Update-AzFileShareSnapshot.Tests.ps1 Implements Update snapshot tests (expanded/json/identity variants)
src/FileShare/FileShare.Autorest/test/Update-AzFileShare.Tests.ps1 Implements Update file share tests (expanded/json/identity variants)
src/FileShare/FileShare.Autorest/test/Test-AzFileShareNameAvailability.Tests.ps1 Implements name availability tests (expanded/json/body/identity variants)
src/FileShare/FileShare.Autorest/test/Remove-AzFileShareSnapshot.Tests.ps1 Implements snapshot delete tests (expanded/identity variants)
src/FileShare/FileShare.Autorest/test/Remove-AzFileShare.Tests.ps1 Implements file share delete tests
src/FileShare/FileShare.Autorest/test/New-AzFileShareSnapshot.Tests.ps1 Implements snapshot create tests (expanded/json/identity variants)
src/FileShare/FileShare.Autorest/test/New-AzFileShare.Tests.ps1 Implements file share create tests (expanded/json variants)
src/FileShare/FileShare.Autorest/test/Get-AzFileShareUsageData.Tests.ps1 Implements usage data tests
src/FileShare/FileShare.Autorest/test/Get-AzFileShareSnapshot.Tests.ps1 Implements snapshot get/list tests
src/FileShare/FileShare.Autorest/test/Get-AzFileShareProvisioningRecommendation.Tests.ps1 Implements provisioning recommendation tests (expanded/json/body/identity variants)
src/FileShare/FileShare.Autorest/test/Get-AzFileShareLimit.Tests.ps1 Implements limit tests (expanded/identity variants)
src/FileShare/FileShare.Autorest/test/Get-AzFileShare.Tests.ps1 Implements file share get/list tests

Comment on lines +53 to +60
$envFilePath = Join-Path $PSScriptRoot $envFile
if (Test-Path -Path $envFilePath) {
$envData = Get-Content $envFilePath | ConvertFrom-Json
$envData.psobject.properties | ForEach-Object {
if (-not $env.Contains($_.Name)) {
$env[$_.Name] = $_.Value
}
}
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Get-Content $envFilePath | ConvertFrom-Json will pass a string array (one item per line) for pretty-printed JSON files; ConvertFrom-Json expects a single JSON string and can fail. Use Get-Content -Raw (or equivalent) to read the whole file as a single string before converting.

Copilot uses AI. Check for mistakes.
Comment on lines +63 to +74
# Create test resource group if it doesn't exist
if ($env.resourceGroup -and $env.location) {
Write-Host "Checking for resource group: $($env.resourceGroup)"
$rg = Get-AzResourceGroup -Name $env.resourceGroup -ErrorAction SilentlyContinue
if (-not $rg) {
Write-Host "Creating resource group: $($env.resourceGroup) in location: $($env.location)"
New-AzResourceGroup -Name $env.resourceGroup -Location $env.location | Out-Null
Write-Host "Resource group created successfully"
} else {
Write-Host "Resource group already exists"
}
}
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resource group creation runs regardless of $TestMode. If this harness supports playback/recording modes, these live Azure calls can break non-live test runs. Gate the RG create/check logic to live mode only (or whatever mode is intended to touch Azure), and keep non-live modes purely data-driven.

Copilot uses AI. Check for mistakes.
}

# For any resources you created for test, you should add it to $env here.
set-content -Path (Join-Path $PSScriptRoot $envFile) -Value (ConvertTo-Json $env)
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This writes back to $envFile under the repo directory. In non-live modes you set $envFile = 'env.json', which is tracked in this PR—test runs will modify the working tree and/or produce dirty diffs. Consider writing only to a local, gitignored file (e.g., localEnv.json), or writing to a temporary location like $TestDrive, and avoid mutating tracked test assets.

Copilot uses AI. Check for mistakes.
Comment on lines +14 to +21
"Tenant": "72f988bf-86f1-41af-91ab-2d7cd011db47",
"testFileShareQuota": 5120,
"fileShareName01": "testshare01",
"testProvisionedIoPerSec": 4024,
"testPublicNetworkAccess": "Enabled",
"testRootSquash": "NoRootSquash",
"testMediaTier": "SSD",
"SubscriptionId": "edefbf29-8c80-4e37-bb49-41432d16a3fe",
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file appears to contain real tenant/subscription GUIDs and environment-specific resource names. These identifiers shouldn’t be committed as defaults in a public/shared repo. Recommend converting this into a template with placeholders, committing only a *.template example, and ensuring the real local env file is excluded (e.g., via .gitignore).

Suggested change
"Tenant": "72f988bf-86f1-41af-91ab-2d7cd011db47",
"testFileShareQuota": 5120,
"fileShareName01": "testshare01",
"testProvisionedIoPerSec": 4024,
"testPublicNetworkAccess": "Enabled",
"testRootSquash": "NoRootSquash",
"testMediaTier": "SSD",
"SubscriptionId": "edefbf29-8c80-4e37-bb49-41432d16a3fe",
"Tenant": "TENANT_ID_PLACEHOLDER",
"testFileShareQuota": 5120,
"fileShareName01": "testshare01",
"testProvisionedIoPerSec": 4024,
"testPublicNetworkAccess": "Enabled",
"testRootSquash": "NoRootSquash",
"testMediaTier": "SSD",
"SubscriptionId": "SUBSCRIPTION_ID_PLACEHOLDER",

Copilot uses AI. Check for mistakes.
Comment on lines +14 to +21
"Tenant": "72f988bf-86f1-41af-91ab-2d7cd011db47",
"testFileShareQuota": 5120,
"fileShareName01": "testshare01",
"testProvisionedIoPerSec": 4024,
"testPublicNetworkAccess": "Enabled",
"testRootSquash": "NoRootSquash",
"testMediaTier": "SSD",
"SubscriptionId": "edefbf29-8c80-4e37-bb49-41432d16a3fe",
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file appears to contain real tenant/subscription GUIDs and environment-specific resource names. These identifiers shouldn’t be committed as defaults in a public/shared repo. Recommend converting this into a template with placeholders, committing only a *.template example, and ensuring the real local env file is excluded (e.g., via .gitignore).

Suggested change
"Tenant": "72f988bf-86f1-41af-91ab-2d7cd011db47",
"testFileShareQuota": 5120,
"fileShareName01": "testshare01",
"testProvisionedIoPerSec": 4024,
"testPublicNetworkAccess": "Enabled",
"testRootSquash": "NoRootSquash",
"testMediaTier": "SSD",
"SubscriptionId": "edefbf29-8c80-4e37-bb49-41432d16a3fe",
"Tenant": "00000000-0000-0000-0000-000000000000",
"testFileShareQuota": 5120,
"fileShareName01": "testshare01",
"testProvisionedIoPerSec": 4024,
"testPublicNetworkAccess": "Enabled",
"testRootSquash": "NoRootSquash",
"testMediaTier": "SSD",
"SubscriptionId": "00000000-0000-0000-0000-000000000000",

Copilot uses AI. Check for mistakes.
Comment on lines +41 to +49
It 'GetViaJsonFilePath' {
{
$jsonFilePath = Join-Path $PSScriptRoot 'test-recommendation.json'
$jsonContent = @{
targetStorageGiB = 1000
targetIoPerSec = 3000
targetThroughputMiBPerSec = 125
} | ConvertTo-Json -Depth 10
Set-Content -Path $jsonFilePath -Value $jsonContent
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test-recommendation.json is introduced as a repo file, but the test rewrites and deletes it. This can break other tests and/or leave the repo dirty. Prefer a temp file path (e.g., $TestDrive) or treat the committed JSON file as a read-only fixture.

Copilot uses AI. Check for mistakes.
Comment on lines +54 to +55
Remove-Item -Path $jsonFilePath -Force -ErrorAction SilentlyContinue
} | Should -Not -Throw
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test-recommendation.json is introduced as a repo file, but the test rewrites and deletes it. This can break other tests and/or leave the repo dirty. Prefer a temp file path (e.g., $TestDrive) or treat the committed JSON file as a read-only fixture.

Copilot uses AI. Check for mistakes.
Comment on lines +18 to +26
It 'UpdateExpanded' {
{
$config = Update-AzFileShareSnapshot -ResourceGroupName $env.resourceGroup `
-ResourceName $env.fileShareName01 `
-Name $env.snapshotName01 `
-Tag @{"updated" = "true"; "environment" = "production"}
$config.Name | Should -Be $env.snapshotName01
$config.Tag["updated"] | Should -Be "true"
} | Should -Not -Throw
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test assumes $env.snapshotName01 already exists but this file never creates it. That creates cross-file/order dependencies (e.g., on New-AzFileShareSnapshot.Tests.ps1) and makes the suite flaky when tests run independently or in different order. Create the snapshot in a BeforeAll inside this file (and clean it up in AfterAll), ideally using a unique name per run.

Copilot uses AI. Check for mistakes.
Comment on lines +18 to 24
It 'Delete' {
{
Remove-AzFileShare -ResourceGroupName $env.resourceGroup -ResourceName $env.fileShareName03 -PassThru
$config = Get-AzFileShare -ResourceGroupName $env.resourceGroup -ResourceName $env.fileShareName03 -ErrorAction SilentlyContinue
$config | Should -BeNullOrEmpty
} | Should -Not -Throw
}
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This deletes a shared, env-defined file share name without setting it up in this test file. If test execution order changes (or tests are run in isolation), the share may not exist or may be needed by other tests, causing instability. Prefer creating a dedicated file share for this delete test within the same file (unique name), then deleting it as part of the test/AfterAll.

Copilot uses AI. Check for mistakes.
Comment on lines +18 to 23
It 'List' {
{
$config = Get-AzFileShare -ResourceGroupName $env.resourceGroup
$config.Count | Should -BeGreaterThan 0
} | Should -Not -Throw
}
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Asserting .Count -BeGreaterThan 0 makes the test dependent on pre-existing state in the resource group. This can fail in clean environments or after other tests delete resources. Consider asserting that the call succeeds and (if needed) that it contains a specifically created/known file share from a BeforeAll in this file.

Copilot uses AI. Check for mistakes.
{ throw [System.NotImplementedException] } | Should -Not -Throw
It 'CreateExpanded' {
{
$config = New-AzFileShare -ResourceName $env.fileShareName01 `
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to cover all none-common parameters. Like it's better to cover "-PublicAccessPropertyAllowedSubnet" in create file share.

$config.Name | Should -Be $env.fileShareName01
$config.ProvisioningState | Should -Be "Succeeded"
$config.MediaTier | Should -Be "SSD"
$config.Protocol | Should -Be "NFS"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to check all (most) parameters which customer will input and server will return.

- Add CRUD operation tests
- Add edge case scenarios tests
- Add negative/error handling tests
- Add pipeline functionality tests
- Add private endpoint tests
- Add resource move tests
- Add complex scenario tests
- Add test utilities
@NoriZC
Copy link
Contributor

NoriZC commented Mar 10, 2026

/azp run

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants