Skip to content

Commit

Permalink
Move get-codeowners.ps1 test mode to common-tests + related impro…
Browse files Browse the repository at this point in the history
…vements & cleanup (#5608)

This PR moves the `Test` flag away from `get-codeowners.ps1` to the established `eng/common-tests` infrastructure, using PowerShell Pester testing framework. As such, it solves:

- #5434

This PR also:
- adds a README for the `eng/common-tests`;
- extracts the `get-codeowners.lib.ps1`, so that the functions can be reused between normal `get-codewners.ps1` usage and through Pester unit test invocations coming from `get-codeowners.tests.ps1`;
- changes some paths for related scripts and test scripts;
- moves some default params to `get-codeowners.lib.ps1` and hides some unused params; this was necessary to avoid default param value duplication between calls from `get-codeowners.ps1` and from `get-codeowners.tests.ps1`;
- applies some refactorings.

Associated changes made manually once this PR got merged:

- deleted internal pipeline [tools - eng-common-tests](https://dev.azure.com/azure-sdk/internal/_build/results?buildId=2605155&view=results); instead, created the public equivalent: [tools - eng-common-tests - ci](https://dev.azure.com/azure-sdk/public/_build?definitionId=5985).
  - For context, see [this comment](#5608 (comment)).

Follow-up work:
- #5878
  • Loading branch information
Konrad Jamrozik authored Mar 31, 2023
1 parent 7a2d461 commit b5cb2ed
Show file tree
Hide file tree
Showing 9 changed files with 248 additions and 200 deletions.
26 changes: 26 additions & 0 deletions eng/common-tests/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# 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 public
[`tools - eng-common-tests`](https://dev.azure.com/azure-sdk/public/_build?definitionId=5985&_a=summary) pipeline upon a PR that makes changes
to `eng/common/scripts/**` or `eng/common-tests/**`. The pipeline source is `eng/common-tests/ci.yml`.

In addition, specific tools might selectively run a subset of tests during their pipeline run, as e.g. done by `tools/code-owners-parser/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).
12 changes: 6 additions & 6 deletions eng/common-tests/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ trigger:
- hotfix/*
paths:
include:
- eng/common/scripts/*
- eng/common-tests/*
- eng/common/scripts
- eng/common-tests

pr:
branches:
Expand All @@ -20,11 +20,11 @@ pr:
- hotfix/*
paths:
include:
- eng/common/scripts/*
- eng/common-tests/*
- eng/common/scripts
- eng/common-tests

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

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

function TestGetCodeowners(
[string] $TargetPath,
[string] $CodeownersFileLocation,
[string[]] $ExpectedOwners
)
{
Write-Host "Test: Owners for path '$TargetPath' in CODEOWNERS file at path '$CodeownersFileLocation' should be '$ExpectedOwners'"

$actualOwners = Get-Codeowners `
-TargetPath $TargetPath `
-CodeownersFileLocation $CodeownersFileLocation `

$actualOwners.Count | Should -Be $ExpectedOwners.Count
for ($i = 0; $i -lt $ExpectedOwners.Count; $i++) {
$ExpectedOwners[$i] | Should -Be $actualOwners[$i]
}
}
}

Describe "Get Codeowners" -Tag "UnitTest" {
It "Should get Codeowners" -TestCases @(
@{
# The $PSScriptRoot is assumed to be azure-sdk-tools/eng/common-tests/get-codeowners/get-codeowners.tests.ps1
codeownersPath = "$PSScriptRoot/../../../.github/CODEOWNERS";
targetPath = "eng/common/scripts/get-codeowners/get-codeowners.ps1";
expectedOwners = @("konrad-jamrozik", "weshaggard", "benbp")
},
@{
# The $PSScriptRoot is assumed to be azure-sdk-tools/eng/common-tests/get-codeowners/get-codeowners.tests.ps1
CodeownersPath = "$PSScriptRoot/../../../tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners.Tests/TestData/test_CODEOWNERS";
targetPath = "tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners.Tests/TestData/InputDir/a.txt";
expectedOwners = @("2star")
}
) {
$resolvedCodeownersPath = (Resolve-Path $codeownersPath)
TestGetCodeowners `
-TargetPath $targetPath `
-CodeownersFileLocation $resolvedCodeownersPath `
-ExpectedOwners $expectedOwners
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,21 @@ variables:
- template: /eng/pipelines/templates/variables/globals.yml

stages:
- stage: 'eng_script_tests'
- stage:
displayName: 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 }}
133 changes: 133 additions & 0 deletions eng/common/scripts/get-codeowners.lib.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
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
}

<#
.SYNOPSIS
A function that given as input $TargetPath param, returns the owners
of that path, as determined by CODEOWNERS file passed in $CodeownersFileLocation
param.
.PARAMETER TargetPath
Required*. Path to file or directory whose owners are to be determined from a
CODEOWNERS file. e.g. sdk/core/azure-amqp/ or sdk/core/foo.txt.
*for backward compatibility, you might provide $TargetDirectory instead.
.PARAMETER TargetDirectory
Obsolete. Replaced by $TargetPath. Kept for backward-compatibility.
If both $TargetPath and $TargetDirectory are provided, $TargetDirectory is
ignored.
.PARAMETER CodeownersFileLocation
Optional. An absolute path to the CODEOWNERS file against which the $TargetPath param
will be checked to determine its owners.
.PARAMETER ToolVersion
Optional. The NuGet package version of the package containing the "retrieve-codeowners"
tool, around which this script is a wrapper.
.PARAMETER ToolPath
Optional. The place to check the "retrieve-codeowners" tool existence.
.PARAMETER DevOpsFeed
Optional. The NuGet package feed from which the "retrieve-codeowners" tool is to be installed.
NuGet feed:
https://dev.azure.com/azure-sdk/public/_artifacts/feed/azure-sdk-for-net/NuGet/Azure.Sdk.Tools.RetrieveCodeOwners
Pipeline publishing the NuGet package to the feed, "tools - code-owners-parser":
https://dev.azure.com/azure-sdk/internal/_build?definitionId=3188
.PARAMETER VsoVariable
Optional. If provided, the determined owners, based on $TargetPath matched against CODEOWNERS file at $CodeownersFileLocation,
will be output to Azure DevOps pipeline log as variable named $VsoVariable.
Reference:
https://learn.microsoft.com/en-us/azure/devops/pipelines/process/variables?view=azure-devops&tabs=yaml%2Cbatch
https://learn.microsoft.com/en-us/azure/devops/pipelines/scripts/logging-commands?view=azure-devops&tabs=bash#logging-command-format
.PARAMETER IncludeNonUserAliases
Optional. Whether to include in the returned owners list aliases that are team aliases, e.g. Azure/azure-sdk-team
.PARAMETER Test
Optional. Whether to run the script against hard-coded tests.
#>
function Get-Codeowners(
[string] $TargetPath,
[string] $TargetDirectory,
[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] $ToolVersion = "1.0.0-dev.20230306.3",
[string] $VsoVariable = "",
[string] $CodeownersFileLocation = "",
[switch] $IncludeNonUserAliases
)
{
if ([string]::IsNullOrWhiteSpace($CodeownersFileLocation)) {
# The $PSScriptRoot is assumed to be azure-sdk-tools/eng/common/scripts/get-codeowners.ps1
$CodeownersFileLocation = (Resolve-Path $PSScriptRoot/../../../.github/CODEOWNERS)
}

# Backward compatibility: 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). Command 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)
}
Loading

0 comments on commit b5cb2ed

Please sign in to comment.