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.

Sign up for GitHub

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 several syntactical, functional logical improvements #11

Merged
merged 5 commits into from
May 20, 2021
Merged

Add several syntactical, functional logical improvements #11

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
44ff419
add several improvements and simplifications
aleks-ivanov May 20, 2021
89725d9
add a few script improvements
aleks-ivanov May 20, 2021
04952d5
add tests logger and upload results as artifact
aleks-ivanov May 20, 2021
fa6afee
add path to source project files search in build step
aleks-ivanov May 20, 2021
5766fc2
add path to project files search in test step
aleks-ivanov May 20, 2021
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
67 changes: 38 additions & 29 deletions .github/workflows/ci-cd.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
name: CI/CD Pipeline

on: [push, pull_request, workflow_dispatch]
on: [ push, pull_request, workflow_dispatch ]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Cosmetic change for better visibility.


jobs:
ci:
name: Continuous Integration
runs-on: ubuntu-latest
outputs:
latest_version: ${{ steps.tag_generator.outputs.new_version }}
is_default_branch: ${{ steps.conditionals_handler.outputs.is_default_branch }}
Copy link
Collaborator Author

@aleks-ivanov aleks-ivanov May 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Removed unnecessary value.

env:
ARTIFACTS_FOLDER: ${{ github.workspace }}/Artifacts
GITHUB_RUN_NUMBER: ${{ github.run_number }}
Expand All @@ -23,7 +22,7 @@ jobs:
shell: pwsh
run: |
# Get default branch
$repo = 'microsoft/OpenAPI.NET'
$repo = "${{ github.repository }}"
Copy link
Collaborator Author

@aleks-ivanov aleks-ivanov May 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Replace static value with more elegant dynamic value - ${{ github.repository }}.

$defaultBranch = Invoke-RestMethod -Method GET -Uri https://api.github.com/repos/$repo | Select-Object -ExpandProperty default_branch
Write-Output "::set-output name=default_branch::$(echo $defaultBranch)"

Expand All @@ -33,11 +32,22 @@ jobs:
run: |
$defaultBranch = "${{ steps.data_gatherer.outputs.default_branch }}"
$githubRef = "${{ github.ref }}"
$githubEventName = "${{ github.event_name }}"
$isDefaultBranch = 'false'
$isPush = 'false'
$isPushToDefaultBranch = 'false'
if ( $githubRef -like "*$defaultBranch*" ) {
$isDefaultBranch = 'true'
}
if ( $githubEventName -eq 'push' ) {
$isPush = 'true'
}
if ( $githubRef -like "*$defaultBranch*" -and $githubEventName -eq 'push' ) {
$isPushToDefaultBranch = 'true'
}
Write-Output "::set-output name=is_default_branch::$(echo $isDefaultBranch)"
Write-Output "::set-output name=is_push::$(echo $isPush)"
Write-Output "::set-output name=is_push_to_default_branch::$(echo $isPushToDefaultBranch)"
Copy link
Collaborator Author

@aleks-ivanov aleks-ivanov May 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

added two more conditional statements:

  1. is_push = when the workflow is triggered by a push commit
  2. is_push_to_default_branch = when the workflow is triggered by the a push commit in the default branch


- name: Checkout repository
id: checkout_repo
Expand All @@ -46,7 +56,7 @@ jobs:
token: ${{ secrets.GITHUB_TOKEN }}
fetch-depth: 0

- if: steps.conditionals_handler.outputs.is_default_branch == 'true'
- if: steps.conditionals_handler.outputs.is_push_to_default_branch == 'true'
Copy link
Collaborator Author

@aleks-ivanov aleks-ivanov May 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Used the newly added is_push_to_default_branch condition, instead of the is_default_branch, to further restrict the new tag generating step to only execute when a push commit to the default branch is made.

name: Bump GH tag
id: tag_generator
uses: mathieudutour/github-tag-action@v5.4
Expand All @@ -59,41 +69,40 @@ jobs:
id: build_projects
shell: pwsh
run: |
$projectsArray = @(
'.\src\Microsoft.OpenApi\Microsoft.OpenApi.csproj',
'.\src\Microsoft.OpenApi.Readers\Microsoft.OpenApi.Readers.csproj',
'.\src\Microsoft.OpenApi.Tool\Microsoft.OpenApi.Tool.csproj'
)
$gitNewVersion = if ("${{ steps.tag_generator.outputs.new_version }}") {"${{ steps.tag_generator.outputs.new_version }}"} else {$null}
$projectCurrentVersion = ([xml](Get-Content .\src\Microsoft.OpenApi\Microsoft.OpenApi.csproj)).Project.PropertyGroup.Version
$projectCurrentVersion = ([xml](Get-Content -Path .\src\Microsoft.OpenApi\Microsoft.OpenApi.csproj)).Project.PropertyGroup.Version
Copy link
Collaborator Author

@aleks-ivanov aleks-ivanov May 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Add argument as best practice. (writing scripts in PowerShell is all cmdlets and arguments should be written and with their full name for optimal legibility)

$projectNewVersion = $gitNewVersion ?? $projectCurrentVersion

$projectsArray | ForEach-Object {
dotnet build $PSItem `
-c Release # `
# -o $env:ARTIFACTS_FOLDER `
# /p:Version=$projectNewVersion
Get-ChildItem -Path src/ -Filter *.csproj -Exclude *Workbench* -File -Recurse | ForEach-Object {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Replace static array of project locations with a more elegant solution.

dotnet build $PSItem.FullName `
--configuration Release # `
# --output $env:ARTIFACTS_FOLDER `
# -property:Version=$projectNewVersion
}

# Move NuGet packages to separate folder for pipeline convenience
# New-Item Artifacts/NuGet -ItemType Directory
# Get-ChildItem Artifacts/*.nupkg | Move-Item -Destination "Artifacts/NuGet"
# New-Item -Name Artifacts/NuGet -ItemType Directory
# Get-ChildItem -Path Artifacts/ -Filter *.nupkg | Move-Item -Destination Artifacts/NuGet
Copy link
Collaborator Author

@aleks-ivanov aleks-ivanov May 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Add arguments as best practice.


- name: Run unit tests
id: run_unit_tests
shell: pwsh
run: |
$testProjectsArray = @(
'.\test\Microsoft.OpenApi.Tests\Microsoft.OpenApi.Tests.csproj',
'.\test\Microsoft.OpenApi.Readers.Tests\Microsoft.OpenApi.Readers.Tests.csproj',
'.\test\Microsoft.OpenApi.SmokeTests\Microsoft.OpenApi.SmokeTests.csproj'
)

$testProjectsArray | ForEach-Object {
dotnet test $PSItem `
-c Release
Get-ChildItem -Path test/ -Filter *.csproj -File -Recurse | ForEach-Object {
$fileBaseName = $PSItem.Basename
dotnet test $PSItem.FullName `
--configuration Release `
--logger "trx;LogFileName=$fileBaseName.trx;verbosity=normal" `
--results-directory TestResults/
Copy link
Collaborator Author

@aleks-ivanov aleks-ivanov May 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Replaced static array of test project locations with a more elegant solution.
Add test logger, which generates test results with normal verbosity in the root/TestResults/ folder.

}

- name: Upload test results as artifacts
id: ul_testresults_artifact
uses: actions/upload-artifact@v1
with:
name: TestResults
path: TestResults/

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Upload test results as pipeline artifact.

# - if: steps.tag_generator.outputs.new_version != ''
# name: Upload NuGet packages as artifacts
# id: ul_packages_artifact
Expand All @@ -103,7 +112,7 @@ jobs:
# path: Artifacts/NuGet/

cd:
if: needs.ci.outputs.is_default_branch == 'true' && needs.ci.outputs.latest_version != ''
if: needs.ci.outputs.latest_version != ''
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed is_default_branch condition, because it is logically unnecessary, since the step that dictates if the other condition needs.ci.outputs.latest_version is true or false, only executes when the workflow is triggered by a push commit in the default branch.

name: Continuous Deployment
needs: ci
runs-on: ubuntu-latest
Expand All @@ -120,8 +129,8 @@ jobs:
# continue-on-error: true
# shell: pwsh
# run: |
# Get-ChildItem NuGet/*.nupkg | ForEach-Object {
# nuget push $PSItem `
# Get-ChildItem -Path NuGet/ -Filter *.nupkg | ForEach-Object {
Copy link
Collaborator Author

@aleks-ivanov aleks-ivanov May 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Add arguments as best practice

# nuget push $PSItem.FullName `
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Add .FullName to ensure the command always takes the file path as value.

# -ApiKey $env:NUGET_API_KEY `
# -Source https://api.nuget.org/v3/index.json
# }
Expand Down