Skip to content

Commit

Permalink
ongoing
Browse files Browse the repository at this point in the history
  • Loading branch information
Konrad Jamrozik committed Mar 2, 2023
1 parent 8a02e02 commit 1ed0a49
Show file tree
Hide file tree
Showing 7 changed files with 182 additions and 123 deletions.
23 changes: 23 additions & 0 deletions eng/common-tests/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# About common-tests

Every PowerShell source that is complex enough to have unit tests should have these tests be written in the [Pester](https://pester.dev/)
framework and placed in `eng/common-tests/**`, i.e. this directory or one of its descendants.
By design, unlike `eng/common`, `eng/common-tests` is not subject to
[mirroring to repositories](https://github.com/Azure/azure-sdk-tools/blob/main/doc/common/common_engsys.md).

## When tests in this directory are executed

The PowerShell Pester unit tests will be executed by the [`tools - eng-common-tests`](https://dev.azure.com/azure-sdk/internal/_build/results?buildId=220791) pipeline
upon a PR that makes changes to `eng/common-tests/**`. The pipeline source is `eng/common-tests/ci.yml`.

## How to ensure your PowerShell pester unit test is executed

By Pester's default convention, the tests are to be placed in files whose
[names ends with `.tests.ps1`](https://pester.dev/docs/usage/file-placement-and-naming),
e.g. [`job-matrix-functions.tests.ps1`](https://github.com/Azure/azure-sdk-tools/blob/8a02e02adfc0d213509fce2764132afa74bd4ba4/eng/common-tests/matrix-generator/tests/job-matrix-functions.tests.ps1).

Furthermore, each test needs to be tagged with `UnitTest`, [e.g. as seen here](https://github.com/Azure/azure-sdk-tools/blob/8a02e02adfc0d213509fce2764132afa74bd4ba4/eng/common-tests/matrix-generator/tests/job-matrix-functions.tests.ps1#L51): `Describe "Matrix-Lookup" -Tag "UnitTest", "lookup"`.

Finally, as already mentioned, all such test files must be located in the path of `eng/common-tests/**`.

For more, see [Invoke-Pester doc](https://pester.dev/docs/commands/Invoke-Pester).
3 changes: 1 addition & 2 deletions eng/common-tests/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ trigger:
- hotfix/*
paths:
include:
- eng/common/scripts/*
- eng/common-tests/*

pr:
Expand All @@ -27,4 +26,4 @@ extends:
template: /eng/common/pipelines/templates/stages/archetype-sdk-tool-pwsh.yml
parameters:
TargetDirectory: eng/common-tests/
TargetTags: 'UnitTest'
TargetTags: UnitTest
53 changes: 53 additions & 0 deletions eng/common-tests/retrieve-codeowners.tests.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
Import-Module Pester

BeforeAll {
. $PSScriptRoot/../common/scripts/get-codeowners-functions.ps1

$ToolVersion = "1.0.0-dev.20230214.3"
$ToolPath = (Join-Path ([System.IO.Path]::GetTempPath()) "codeowners-tool")
$DevOpsFeed = "https://pkgs.dev.azure.com/azure-sdk/public/_packaging/azure-sdk-for-net/nuget/v3/index.json"
$VsoVariable = ""

function TestGetCodeowners([string]$targetPath, [string]$codeownersFileLocation, [bool]$includeNonUserAliases = $false, [string[]]$expectedOwners) {
Write-Host "Test: Owners for path '$targetPath' in CODEOWNERS file at path '$codeownersFileLocation' should be '$expectedOwners'"

$actualOwners = Get-Codeowners `
-ToolVersion $ToolVersion `
-ToolPath $ToolPath `
-DevOpsFeed $DevOpsFeed `
-VsoVariable $VsoVariable `
-targetPath $targetPath `
-codeownersFileLocation $codeownersFileLocation `
-includeNonUserAliases $IncludeNonUserAliases

if ($actualOwners.Count -ne $expectedOwners.Count) {
Write-Error "The length of actual result is not as expected. Expected length: $($expectedOwners.Count), Actual length: $($actualOwners.Count)."
exit 1
}
for ($i = 0; $i -lt $expectedOwners.Count; $i++) {
if ($expectedOwners[$i] -ne $actualOwners[$i]) {
Write-Error "Expect result $expectedOwners[$i] is different than actual result $actualOwners[$i]."
exit 1
}
}
}
}

Describe "Retrieve Codeowners" -Tag "UnitTest" {
It "Should retrieve Codeowners" -TestCases @(
@{
codeownersPath = "$PSScriptRoot/../../.github/CODEOWNERS";
targetPath = "eng/common/scripts/get-codeowners.ps1";
expectedOwners = @("konrad-jamrozik", "weshaggard", "benbp")
},
@{
codeownersPath = "$PSScriptRoot/../../tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners.Tests/TestData/glob_path_CODEOWNERS";
targetPath = "tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners.Tests/TestData/InputDir/a.txt";
expectedOwners = @("2star")
}
) {
$azSdkToolsCodeowners = (Resolve-Path $codeownersPath)
TestGetCodeowners -targetPath $targetPath -codeownersFileLocation $azSdkToolsCodeowners -includeNonUserAliases $true -expectedOwners $expectedOwners
$LASTEXITCODE | Should -Be 0
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,20 @@ variables:
- template: /eng/pipelines/templates/variables/globals.yml

stages:
- stage: 'eng_script_tests'
- stage: Run_PowerShell_Tests
jobs:
- job: 'Test'
- job: Test
strategy:
matrix:
Windows:
Pool: 'azsdk-pool-mms-win-2022-general'
Image: 'windows-2022'
Pool: azsdk-pool-mms-win-2022-general
Image: windows-2022
Linux:
Pool: azsdk-pool-mms-ubuntu-2204-general
Image: MMSUbuntu22.04
Mac:
Pool: 'Azure Pipelines'
Image: 'macos-11'
Pool: Azure Pipelines
Image: macos-11

pool:
name: $(Pool)
Expand Down
20 changes: 11 additions & 9 deletions eng/common/pipelines/templates/steps/run-pester-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ parameters:

steps:
- pwsh: |
Install-Module -Name Pester -Force
Install-Module -Name Pester -Force
displayName: Install Pester
# default test steps
Expand All @@ -30,26 +30,28 @@ steps:
$config.Filter.Tag = $tags
}
Write-Host "Calling 'Invoke-Pester' in workingDirectory '$(Build.SourcesDirectory)/${{ parameters.TargetDirectory }}'. Pester tags filter: '$tags'"
# https://pester.dev/docs/commands/Invoke-Pester
Invoke-Pester -Configuration $config
displayName: Run Tests
displayName: Run Pester Tests
env: ${{ parameters.EnvVars }}
workingDirectory: $(Build.SourcesDirectory)/${{ parameters.TargetDirectory }}
- ${{ if not(eq(length(parameters.CustomTestSteps), 0)) }}:
- ${{ parameters.CustomTestSteps }}

- task: PublishTestResults@2
displayName: 'Publish Test Results'
displayName: Publish Test Results
condition: succeededOrFailed()
inputs:
testResultsFormat: 'NUnit'
testResultsFormat: NUnit
testResultsFiles: $(Build.SourcesDirectory)/${{ parameters.TargetDirectory }}/testResults.xml
testRunTitle: '$(System.StageName)_$(Agent.JobName)_Tests'
testRunTitle: $(System.StageName)_$(Agent.JobName)_Tests

- task: PublishCodeCoverageResults@1
displayName: 'Publish Code Coverage to Azure DevOps'
displayName: Publish Code Coverage to Azure DevOps
condition: succeededOrFailed()
inputs:
codeCoverageTool: 'JaCoCo'
summaryFileLocation: '$(Build.SourcesDirectory)/${{ parameters.TargetDirectory }}/coverage.xml'
pathToSources: '$(Build.SourcesDirectory)/${{ parameters.TargetDirectory }}'
codeCoverageTool: JaCoCo
summaryFileLocation: $(Build.SourcesDirectory)/${{ parameters.TargetDirectory }}/coverage.xml
pathToSources: $(Build.SourcesDirectory)/${{ parameters.TargetDirectory }}
75 changes: 75 additions & 0 deletions eng/common/scripts/get-codeowners-functions.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
function Get-CodeownersTool([string] $ToolPath, [string] $DevOpsFeed, [string] $ToolVersion)
{
$codeownersToolCommand = Join-Path $ToolPath "retrieve-codeowners"
# Check if the retrieve-codeowners tool exists or not.
if (Get-Command $codeownersToolCommand -errorAction SilentlyContinue) {
return $codeownersToolCommand
}
if (!(Test-Path $ToolPath)) {
New-Item -ItemType Directory -Path $ToolPath | Out-Null
}
Write-Host "Installing the retrieve-codeowners tool under tool path: $ToolPath ..."

# Run command under tool path to avoid dotnet tool install command checking .csproj files.
# This is a bug for dotnet tool command. Issue: https://github.com/dotnet/sdk/issues/9623
Push-Location $ToolPath
dotnet tool install --tool-path $ToolPath --add-source $DevOpsFeed --version $ToolVersion "Azure.Sdk.Tools.RetrieveCodeOwners" | Out-Null
Pop-Location
# Test to see if the tool properly installed.
if (!(Get-Command $codeownersToolCommand -errorAction SilentlyContinue)) {
Write-Error "The retrieve-codeowners tool is not properly installed. Please check your tool path: $ToolPath"
return
}
return $codeownersToolCommand
}

function Get-Codeowners(
[string] $ToolPath,
[string] $DevOpsFeed,
[string] $ToolVersion,
[string] $VsoVariable,
[string] $targetPath,
[string] $targetDirectory,
[string] $codeownersFileLocation,
[bool] $includeNonUserAliases = $false)
{
# Backward compaitiblity: if $targetPath is not provided, fall-back to the legacy $targetDirectory
if ([string]::IsNullOrWhiteSpace($targetPath)) {
$targetPath = $targetDirectory
}
if ([string]::IsNullOrWhiteSpace($targetPath)) {
Write-Error "TargetPath (or TargetDirectory) parameter must be neither null nor whitespace."
return ,@()
}

$codeownersToolCommand = Get-CodeownersTool -ToolPath $ToolPath -DevOpsFeed $DevOpsFeed -ToolVersion $ToolVersion
Write-Host "Executing: & $codeownersToolCommand --target-path $targetPath --codeowners-file-path-or-url $codeownersFileLocation --exclude-non-user-aliases:$(!$includeNonUserAliases)"
$commandOutput = & $codeownersToolCommand `
--target-path $targetPath `
--codeowners-file-path-or-url $codeownersFileLocation `
--exclude-non-user-aliases:$(!$includeNonUserAliases) `
2>&1

if ($LASTEXITCODE -ne 0) {
Write-Host "Command $codeownersToolCommand execution failed (exit code = $LASTEXITCODE). Output string: $commandOutput"
return ,@()
} else
{
Write-Host "Command $codeownersToolCommand executed successfully (exit code = 0). Output string length: $($commandOutput.length)"
}

# Assert: $commandOutput is a valid JSON representing:
# - a single CodeownersEntry, if the $targetPath was a single path
# - or a dictionary of CodeownerEntries, keyes by each path resolved from a $targetPath glob path.
#
# For implementation details, see Azure.Sdk.Tools.RetrieveCodeOwners.Program.Main

$codeownersJson = $commandOutput | ConvertFrom-Json

if ($VsoVariable) {
$codeowners = $codeownersJson.Owners -join ","
Write-Host "##vso[task.setvariable variable=$VsoVariable;]$codeowners"
}

return ,@($codeownersJson.Owners)
}
119 changes: 13 additions & 106 deletions eng/common/scripts/get-codeowners.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -63,115 +63,22 @@ param (
# Remove the obsolete, prefix-based CODEOWNERS matcher & related tests
# https://github.com/Azure/azure-sdk-tools/pull/5431
[string]$ToolVersion = "1.0.0-dev.20230214.3",
[string]$ToolPath = (Join-Path ([System.IO.Path]::GetTempPath()) "codeowners-tool-path"),
[string]$ToolPath = (Join-Path ([System.IO.Path]::GetTempPath()) "codeowners-tool"),
[string]$DevOpsFeed = "https://pkgs.dev.azure.com/azure-sdk/public/_packaging/azure-sdk-for-net/nuget/v3/index.json",
[string]$VsoVariable = "",
[switch]$IncludeNonUserAliases,
[switch]$Test
)

function Get-CodeownersTool()
{
$codeownersToolCommand = Join-Path $ToolPath "retrieve-codeowners"
# Check if the retrieve-codeowners tool exists or not.
if (Get-Command $codeownersToolCommand -errorAction SilentlyContinue) {
return $codeownersToolCommand
}
if (!(Test-Path $ToolPath)) {
New-Item -ItemType Directory -Path $ToolPath | Out-Null
}
Write-Host "Installing the retrieve-codeowners tool under tool path: $ToolPath ..."

# Run command under tool path to avoid dotnet tool install command checking .csproj files.
# This is a bug for dotnet tool command. Issue: https://github.com/dotnet/sdk/issues/9623
Push-Location $ToolPath
dotnet tool install --tool-path $ToolPath --add-source $DevOpsFeed --version $ToolVersion "Azure.Sdk.Tools.RetrieveCodeOwners" | Out-Null
Pop-Location
# Test to see if the tool properly installed.
if (!(Get-Command $codeownersToolCommand -errorAction SilentlyContinue)) {
Write-Error "The retrieve-codeowners tool is not properly installed. Please check your tool path: $ToolPath"
return
}
return $codeownersToolCommand
}

function Get-Codeowners(
[string]$targetPath,
[string]$targetDirectory,
[string]$codeownersFileLocation,
[bool]$includeNonUserAliases = $false)
{
# Backward compaitiblity: if $targetPath is not provided, fall-back to the legacy $targetDirectory
if ([string]::IsNullOrWhiteSpace($targetPath)) {
$targetPath = $targetDirectory
}
if ([string]::IsNullOrWhiteSpace($targetPath)) {
Write-Error "TargetPath (or TargetDirectory) parameter must be neither null nor whitespace."
return ,@()
}

$codeownersToolCommand = Get-CodeownersTool
Write-Host "Executing: & $codeownersToolCommand --target-path $targetPath --codeowners-file-path-or-url $codeownersFileLocation --exclude-non-user-aliases:$(!$includeNonUserAliases)"
$commandOutput = & $codeownersToolCommand `
--target-path $targetPath `
--codeowners-file-path-or-url $codeownersFileLocation `
--exclude-non-user-aliases:$(!$includeNonUserAliases) `
2>&1

if ($LASTEXITCODE -ne 0) {
Write-Host "Command $codeownersToolCommand execution failed (exit code = $LASTEXITCODE). Output string: $commandOutput"
return ,@()
} else
{
Write-Host "Command $codeownersToolCommand executed successfully (exit code = 0). Output string length: $($commandOutput.length)"
}

# Assert: $commandOutput is a valid JSON representing:
# - a single CodeownersEntry, if the $targetPath was a single path
# - or a dictionary of CodeownerEntries, keyes by each path resolved from a $targetPath glob path.
#
# For implementation details, see Azure.Sdk.Tools.RetrieveCodeOwners.Program.Main

$codeownersJson = $commandOutput | ConvertFrom-Json

if ($VsoVariable) {
$codeowners = $codeownersJson.Owners -join ","
Write-Host "##vso[task.setvariable variable=$VsoVariable;]$codeowners"
}

return ,@($codeownersJson.Owners)
}

function TestGetCodeowners([string]$targetPath, [string]$codeownersFileLocation, [bool]$includeNonUserAliases = $false, [string[]]$expectReturn) {
Write-Host "Test: find owners matching '$targetPath' ..."

$actualReturn = Get-Codeowners -targetPath $targetPath -codeownersFileLocation $codeownersFileLocation -includeNonUserAliases $IncludeNonUserAliases

if ($actualReturn.Count -ne $expectReturn.Count) {
Write-Error "The length of actual result is not as expected. Expected length: $($expectReturn.Count), Actual length: $($actualReturn.Count)."
exit 1
}
for ($i = 0; $i -lt $expectReturn.Count; $i++) {
if ($expectReturn[$i] -ne $actualReturn[$i]) {
Write-Error "Expect result $expectReturn[$i] is different than actual result $actualReturn[$i]."
exit 1
}
}
}

if ($Test) {
# Most of tests here have been removed; now instead we should run tests from RetrieveCodeOwnersProgramTests, and in a way as explained in:
# https://github.com/Azure/azure-sdk-tools/issues/5434
# https://github.com/Azure/azure-sdk-tools/pull/5103#discussion_r1068680818
Write-Host "Running reduced test suite at `$PSScriptRoot of $PSSCriptRoot. Please see https://github.com/Azure/azure-sdk-tools/issues/5434 for more."

$azSdkToolsCodeowners = (Resolve-Path "$PSScriptRoot/../../../.github/CODEOWNERS")
TestGetCodeowners -targetPath "eng/common/scripts/get-codeowners.ps1" -codeownersFileLocation $azSdkToolsCodeowners -includeNonUserAliases $true -expectReturn @("konrad-jamrozik", "weshaggard", "benbp")

$testCodeowners = (Resolve-Path "$PSScriptRoot/../../../tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners.Tests/TestData/glob_path_CODEOWNERS")
TestGetCodeowners -targetPath "tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners.Tests/TestData/InputDir/a.txt" -codeownersFileLocation $testCodeowners -includeNonUserAliases $true -expectReturn @("2star")
exit 0
}
else {
return Get-Codeowners -targetPath $TargetPath -targetDirectory $TargetDirectory -codeownersFileLocation $CodeOwnerFileLocation -includeNonUserAliases $IncludeNonUserAliases
}
. $PSScriptRoot/get-codeowners-functions.ps1

return Get-Codeowners `
-ToolVersion $ToolVersion `
-ToolPath $ToolPath `
-DevOpsFeed $DevOpsFeed `
-VsoVariable $VsoVariable `
-targetPath $TargetPath `
-targetDirectory $TargetDirectory `
-codeownersFileLocation $CodeOwnerFileLocation `
-includeNonUserAliases $IncludeNonUserAliases

0 comments on commit 1ed0a49

Please sign in to comment.