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

[TypeSpecRequirement] Add tests #29639

Merged
merged 30 commits into from
Jul 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
91093de
Add _ResponseCache parameter for testing
mikeharder Jun 29, 2024
6862c6a
Improve comment
mikeharder Jun 29, 2024
9dc25d8
Add skeleton test project
mikeharder Jun 29, 2024
00fd6ae
Add action
mikeharder Jun 29, 2024
05f2d46
Add concurrent tests
mikeharder Jun 29, 2024
2af85df
npm update
mikeharder Jun 29, 2024
e17cf79
Add test for suppressions
mikeharder Jun 30, 2024
31f6b6a
Add test for no tspconfig.yaml
mikeharder Jul 1, 2024
4674bfa
Remove debugging
mikeharder Jul 1, 2024
0da256e
Add debugging
mikeharder Jul 1, 2024
ae8447b
More debugging
mikeharder Jul 1, 2024
16c7c6c
Remove debugging
mikeharder Jul 1, 2024
81a0078
Write GitHub logs to correct stream
mikeharder Jul 1, 2024
1c523de
Add trailing newline
mikeharder Jul 1, 2024
2dde729
Remove blank line
mikeharder Jul 1, 2024
bdb3bfc
Add test for error parsing json
mikeharder Jul 1, 2024
ad2c350
Add test for success case
mikeharder Jul 1, 2024
3e89bf8
Add test for hand-written exists in main
mikeharder Jul 1, 2024
dc4a47e
Add tests for not exist in main
mikeharder Jul 1, 2024
27c33af
Rename test folder
mikeharder Jul 2, 2024
951b992
Add content to test file
mikeharder Jul 2, 2024
0f86d47
Add readme to fix Avocado
mikeharder Jul 2, 2024
ad4101d
Delete test readme
mikeharder Jul 2, 2024
ad07733
Add vite config
mikeharder Jul 2, 2024
a4caceb
Add test content
mikeharder Jul 2, 2024
a038aae
Fill empty file
mikeharder Jul 2, 2024
d542598
Merge branch 'main' into tsr-unit-tests
mikeharder Jul 2, 2024
01d3b65
Remove debug statement
mikeharder Jul 2, 2024
f6898c7
Merge branch 'tsr-unit-tests' of github.com:mikeharder/azure-rest-api…
mikeharder Jul 2, 2024
016ce40
npm update
mikeharder Jul 2, 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
23 changes: 23 additions & 0 deletions .github/workflows/typespec-requirement-test.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
name: TypeSpec Requirement - Test

on:
push:
branches:
- main
pull_request:
paths:
- package-lock.json
- package.json
- tsconfig.json
- .github/workflows/_reusable-eng-tools-test.yaml
- .github/workflows/typespec-requirement-test.yaml
- eng/scripts/**
- eng/tools/package.json
- eng/tools/tsconfig.json
- eng/tools/typespec-requirement/**

jobs:
typespec-requirement:
uses: ./.github/workflows/_reusable-eng-tools-test.yaml
with:
package: typespec-requirement
8 changes: 4 additions & 4 deletions eng/scripts/Logging-Functions.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ function LogWarning {
Write-Host ("##vso[task.LogIssue type=warning;]$args" -replace "`n", "%0D%0A")
}
elseif (Test-SupportsGitHubLogging) {
Write-Host ("::warning::$args" -replace "`n", "%0D%0A")
Write-Warning ("::warning::$args" -replace "`n", "%0D%0A")
Copy link
Member

Choose a reason for hiding this comment

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

What does this give us? Doesn't GH just interpret these logging messages and not display them?

Copy link
Member Author

@mikeharder mikeharder Jul 2, 2024

Choose a reason for hiding this comment

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

At least for GH, it seemed more correct to use both the intended pwsh Write-* method, along with including the GH-specific logging strings.

Write-Warning might write to stdout instead of stderr, so it might not matter in practice. What did matter in practice was using Write-Error instead of Write-Host, to ensure errors are always written to stderr.

Example:

test.concurrent("No tspconfig.yaml", async ({ expect }) => {
const { stderr, exitCode } = await checkAllUnder("specification/no-tspconfig");
expect(stderr).toContain("no files named 'tspconfig.yaml'");
expect(exitCode).toBe(1);
});

}
else {
Write-Warning "$args"
Expand All @@ -28,7 +28,7 @@ function LogErrorForFile($file, $errorString)
Write-Host ("##vso[task.logissue type=error;sourcepath=$file;linenumber=1;columnnumber=1;]$errorString" -replace "`n", "%0D%0A")
}
elseif (Test-SupportsGitHubLogging) {
Write-Host ("::error file=$file,line=1,col=1::$errorString" -replace "`n", "%0D%0A")
Write-Error ("::error file=$file,line=1,col=1::$errorString" -replace "`n", "%0D%0A")
}
else {
Write-Error "[Error in file $file]$errorString"
Expand All @@ -39,7 +39,7 @@ function LogError {
Write-Host ("##vso[task.LogIssue type=error;]$args" -replace "`n", "%0D%0A")
}
elseif (Test-SupportsGitHubLogging) {
Write-Host ("::error::$args" -replace "`n", "%0D%0A")
Write-Error ("::error::$args" -replace "`n", "%0D%0A")
}
else {
Write-Error "$args"
Expand All @@ -51,7 +51,7 @@ function LogDebug {
Write-Host "[debug]$args"
}
elseif (Test-SupportsGitHubLogging) {
Write-Host "::debug::$args"
Write-Debug "::debug::$args"
}
else {
Write-Debug "$args"
Expand Down
25 changes: 14 additions & 11 deletions eng/scripts/TypeSpec-Requirement.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ param (
[string] $TargetCommitish = "HEAD",
[Parameter(Position = 2)]
[string] $SpecType = "data-plane|resource-manager",
[string] $CheckAllUnder
[string] $CheckAllUnder,
# Reserved for testing. Call using:
# $ pwsh -Command '... -_ResponseCache @{"url"=200}'
[Parameter(DontShow)]
[hashtable] $_ResponseCache = @{}
)
Set-StrictMode -Version 3

Expand Down Expand Up @@ -59,7 +63,7 @@ $repoPath = Resolve-Path "$PSScriptRoot/../.."
$pathsWithErrors = @()

$filesToCheck = $CheckAllUnder ?
(Get-ChildItem -Path $CheckAllUnder -Recurse -File | Resolve-Path -Relative | ForEach-Object { $_ -replace '\\', '/' }) :
(Get-ChildItem -Path $CheckAllUnder -Recurse -File | Resolve-Path -Relative -RelativeBasePath $repoPath | ForEach-Object { $_ -replace '\\', '/' }) :
Copy link
Member Author

Choose a reason for hiding this comment

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

Fixes bug when testing files outside root "specification" folder

(Get-ChangedSwaggerFiles (Get-ChangedFiles $BaseCommitish $TargetCommitish))

$filesToCheck = $filesToCheck.Where({
Expand All @@ -72,7 +76,7 @@ if (!$filesToCheck) {
}
else {
# Cache responses to GitHub web requests, for efficiency and to prevent rate limiting
$responseCache = @{}
$responseCache = $_ResponseCache

# - Forward slashes on both Linux and Windows
# - May be nested 4 or 5 levels deep, perhaps even deeper
Expand Down Expand Up @@ -107,16 +111,15 @@ else {
if ($null -ne ${jsonContent}?["info"]?["x-typespec-generated"]) {
LogInfo " OpenAPI was generated from TypeSpec (contains '/info/x-typespec-generated')"

if ($file -match "specification/(?<rpFolder>[^/]+)/") {
$rpFolder = $Matches["rpFolder"];
$tspConfigs = @(Get-ChildItem -Path (Join-Path $repoPath "specification" $rpFolder) -Recurse -File
| Where-Object { $_.Name -eq "tspconfig.yaml" })
if ($file -match "^.*specification/[^/]+/") {
$rpFolder = $Matches[0];
$tspConfigs = @(Get-ChildItem -Path $rpFolder -Recurse -File | Where-Object { $_.Name -eq "tspconfig.yaml" })
Copy link
Member Author

Choose a reason for hiding this comment

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

Fixes bug when testing files outside root "specification" folder


if ($tspConfigs) {
LogInfo " Folder 'specification/$rpFolder' contains $($tspConfigs.Count) file(s) named 'tspconfig.yaml'"
LogInfo " Folder '$rpFolder' contains $($tspConfigs.Count) file(s) named 'tspconfig.yaml'"
}
else {
LogError ("OpenAPI was generated from TypeSpec, but folder 'specification/$rpFolder' contains no files named 'tspconfig.yaml'." `
LogError ("OpenAPI was generated from TypeSpec, but folder '$rpFolder' contains no files named 'tspconfig.yaml'." `
+ " The TypeSpec used to generate OpenAPI must be added to this folder.")
LogJobFailure
exit 1
Expand Down Expand Up @@ -176,12 +179,12 @@ else {
if ($responseStatus -eq 200) {
LogInfo " Branch 'main' contains path '$servicePath/stable', so spec already exists and is not required to use TypeSpec"
}
elseif ($response.StatusCode -eq 404) {
elseif ($responseStatus -eq 404) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Bug fix (unlikely to hit in practice)

LogInfo " Branch 'main' does not contain path '$servicePath/stable', so spec is new and must use TypeSpec"
$pathsWithErrors += $file
}
else {
LogError "Unexpected response from ${logUrlToStableFolder}: ${response.StatusCode}"
LogError "Unexpected response from ${logUrlToStableFolder}: ${responseStatus}"
LogJobFailure
exit 1
}
Expand Down
1 change: 1 addition & 0 deletions eng/tools/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"name": "azure-rest-api-specs-eng-tools",
"devDependencies": {
"@azure-tools/suppressions": "file:suppressions",
"@azure-tools/typespec-requirement": "file:typespec-requirement",
"@azure-tools/typespec-validation": "file:typespec-validation"
},
"private": true
Expand Down
21 changes: 21 additions & 0 deletions eng/tools/typespec-requirement/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
{
"name": "@azure-tools/typespec-requirement",
"private": true,
"type": "module",
"devDependencies": {
"@types/node": "^18.19.31",
"@vitest/coverage-v8": "^1.6.0",
"execa": "^9.3.0",
"typescript": "~5.4.5",
"vitest": "^1.6.0"
},
"scripts": {
"build": "tsc",
"postinstall": "npm run build",
"test": "vitest",
"test:ci": "vitest run --coverage --reporter=verbose"
},
"engines": {
"node": ">= 18.0.0"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Test
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"info": {
"x-typespec-generated": [
{
"emitter": "@azure-tools/typespec-autorest"
}
]
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
- tool: TypeSpecRequirement
path: ./resource-manager/Microsoft.Suppression/preview/2024-01-01-preview/*.json
reason: Test
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Test
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"info": {
"x-typespec-generated": [
{
"emitter": "@azure-tools/typespec-autorest"
}
]
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import { execa } from 'execa';
import { join } from 'path';
import { test } from 'vitest';

async function checkAllUnder(path: string, responseCache?: string) {
const repoRoot = join(__dirname, '..', '..', '..', '..');
const script = join('eng', 'scripts', 'TypeSpec-Requirement.ps1');

let command = `${script} -CheckAllUnder ${join(__dirname, path)}`;
if (responseCache) {
command += ` -_ResponseCache ${responseCache}`;
}

return await execa("pwsh", ["-Command", command], { cwd: repoRoot, reject: false });
}

test.concurrent("No files to check", async ({ expect }) => {
const { stdout, exitCode } = await checkAllUnder("specification/empty");

expect(stdout).toMatchInlineSnapshot(`"No OpenAPI files found to check"`);
expect(exitCode).toBe(0);
});

test.concurrent("Suppression", async ({ expect }) => {
const { stdout, exitCode } = await checkAllUnder("specification/suppression");

expect(stdout).toContain("Suppressed");
expect(exitCode).toBe(0);
});

test.concurrent("Parse error", async ({ expect }) => {
const { stdout, exitCode } = await checkAllUnder("specification/parse-error");

expect(stdout).toContain("cannot be parsed as JSON");
expect(exitCode).toBe(1);
});

test.concurrent("No tspconfig.yaml", async ({ expect }) => {
const { stderr, exitCode } = await checkAllUnder("specification/no-tspconfig");

expect(stderr).toContain("no files named 'tspconfig.yaml'");
expect(exitCode).toBe(1);
});

test.concurrent("Generated from TypeSpec", async ({ expect }) => {
const { stdout, exitCode } = await checkAllUnder("specification/typespec-generated");

expect(stdout).toContain("was generated from TypeSpec");
expect(exitCode).toBe(0);
});

test.concurrent("Hand-written, exists in main", async ({ expect }) => {
const { stdout, exitCode } = await checkAllUnder(
"specification/hand-written",
'@{"https://github.com/Azure/azure-rest-api-specs/tree/main/specification/hand-written/resource-manager/Microsoft.HandWritten/stable"=200}'
);

expect(stdout).toContain("was not generated from TypeSpec");
expect(stdout).toContain("'main' contains path");
expect(exitCode).toBe(0);
});

test.concurrent("Hand-written, does not exist in main", async ({ expect }) => {
const { stdout, exitCode } = await checkAllUnder(
"specification/hand-written",
'@{"https://github.com/Azure/azure-rest-api-specs/tree/main/specification/hand-written/resource-manager/Microsoft.HandWritten/stable"=404}'
);

expect(stdout).toContain("was not generated from TypeSpec");
expect(stdout).toContain("'main' does not contain path");
expect(exitCode).toBe(1);
});

test.concurrent("Hand-written, unexpected response checking main", async ({ expect }) => {
const { stdout, stderr, exitCode } = await checkAllUnder(
"specification/hand-written",
'@{"https://github.com/Azure/azure-rest-api-specs/tree/main/specification/hand-written/resource-manager/Microsoft.HandWritten/stable"=519}'
);

expect(stdout).toContain("was not generated from TypeSpec");
expect(stderr).toContain("Unexpected response");
expect(exitCode).toBe(1);
});
6 changes: 6 additions & 0 deletions eng/tools/typespec-requirement/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"extends": "../tsconfig.json",
"compilerOptions": {
"outDir": "./dist",
}
}
8 changes: 8 additions & 0 deletions eng/tools/typespec-requirement/vite.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { defineConfig } from 'vite'

export default defineConfig({
test: {
// Default timeout of 5 seconds is too low
testTimeout: 20000
}
})
Loading