Skip to content

Commit

Permalink
PR checks perf improvements and organization (#568)
Browse files Browse the repository at this point in the history
Result when from 30-35min pipelines to under 20min

- Use corepack to install pnpm: Faster and respect the pnpm version set
in package.json instead of having another place to keep up to date
- dependency cache
- move swagger check to seperate job
- upgrade to new code coverage task
- Don't run core tests in ci
- Move all consitency check to independent github action workflow(Makes
it easier to see which one failed immediately without having to open
devops and dig into the steps)
  • Loading branch information
timotheeguerin committed Apr 2, 2024
1 parent 88b633a commit 0551990
Show file tree
Hide file tree
Showing 28 changed files with 244 additions and 142 deletions.
9 changes: 9 additions & 0 deletions .chronus/changes/improve-pr-speed-2024-3-2-0-21-51.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
changeKind: fix
packages:
- "@azure-tools/typespec-autorest-canonical"
- "@azure-tools/typespec-autorest"
---

Small performance improvements
9 changes: 9 additions & 0 deletions .chronus/changes/improve-pr-speed-2024-3-2-3-18-19.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
changeKind: internal
packages:
- "@azure-tools/typespec-azure-core"
- "@azure-tools/typespec-azure-portal-core"
- "@azure-tools/typespec-azure-resource-manager"
- "@azure-tools/typespec-client-generator-core"
---
19 changes: 19 additions & 0 deletions .github/actions/setup/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
name: Install dependencies
description: Setup for node. pnpm and dependencies
inputs:
node-version:
required: false
description: Node version for setup-node
default: 20.x

runs:
using: composite

steps:
- name: Install pnpm
uses: pnpm/action-setup@v2

- name: Set node version to ${{ inputs.node-version }}
uses: actions/setup-node@v4
with:
node-version: ${{ inputs.node-version }}
85 changes: 78 additions & 7 deletions .github/workflows/consistency.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ on:
branches: ["main"]
pull_request:
branches: ["main"]
merge_group:
workflow_dispatch: {}

# A workflow run is made up of one or more jobs that can run sequentially or in parallel
Expand All @@ -20,16 +21,11 @@ jobs:
fetch-depth: 0 ## Needed for Changesets to find `main` branch
submodules: recursive

- uses: ./.github/actions/setup

- run: git pull --force --no-tags origin main:main
name: Get main ref

- name: Use Node.js 20.x
uses: actions/setup-node@v3
with:
node-version: 20.x

- uses: pnpm/action-setup@v2

- run: pnpm install
name: Install dependencies

Expand All @@ -39,3 +35,78 @@ jobs:

- run: node eng/scripts/validate-core-submodule.js
name: Check that core submodule is merged to core repo

# Validate spell check
spellcheck:
name: Spell check
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
submodules: recursive
- uses: ./.github/actions/setup

- run: pnpm install
name: Install dependencies

- run: pnpm run cspell
name: Spell check

# Validate formatting
format:
name: Format
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
submodules: recursive
- uses: ./.github/actions/setup

- run: pnpm install
name: Install dependencies

- run: pnpm --filter="@typespec/prettier-plugin-typespec..." run build
name: Build prettier plugin

- run: pnpm run format:check
name: Check formatting

# Lint
lint:
name: Lint
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
submodules: recursive

- uses: ./.github/actions/setup

- run: pnpm install
name: Install dependencies

- run: pnpm --filter="@typespec/eslint-plugin..." run build
name: Build prettier plugin

- run: pnpm run lint
name: Lint

# Verify Arm OpenAPI common types are up to date
common-types-up-to-date:
name: Common types up to date
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
submodules: recursive

- uses: ./.github/actions/setup

- run: pnpm install
name: Install dependencies

- run: node eng/scripts/download-common-types.js v3
name: Swagger - Fetch common-types v3

- run: node eng/scripts/check-for-changed-files.js
name: Check Git Status For Changed Files
2 changes: 1 addition & 1 deletion eng/pipelines/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ extends:

- template: /eng/pipelines/templates/build.yml@self

- script: pnpm run test-official
- script: pnpm run test:ci
displayName: Test

- template: /eng/pipelines/templates/upload-coverage.yml@self
Expand Down
50 changes: 3 additions & 47 deletions eng/pipelines/jobs/build-and-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,73 +14,29 @@ steps:
parameters:
nodeVersion: ${{ parameters.nodeVersion }}

# Only running UI test on linux
- ${{ if ne(variables['Agent.OS'], 'Windows_NT') }}:
- template: /eng/pipelines/templates/install-browsers.yml

- script: pnpm run check-version-mismatch
displayName: Check version mismatch

- template: /eng/pipelines/templates/build.yml

- script: pnpm run test-official
- script: pnpm run test:ci
displayName: Test

- template: /eng/pipelines/templates/upload-coverage.yml

- script: pnpm run check-format
displayName: Check Formatting
condition: ne(variables['Agent.OS'], 'Windows_NT')

- script: pnpm run lint
displayName: Lint
condition: ne(variables['Agent.OS'], 'Windows_NT')

- script: node eng/scripts/download-common-types.js v3
displayName: Swagger - Fetch common-types v3
condition: ne(variables['Agent.OS'], 'Windows_NT')

- script: node eng/scripts/check-for-changed-files.js
displayName: Check Git Status For Changed Files

- script: pnpm run cspell
displayName: Spell check
condition: ne(variables['Agent.OS'], 'Windows_NT')

# Unlink node_modules folders to significantly improve performance of subsequent tasks
# which need to walk the directory tree (and are hardcoded to follow symlinks).
- script: pnpm run purge
displayName: "Purge dependencies"
condition: always()

# It's important for performance to pass "packages" as "searchFolder" to avoid looking under root "node_modules".
# PublishTestResults.searchFolder only supports absolute paths, not relative.
- task: PublishTestResults@2
inputs:
testResultsFormat: "JUnit"
searchFolder: "$(System.DefaultWorkingDirectory)/core/packages"
testResultsFiles: "*/test-results.xml"
mergeTestResults: true
failTaskOnFailedTests: false
testRunTitle: "[CORE] Test os: ${{ parameters.osName }}, node: ${{ parameters.nodeVersion }}"
displayName: Publish core test results
condition: always()

- task: PublishTestResults@2
inputs:
testResultsFormat: "JUnit"
searchFolder: "$(System.DefaultWorkingDirectory)/packages"
testResultsFiles: "*/test-results.xml"
mergeTestResults: true
failTaskOnFailedTests: false
testRunTitle: "[NON-CORE] Test os: ${{ parameters.osName }}, node: ${{ parameters.nodeVersion }}"
displayName: Publish non-core test results
testRunTitle: "Test os: ${{ parameters.osName }}, node: ${{ parameters.nodeVersion }}"
displayName: Publish test results
condition: always()

# Disable for now as e2e test are disabled - REMOVE BEFORE MERGING
# - task: 1ES.PublishPipelineArtifact@1
# displayName: Publish UI tests artifacts
# condition: ne(variables['Agent.OS'], 'Windows_NT')
# inputs:
# path: : ./core/packages/playground-website/test-results
# artifact: "uitestresults-${{ parameters.osName }}-node-${{ parameters.nodeVersion }}"
9 changes: 6 additions & 3 deletions eng/pipelines/jobs/e2e-job.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
jobs:
- job: e2e
displayName: "E2E Tests"

pool:
name: $(LINUXPOOL)
Expand All @@ -15,10 +16,12 @@ jobs:
submodules: true

- template: /eng/pipelines/templates/install.yml
- template: /eng/pipelines/templates/install-browsers.yml

- template: /eng/pipelines/templates/build.yml
parameters:
nodeVersion: "20.x"

- script: node packages/e2e-tests/e2e-tests.mjs
displayName: Run E2E tests
displayName: Cadl Ranch e2e tests

- script: pnpm test:e2e
displayName: UI Tests
29 changes: 29 additions & 0 deletions eng/pipelines/jobs/policies.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
parameters:

steps:
- checkout: self
submodules: true

- template: /eng/pipelines/templates/install.yml

- script: pnpm run check-version-mismatch
displayName: Check version mismatch

- script: pnpm run check-format
displayName: Check Formatting
condition: ne(variables['Agent.OS'], 'Windows_NT')

- script: pnpm run lint
displayName: Lint
condition: ne(variables['Agent.OS'], 'Windows_NT')

- script: node eng/scripts/download-common-types.js v3
displayName: Swagger - Fetch common-types v3
condition: ne(variables['Agent.OS'], 'Windows_NT')

- script: node eng/scripts/check-for-changed-files.js
displayName: Check Git Status For Changed Files

- script: pnpm run cspell
displayName: Spell check
condition: ne(variables['Agent.OS'], 'Windows_NT')
1 change: 1 addition & 0 deletions eng/pipelines/jobs/publish-artifacts.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
jobs:
- job: publish_artifacts
displayName: Publish Artifacts
condition: eq(variables['Build.Reason'], 'PullRequest') # Don't publish artifacts for merge queue builds

variables:
TYPESPEC_VS_CI_BUILD: true # Enable official Visual Studio extension build
Expand Down
28 changes: 17 additions & 11 deletions eng/pipelines/pr.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
trigger: none
trigger:
branches: # Used to trigger for GitHub merge queues
include:
- gh-readonly-queue/*
pr:
- main
- release/*
Expand All @@ -9,9 +12,9 @@ extends:
variables:
- template: /eng/pipelines/templates/variables/globals.yml@self
stages:
- stage: Build_And_Test
- stage: CI
jobs:
- job: Build_And_Test_Linux
- job: Linux
variables:
TYPESPEC_VS_CI_BUILD: true # Enable official Visual Studio extension build
TYPESPEC_SKIP_DOCUSAURUS_BUILD: true # Disable docusaurus build
Expand All @@ -35,11 +38,7 @@ extends:
dotnetVersion: 8.0.x
osName: linux

- template: /eng/pipelines/templates/swagger-checks.yml@self
parameters:
nodeVersion: $(nodeVersion)

- job: Build_And_Test_Windows
- job: Windows
variables:
TYPESPEC_VS_CI_BUILD: true # Enable official Visual Studio extension build
TYPESPEC_SKIP_DOCUSAURUS_BUILD: true # Disable docusaurus build
Expand All @@ -63,9 +62,16 @@ extends:
dotnetVersion: 8.0.x
osName: windows

- template: /eng/pipelines/jobs/e2e-job.yml@self
- job: AutorestChecks
displayName: "Autorest Checks"
pool:
name: $(LINUXPOOL)
image: $(LINUXVMIMAGE)
os: linux

steps:
- template: /eng/pipelines/templates/swagger-checks.yml@self
parameters:
nodeVersion: $(nodeVersion)

- template: /eng/pipelines/jobs/e2e-job.yml@self
nodeVersion: 20.x
- template: /eng/pipelines/jobs/publish-artifacts.yml@self
15 changes: 13 additions & 2 deletions eng/pipelines/templates/install.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ parameters:
- name: nodeVersion
type: string
default: 20.x

- name: pnpmStorePath
type: string
default: $(Pipeline.Workspace)/.pnpm-store
steps:
- task: UseDotNet@2
inputs:
Expand All @@ -15,8 +17,17 @@ steps:
displayName: Install Node.js
retryCountOnTaskFailure: 3

- script: npm install -g [email protected]
- task: Cache@2
inputs:
key: 'pnpm | "$(Agent.OS)" | pnpm-lock.yaml'
path: ${{ parameters.pnpmStorePath }}
displayName: Cache pnpm store
- script: |
corepack enable
corepack prepare pnpm@latest-8 --activate
displayName: Install pnpm
- script: pnpm config set store-dir ${{ parameters.pnpmStorePath }}
displayName: Setup pnpm cache dir

- script: |
echo "Node:"
Expand Down
Loading

0 comments on commit 0551990

Please sign in to comment.