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

Migrate typespec validation #31033

Merged
merged 52 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
43cb00b
test breaking change mlc in js (#3451)
wanlwanl Aug 6, 2024
b3b9f1a
Update specificationRepositoryConfiguration.json (#3457)
Alancere Aug 7, 2024
808ea5f
Update specificationRepositoryConfiguration.json (#3459)
msyyc Aug 7, 2024
96b6c82
Update specificationRepositoryConfiguration.json (#3462)
raych1 Aug 9, 2024
1ebe623
Update specificationRepositoryConfiguration.json (#3463)
msyyc Aug 9, 2024
dba759b
Migrate typespec-validation.yml to GH Actions
danieljurek Oct 14, 2024
4136688
path, fetch depth
danieljurek Oct 14, 2024
bab1ae6
Introduce error
danieljurek Oct 14, 2024
d923e56
Test a feature that AI says will collapse logs (doubtful it'll work)
danieljurek Oct 14, 2024
0e71fc9
Revert "Test a feature that AI says will collapse logs (doubtful it'l…
danieljurek Oct 14, 2024
6839fbb
Log $LASTEXITCODE
danieljurek Oct 14, 2024
ec7af3d
Invalid syntax
danieljurek Oct 14, 2024
be5efe7
Use error logging, remove continue-on-error because that is different…
danieljurek Oct 14, 2024
985bf9b
Errors also get annotations in GitHub Actions
danieljurek Oct 14, 2024
407cb5e
String replacement
danieljurek Oct 14, 2024
e1ec118
Revert Logging-Functions.ps1
danieljurek Oct 14, 2024
b32b546
Test composite action
danieljurek Oct 14, 2024
1057f5d
File location
danieljurek Oct 14, 2024
8bdee66
Invocation
danieljurek Oct 14, 2024
9eba441
Add typespec-validation-all.yml
danieljurek Oct 14, 2024
92e359b
Migrate typespec-validation-all.yml
danieljurek Oct 14, 2024
2723721
Revert main.tsp invalid spec
danieljurek Oct 15, 2024
d232bb5
fetch-depth: 2
danieljurek Oct 15, 2024
e7b871b
Long paths
danieljurek Oct 15, 2024
772ef9f
checkout@v4
danieljurek Oct 15, 2024
a154939
Remove pipelines
danieljurek Oct 15, 2024
c20eb68
*.yml -> *.yaml
danieljurek Oct 15, 2024
7856ddf
Do not change specificationRepositoryConfiguration.json
danieljurek Oct 15, 2024
4f904de
Job names (for checks)
danieljurek Oct 15, 2024
0b7b6a6
Add Skip/Take logic and matrix
danieljurek Oct 24, 2024
cfddb9e
Syntax
danieljurek Oct 24, 2024
b3d1458
Rename
danieljurek Oct 25, 2024
906a8a2
Remove "shell"
danieljurek Oct 25, 2024
d49e44f
Try defaults.run.shell (unlikely to work)
danieljurek Oct 25, 2024
95a6cc3
bash
danieljurek Oct 25, 2024
62a57b5
Name for actions/setup-node@v4 does not appear in the logs. Remove.
danieljurek Oct 25, 2024
7109da1
Use sharding semantics
danieljurek Oct 25, 2024
e4410d9
Shard
danieljurek Oct 25, 2024
36de665
Review feedback
danieljurek Oct 25, 2024
5f1b7c6
Review feedback
danieljurek Oct 25, 2024
a088013
+1
danieljurek Oct 25, 2024
889143e
Revert "Shard"... Display is confusing because it starts at 0 and doe…
danieljurek Oct 25, 2024
6c836b3
Shard only if TotalShards > 0
danieljurek Oct 25, 2024
3b50e1a
Remove description (schema validaiton does not pass, let's see what G…
danieljurek Oct 25, 2024
ecaf771
The `description` property is not absolutely required by GH Actions a…
danieljurek Oct 25, 2024
34a7d4b
Add array functions and tests
danieljurek Oct 25, 2024
a316274
Move Copy-ApiVersion.Tests.ps1 out of folder (adjust paths so they ar…
danieljurek Oct 25, 2024
be61a9c
Add trailing newline
mikeharder Oct 25, 2024
2886209
Add trailing newline
mikeharder Oct 25, 2024
7fa3248
Update .github/workflows/typespec-validation.yaml
danieljurek Oct 25, 2024
4f4cfd9
Review feedback
danieljurek Oct 25, 2024
1fbda3c
Update eng/scripts/TypeSpec-Validation.ps1
danieljurek Oct 25, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions .github/actions/setup-node-npm-ci/action.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
name: Setup Node 20 and run `npm ci`
description: Uses specified Node version and runs npm commands to set up the environment for REST API CI
danieljurek marked this conversation as resolved.
Show resolved Hide resolved

inputs:
node-version:
description: 'Node version to use'
default: 20.x

runs:
using: "composite"

steps:
- uses: actions/setup-node@v4
with:
node-version: ${{ inputs.node-version }}

- run: npm ci
shell: bash

- run: npm ls -a
shell: bash
continue-on-error: true
69 changes: 69 additions & 0 deletions .github/workflows/typespec-validation-all.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
name: TypeSpec Validation All

on:
push:
branches:
- main
- RPSaaSMaster
- typespec-next

pull_request:
mikeharder marked this conversation as resolved.
Show resolved Hide resolved
branches:
- main
- RPSaaSMaster
- typespec-next
paths:
- .gitattributes
- .prettierrc.json
- package-lock.json
- package.json
- tsconfig.json
- eng/**
- specification/suppressions.yaml
- specification/common-types/**

# Workflow and workflow dependencies
- .github/workflows/typespec-validation-all.yaml
- .github/actions/setup-node-npm-ci/**

schedule:
# Run 4x/day
- cron: '0 0,6,12,18 * * * '

jobs:
typespec-validation-all:
name: TypeSpec Validation All
strategy:
matrix:
os: [ubuntu-latest, windows-latest]
# shards must start at 0 and increment by 1
shard: [0, 1, 2]
# total-shards must be an accurate count of the number of shards
total-shards: [3]

runs-on: ${{ matrix.os }}

steps:
- name: Enable git long paths
if: runner.os == 'Windows'
run: git config --global core.longpaths true

- uses: actions/checkout@v4
with:
fetch-depth: 2

- name: Setup Node 20 and run `npm ci`
uses: ./.github/actions/setup-node-npm-ci
Copy link
Member

Choose a reason for hiding this comment

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

@mikeharder @danieljurek I thought github force these to be under the workflows directory. Is this a way we can make other reusable pieces for actions?

Copy link
Member

@mikeharder mikeharder Oct 28, 2024

Choose a reason for hiding this comment

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

The workflow itself must be under .github/workflows, but I believe custom actions can live anywhere in your repo, in another repo, etc.

We're still experimenting, but we're considering trying to provide most reusable code through custom actions, rather than using github-script everywhere and sharing JS code.

Custom actions can still use github-script internally, but the goal would be to make most of our leaf-node workflows as declarative as possible (no github-script).


- name: Validate All Specs
run: |
./eng/scripts/TypeSpec-Validation.ps1 `
-Shard ${{ matrix.shard }} `
-TotalShards ${{ matrix.total-shards }} `
-CheckAll `
-GitClean `
-Verbose

# Effectively the same as ignoreLASTEXITCODE: true in Azure DevOps
exit 0
shell: pwsh
24 changes: 24 additions & 0 deletions .github/workflows/typespec-validation.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
name: Typespec Validation

on: pull_request

jobs:
typespec-validation:
name: Typespec Validation
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v4
with:
fetch-depth: 2

- name: Setup Node 20 and run `npm ci`
uses: ./.github/actions/setup-node-npm-ci

- name: Validate Impacted Specs
run: |
./eng/scripts/TypeSpec-Validation.ps1 -GitClean -Verbose
mikeharder marked this conversation as resolved.
Show resolved Hide resolved

# Effectively the same as ignoreLASTEXITCODE: true in Azure DevOps
exit 0
shell: pwsh
47 changes: 0 additions & 47 deletions eng/pipelines/typespec-validation-all.yml

This file was deleted.

16 changes: 0 additions & 16 deletions eng/pipelines/typespec-validation.yml

This file was deleted.

18 changes: 18 additions & 0 deletions eng/scripts/Array-Functions.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
function ShardArray($array, $shard, $totalShards) {
if ($totalShards -lt 2) {
return $array
}

if ($shard -ge $totalShards) {
throw "Shard index ($shard) must be less than total shards ($totalShards)"
}

if ($totalShards -gt $array.Length) {
throw "Cannot shard array into more pieces than there are elements"
Copy link
Member

Choose a reason for hiding this comment

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

Would we hit this case if there were only one spec discovered but we have it sharded in 3? I assume this wouldn't be the case for tsv-all but could be in other cases if we use this logic.

Copy link
Member

Choose a reason for hiding this comment

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

}

$shardSize = [math]::Ceiling($array.Length / $totalShards)
$start = $shard * $shardSize
$end = [math]::Min($start + $shardSize, $array.Length)
return $array[$start..($end - 1)]
}
77 changes: 77 additions & 0 deletions eng/scripts/Tests/Array-Functions.Tests.ps1
danieljurek marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
BeforeAll {
. "$PSScriptRoot\..\Array-Functions.ps1"
}

# 0..78 makes an array of 79 (prime number) items

Describe "ShardArray" {
Context "Input Validation" {
It "throws when sharding into more pieces than there are elements" {
$array = 0..78
$shard = 0
$totalShards = 150

{ ShardArray $array $shard $totalShards } | Should -Throw
}

It "returns the full array when given <totalShards> shards" -ForEach @(
@{ totalShards = 1 },
@{ totalShards = 0 },
@{ totalShards = -1 },
@{ totalShards = -2 }
) {
$array = 0..78
$shard = 0

$result = ShardArray $array $shard $totalShards

$result | Should -Be $array
}

It "throws when the shard index is greater than the total shards" {
$array = 0..78
$shard = 10
$totalShards = 3

{ ShardArray $array $shard $totalShards } | Should -Throw
}

It "does not throw when totalShards equals the array length" {
$array = 0..78
$shard = 0
$totalShards = 79

{ ShardArray $array $shard $totalShards } | Should -Not -Throw
}

}

Context "Shards arrays" -ForEach @(
@{ array = 0..78; totalShards = 1 },
@{ array = 0..78; totalShards = 2 },
@{ array = 0..11; totalShards = 3 },
@{ array = 0..10; totalShards = 4 },
@{ array = 0..78; totalShards = 79 }
) {
It "returns all of the values in order when sharding (Total Shards: <totalShards>) " {
$shards = New-Object object[] $totalShards
for ($i = 0; $i -lt $totalShards; $i++) {
# Assigning directly avoids flattening the array.
$shards[$i] = ShardArray $array $i $totalShards
}

# Flatten the array for comparison
$actual = $shards | ForEach-Object { $_ }

$actual | Should -Be $array
}

It "returns arrays of expected valid size (Total Shards: <totalShards>)" {
$maxLength = [math]::Ceiling($array.Length / $totalShards)
for ($i = 0; $i -lt $totalShards; $i++) {
$shard = ShardArray $array $i $totalShards
$shard.Length | Should -BeLessOrEqual $maxLength
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Import-Module Pester

BeforeAll {
. "$PSScriptRoot\..\..\Copy-ApiVersion-Functions.ps1"
. "$PSScriptRoot\..\Copy-ApiVersion-Functions.ps1"
}

Describe "Copy-ApiVersion regex tests" {
Expand All @@ -11,25 +11,25 @@ Describe "Copy-ApiVersion regex tests" {
version = "2024-01-01-preview"
provider = "Microsoft.AgFoodPlatform"
versionStatus = "preview"
specsDir = '..\..\..\..\..\specification\agrifood\resource-manager\Microsoft.AgFoodPlatform\preview\2023-06-01-preview'
specsDir = '..\..\..\..\specification\agrifood\resource-manager\Microsoft.AgFoodPlatform\preview\2023-06-01-preview'
},
@{
version = "2024-01-01"
provider = "Microsoft.Compute\ComputeRP"
versionStatus = "stable"
specsDir = '..\..\..\..\..\specification\compute\resource-manager\Microsoft.Compute\ComputeRP\stable\2023-09-01'
specsDir = '..\..\..\..\specification\compute\resource-manager\Microsoft.Compute\ComputeRP\stable\2023-09-01'
},
@{
version = "7.6-preview.1"
provider = "Microsoft.KeyVault"
versionStatus = "preview"
specsDir = '..\..\..\..\..\specification\keyvault\data-plane\Microsoft.KeyVault\preview\7.6-preview.1'
specsDir = '..\..\..\..\specification\keyvault\data-plane\Microsoft.KeyVault\preview\7.6-preview.1'
},
@{
version = "7.5"
provider = "Microsoft.Compute\ComputeRP"
versionStatus = "stable"
specsDir = '..\..\..\..\..\specification\keyvault\data-plane\Microsoft.KeyVault\stable\7.5'
specsDir = '..\..\..\..\specification\keyvault\data-plane\Microsoft.KeyVault\stable\7.5'
}
) {
param($version, $provider, $versionStatus, $specsDir)
Expand All @@ -48,12 +48,12 @@ Describe "Copy-ApiVersion regex tests" {
# TODO: This is fragile. The tests stop working when a service team updates their readme.md. We should instead take fixed copies or something.
It "Default version gets updated" -TestCases @(
@{
inputReadme = '..\..\..\..\..\specification\compute\resource-manager\readme.md'
inputReadme = '..\..\..\..\specification\compute\resource-manager\readme.md'
apiVersion = "2024-01-01"
versionStatus = "stable"
},
@{
inputReadme = '..\..\..\..\..\specification\keyvault\data-plane\readme.md'
inputReadme = '..\..\..\..\specification\keyvault\data-plane\readme.md'
apiVersion = "7.6-preview.1"
versionStatus = "preview"
}
Expand Down
21 changes: 20 additions & 1 deletion eng/scripts/TypeSpec-Validation.ps1
Original file line number Diff line number Diff line change
@@ -1,16 +1,35 @@
[CmdletBinding()]
param (
[switch]$CheckAll = $false,
[int]$Shard = 0,
[int]$TotalShards = 1,
[switch]$GitClean = $false,
[switch]$DryRun = $false,
[string]$BaseCommitish = "HEAD^",
[string]$TargetCommitish = "HEAD"
)

if ($TotalShards -gt 0 -and $Shard -ge $TotalShards) {
throw "Shard ($Shard) must be less than TotalShards ($TotalShards)"
}

. $PSScriptRoot/Logging-Functions.ps1
. $PSScriptRoot/Suppressions-Functions.ps1
. $PSScriptRoot/Array-Functions.ps1

$typespecFolders, $checkedAll = &"$PSScriptRoot/Get-TypeSpec-Folders.ps1" `
-BaseCommitish:$BaseCommitish `
-TargetCommitish:$TargetCommitish `
-CheckAll:$CheckAll

$typespecFolders, $checkedAll = &"$PSScriptRoot/Get-TypeSpec-Folders.ps1" -BaseCommitish:$BaseCommitish -TargetCommitish:$TargetCommitish -CheckAll:$CheckAll
if ($TotalShards -gt 1 -and $TotalShards -le $typespecFolders.Count) {
$typespecFolders = shardArray $typespecFolders $Shard $TotalShards
}

Write-Host "Checking $($typespecFolders.Count) TypeSpec folders:"
foreach ($typespecFolder in $typespecFolders) {
Write-Host " $typespecFolder"
}

$typespecFoldersWithFailures = @()
if ($typespecFolders) {
Expand Down