Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add path based rules and pester tests #3182

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 23 additions & 0 deletions eng/common/pipelines/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,26 @@ extends:
parameters:
DependsOn: InitStage
Condition: eq('true', stageDependencies.InitStage.outputs['InitJob.InitStep.RunCore'])

# Run eng test stage if RunEng == true
- stage: EngTest
displayName: Eng - Test
dependsOn: InitStage
condition: eq('true', stageDependencies.InitStage.outputs['InitJob.InitStep.RunEng'])
jobs:
- job: EngTest
displayName: Eng - Test
pool:
name: $(LINUXPOOL)
image: $(LINUXVMIMAGE)
os: linux
steps:
- pwsh: |
Write-Host "Installing Pester"
hallipr marked this conversation as resolved.
Show resolved Hide resolved
Install-Module -Name Pester -RequiredVersion '5.5.0' -Force -SkipPublisherCheck
Import-Module Pester

Write-Host "Running pester tests"
Invoke-Pester
displayName: "Run pester tests"
workingDirectory: $(Build.SourcesDirectory)/eng
24 changes: 18 additions & 6 deletions eng/common/scripts/Analyze-Changes.Tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,14 @@ Describe 'Analyze-Changes' {
$_ | Should -BeIn $expected
}
}

It 'Should return RunEng if testable files in eng are changed' {
$variables = Get-ActiveVariables @(
"eng/common/scripts/some-file.ps1"
)

$variables | Should -Contain 'RunEng'
}

It 'Should return a combination of core and isolated packages' {
$actual = Get-ActiveVariables @(
Expand Down Expand Up @@ -64,13 +72,17 @@ Describe 'Analyze-Changes' {
".prettierrc.json",
"cspell.yaml",
"esling.config.json"
"packages/http-client-csharp/emitter/src/constants.ts"
)

$expected = @('RunCore', 'RunCSharp')
$actual | Should -Not -Contain 'RunCore'
}

$actual | ForEach-Object {
$_ | Should -BeIn $expected
}
}
It 'Should return runCore for arbitrary files in the root directory' {
$actual = Get-ActiveVariables @(
"some.file",
"file/in/deep/directory"
)

$actual | Should -Contain 'RunCore'
}
}
207 changes: 61 additions & 146 deletions eng/common/scripts/Analyze-Changes.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -4,161 +4,76 @@ param(
[string] $TargetBranch
)

# Represents an isolated package which has its own stages in typespec - ci pipeline
class IsolatedPackage {
[string[]] $Paths
[string] $RunVariable
[bool] $RunValue

IsolatedPackage([string[]]$paths, [string]$runVariable, [bool]$runValue) {
$this.Paths = $paths
$this.RunVariable = $runVariable
$this.RunValue = $runValue
}
$runAll = @('^eng/common/', '^vitest.config.ts')

# Map of rule name to paths to include/exclude as regex patterns
# exclude paths start with '!'
$pathBasedRules = @{
'RunEng' = @('^eng/common/scripts/')
'RunCSharp' = $runAll, '^packages/http-client-csharp', '^\.editorconfig$'
Copy link
Member

Choose a reason for hiding this comment

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

should editorconfig trigger all emitters?

'RunJava' = $runAll, '^packages/http-client-java/'
'RunTypeScript' = $runAll, '^packages/http-client-typescript/'
'RunPython' = $runAll, '^packages/http-client-python/'
'RunCore' = @(
Copy link
Member

Choose a reason for hiding this comment

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

there is more than should be excluded here around eng/emitter stuff

#any file `
'.',
#except for these `
'!^packages/http-client-(csharp|java|typescript|python)/',
'!\.prettierignore',
'!\.prettierrc\.json',
'!cspell\.yaml',
'!esling\.config\.json'
)
}

# Emitter packages in the repo
$isolatedPackages = @{
"http-client-csharp" = [IsolatedPackage]::new(@("packages/http-client-csharp", ".editorconfig"), "RunCSharp", $false)
"http-client-java" = [IsolatedPackage]::new(@("packages/http-client-java"), "RunJava", $false)
"http-client-typescript" = [IsolatedPackage]::new(@("packages/http-client-typescript"), "RunTypeScript", $false)
"http-client-python" = [IsolatedPackage]::new(@("packages/http-client-python"), "RunPython", $false)
}

# A tree representation of a set of files
# Each node represents a directory and contains a list of child nodes.
class TreeNode {
[string] $Name
[System.Collections.Generic.List[TreeNode]] $Children

TreeNode([string]$name) {
$this.Name = $name
$this.Children = @()
}

# Add a file to the tree
[void] Add([string]$filePath) {
$parts = $filePath -split '/'

$currentNode = $this
foreach ($part in $parts) {
$childNode = $currentNode.Children | Where-Object { $_.Name -eq $part }
if (-not $childNode) {
$childNode = [TreeNode]::new($part)
$currentNode.Children.Add($childNode)
}
$currentNode = $childNode
function Get-ActiveVariables([string[]]$changes) {
# exit early if no changes detected
if ($changes.Length -eq 0) {
Write-Host '##[error] No changes detected'
exit 1
}

$variables = @()

foreach ($rule in $pathBasedRules.Keys) {
$paths = $pathBasedRules[$rule]
$includes = $paths | Where-Object { -not $_.StartsWith('!') }
$excludes = $paths | Where-Object { $_.StartsWith('!') } | ForEach-Object { $_.Substring(1) }

foreach($change in $changes) {
# if any changed file matches an include and no excludes, set the variable
$matchesAnyInclude = $false
foreach($path in $includes) {
if($change -match $path) {
$matchesAnyInclude = $true
break
}
}

# Check if a file exists in the tree
[bool] PathExists([string]$filePath) {
$parts = $filePath -split '/'

$currentNode = $this
foreach ($part in $parts) {
$childNode = $currentNode.Children | Where-Object { $_.Name -eq $part }
if (-not $childNode) {
return $false
}
$currentNode = $childNode
}
return $true
}

# Check if anything outside of emitter packages exists
[bool] AnythingOutsideIsolatedPackagesExists($isolatedPackages) {
if ($this.Children.Count -eq 0) {
return $false
}

if(!$matchesAnyInclude) {
continue
}

$matchesAnyExclude = $false
foreach($path in $excludes) {
foreach($change in $changes) {
if($change -match $path) {
$matchesAnyExclude = $true
break
}
}
}

# if anything in first level is not 'packages', return true
foreach ($child in $this.Children) {
# skip .prettierignore, .prettierrc.json, cspell.yaml, esling.config.json since these are all covered by github actions globally
if ($child.Name -in @('.prettierignore', '.prettierrc.json', 'cspell.yaml', 'esling.config.json')) {
continue
}

if ($child.Name -ne 'packages') {
return $true
}
}

$packagesNode = $this.Children | Where-Object { $_.Name -eq "packages" }
if (-not $packagesNode) {
return $false
}

# if anything in second level is not an emitter package, return true
foreach ($child in $packagesNode.Children) {
if ($child.Name -notin $isolatedPackages.Keys) {
return $true
}
}

return $false
if(!$matchesAnyExclude) {
$variables += $rule
break
}
}
}

[string] ToString() {
return $this.Name
}
return $variables
}

function Get-ActiveVariables($changes) {
# initialize tree
$root = [TreeNode]::new('Root')
$variables = @()

$changes | ForEach-Object {
$root.Add($_)
}

# exit early if no changes detected
if ($root.Children.Count -eq 0) {
Write-Host "##[error] No changes detected"
exit 1
}

# set global flag to run all if common files are changed
$runAll = $root.PathExists('eng/common')

# set global isolated package flag to run if any eng/emiters files changed
$runIsolated = $root.PathExists('eng/emitters')

# no need to check individual packages if runAll is true
if (-not $runAll) {
if (-not $runIsolated) {
# set each isolated package flag
foreach ($package in $isolatedPackages.Values) {
foreach ($path in $package.Paths) {
$package.RunValue = $package.RunValue -or $root.PathExists($path)
if ($package.RunValue) {
break
}
}
}
}

# set runCore to true if none of the
$runCore = $root.AnythingOutsideIsolatedPackagesExists($isolatedPackages)
}

# set log commands
if ($runAll -or $runCore) {
$variables += "RunCore"
}

# foreach isolated package, set log commands if the RunValue is true
foreach ($package in $isolatedPackages.Values) {
if ($runAll -or $runIsolated -or $package.RunValue) {
$variables += $package.RunVariable
}
}

return $variables
}


# add all changed files to the tree
Write-Host "Checking for changes in current branch compared to $TargetBranch"
$changes = git diff --name-only origin/$TargetBranch...
Expand Down