Add several syntactical, functional and logic improvements (#11) #595
Conversation
* add several improvements and simplifications * add a few script improvements * add tests logger and upload results as artifact * add path to source project files search in build step * add path to project files search in test step
.github/workflows/ci-cd.yml
Outdated
| @@ -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' | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I'm somewhat confused by this process. First, the default branch is vnext, not master. We don't create releases when we are updating vnext. Also, we don't push directly to vnext, we generally PR to it. What am I missing?
There was a problem hiding this comment.
We don't create releases when we are updating vnext
If you are not making releases from your default branch (vnext), we will change it to the correct one. Is it master or another branch ?
Also not every push commit creates a new GitHub tag and release. In the documentation we submitted with the last PR, you can check out the exact prerequisite to trigger an output (meaning create a new tag) from the "Bump GH tag" step, which in turn triggers a GitHub release.
The path is this:
Push commit in release branch with SemVer label in the commit message
-> "Bump GH tag" step creates a new GitHub tag and pushes it to the repo
-> CD job evaluates if there is a new GitHub tag and if there is, executes
-> the "Create and publish release" step in the CD job uses the newly created and pushed GitHub tag to create a GitHub release
we generally PR to it
The merge of a PR triggers a push commit, so you can either create a release though your release branch, by committing into it or merging a PR.
If you have any other questions about the automated release process, please let us know! 🙂
There was a problem hiding this comment.
We create releases off the master branch.
There was a problem hiding this comment.
@darrelmiller with the last commit:
- switch the release branch from default_branch to master
- remove the data gatherer and conditionals handler steps, since they were used specifically for the default_branch as release branch logic
In the current state of the pipeline if there are no errors present, a push commit or PR merge to the master branch with a SemVer label in the title, would deploy (publish packages and create a GitHub release).
Push commits or PR merges to the default_branch (or any other branch, except the default_branch) would trigger the pipeline, but not deploy.
| -c Release # ` | ||
| # -o $env:ARTIFACTS_FOLDER ` | ||
| # /p:Version=$projectNewVersion | ||
| Get-ChildItem -Path src/ -Filter *.csproj -Exclude *Workbench* -File -Recurse | ForEach-Object { |
There was a problem hiding this comment.
Replaced static array of project locations with a more elegant solution.
There was a problem hiding this comment.
out of curiosity, any reason why you don't simply target the sln?
There was a problem hiding this comment.
We just took the build.cmd file as an example and just never gave it a second thought, but now that you mention it unless there is a specific reason not to, we could change the build step to building only the solution instead 🙂
There was a problem hiding this comment.
We don't publish the Workbench tool, so I didn't bother building it for publishing nugets. I don't have a problem building it as part of the pipeline. However currently dotnet build can't build the Workbench project because of some error about the GenerateResource task. Not sure what that is about.
| -c Release # ` | ||
| # -o $env:ARTIFACTS_FOLDER ` | ||
| # /p:Version=$projectNewVersion | ||
| Get-ChildItem -Path src/ -Filter *.csproj -Exclude *Workbench* -File -Recurse | ForEach-Object { |
There was a problem hiding this comment.
out of curiosity, any reason why you don't simply target the sln?
|
Also, I forgot to mention that based on this PRs resolution, we will optimize the CodeQL pipeline as well. |
|
@baywet it is an incredible coincidence, but today we discovered that the action for creating release in this pipeline is unmaintained and is quite unstable, so we decided to replace it with a short script utilizing GitHub CLI. I was just about to commit the change and it will show in this PR, so it would be more convenient for merging. |
You will see that the pipeline fails at the "Run unit tests" step and here is why.
We discovered that on
ubuntu-latest(Ubuntu 20.04), one of the tests from theMicrosoft.OpenApi.Testsproject fails. The test is calledMicrosoft.OpenApi.Tests.OpenApiWorkspaceTests.OpenApiWorkspacesShouldNormalizeDocumentLocations.The same test completes successful on
Windows 10andwindows-latest(Windows Server 2019).Now to make it even more interesting, in the previous iteration of the pipeline you can see in the logs that the test fails, but for some odd reason, there is no exit code and therefor the pipeline doesn't fail.
In the one this PR is for, the same test again fails BUT now the pipeline also fails as it should.
In case this fail is a false positive, we've tested the pipeline on
windows-latestand everything runs smoothly, since the test in question does not fail, so we can switch the runner of the CI job fromubuntu-latesttowindows-latest.Other than that, check out in a bit the comments on each change for info on the optimizations.
Resolves #594