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

Found a possibility of limiting the Linux build only for the 24.0.9 LTI version #1218

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
29 changes: 21 additions & 8 deletions .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -92,21 +92,34 @@ jobs:

build:
needs: fetch_merge_commit_sha_from_lsp4ij_PR
runs-on: ${{ matrix.os }}
name: build ( ${{ matrix.runtime.name }}, ${{ matrix.excludeLTITag }} )
runs-on: ${{ matrix.runtime.os }}
strategy:
fail-fast: false
matrix:
runtime: [ linux, mac, windows ]
include:
- runtime: linux
runtime:
- name: linux
os: ubuntu-latest
reportName: linux-test-report
- runtime: mac
- name: mac
os: macOS-latest
reportName: mac-test-report
- runtime: windows
- name: windows
os: windows-latest
reportName: windows-test-report
# It’s not necessary to remove the below condition even after removing the '24.0.9' version from the 'tag' array in the cron job build.
# Specify the LTI tag to limit the OS.
# Adding a check to verify if 'refLTITag' is equal to '24.0.9'.
excludeLTITag:
- ${{ inputs.refLTITag == '24.0.9' }}
# Adding a check on macOS and Windows to verify if 'excludeLTITag' is equal to 'true'. The value of 'excludeLTITag' is used to determine whether the job need to be executed.
exclude:
- excludeLTITag: true
runtime:
name: mac
- excludeLTITag: true
runtime:
name: windows
env:
USE_LOCAL_PLUGIN: ${{ inputs.useLocalPlugin || false }}
REF_LSP4IJ: ${{ needs.fetch_merge_commit_sha_from_lsp4ij_PR.outputs.pr_details }}
Expand Down Expand Up @@ -168,6 +181,6 @@ jobs:
if: ${{ failure() && steps.run_tests.conclusion == 'failure' }}
uses: actions/[email protected]
with:
name: ${{ matrix.reportName }}-LTI-${{ env.REF_LTI_TAG || 'default' }}-LSP4IJ-${{ env.LSP4IJ_BRANCH || 'default' }}
name: ${{ matrix.runtime.reportName }}-LTI-${{ env.REF_LTI_TAG || 'default' }}-LSP4IJ-${{ env.LSP4IJ_BRANCH || 'default' }}
Copy link
Contributor

Choose a reason for hiding this comment

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

@vaisakhkannan Can you explain the reason of adding runtime here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dessina-devasia , There is a change in the matrix strategy with this PR.
Previously, the code was:

matrix:
  runtime: [linux, mac, windows]
  include:
    - runtime: linux
      os: ubuntu-latest
      reportName: linux-test-report
    - runtime: mac
      os: macOS-latest
      reportName: mac-test-report
    - runtime: windows
      os: windows-latest
      reportName: windows-test-report

With this PR, the goal is to retrieve the unique reportName using only via matrix.runtime.reportName.

path: |
liberty-tools-intellij/build/reports/
liberty-tools-intellij/build/reports/
Copy link
Contributor

Choose a reason for hiding this comment

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

No change in this section, seems an extra line added here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems like there is a bug from GitHub, I could not see an extra line.

Copy link
Member

Choose a reason for hiding this comment

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

Most likely you added a carriage return at the end of the line. Or removed one. It is highly unlikely there is a bug with github.

26 changes: 25 additions & 1 deletion docs/LSP4IJ-Continuous-Integration.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ This guide offers a detailed explanation of the Continuous Integration (CI) setu
- [How to specify LTI tags or branches in CI-CD builds](#how-to-specify-lti-tags-or-branches-in-ci-cd-builds)
- [How to get Slack notifications for LSP4IJ PRs](#how-to-get-slack-notifications-for-lsp4ij-prs)
- [Sending build results to the Slack channel](#sending-build-results-to-the-slack-channel)
- [How to specify an LTI tag to limit OS in CI-CD Cron Job builds](#how-to-specify-an-lti-tag-to-limit-os-in-ci-cd-cron-job-builds)


# Workflow of normal build.yaml execution.
Expand Down Expand Up @@ -192,6 +193,8 @@ The next job, **Run Lsp4ij Main**, is the build which runs against the LSP4IJ ma

![Result](images/result-cron-job.png)

The `runtime` name against which the job `build` is running is displayed in brackets, along with either `true` or `false`. Value `true` means the job is currently running under the **LTI** tag, which is preventing other **OS** from running the builds. This means the screenshot above shows that there is no run for both **Windows** and **Mac** for LTI version `24.0.9`. Value `false` means the job is running under the **LTI** tag or the **main** branch, which is not limiting other **OS** from running the builds. You can see jobs under the **main** branch and **24.0.12** tag.

The Annotations section contains errors that show the details of the failed builds and also warnings that show PRs with merge conflicts.

![Warnings](images/Warnings-PR.png)
Expand Down Expand Up @@ -315,4 +318,25 @@ By following these steps, this job ensures that a notification is sent to Slack

### This is how the slack message shows the build status

![Build Results in Slack](images/build-results-slack-view.png)
![Build Results in Slack](images/build-results-slack-view.png)

# How to specify an LTI tag to limit OS in CI-CD Cron Job builds

In order to limit the **OS** to run the builds against the **LTI** Tag, the **LTI** Tag must be specified in one location within the **build.yaml** file. This is used in the `build` job. There is a variable `excludeLTITag` under the `matrix` that is used to check the **LTI** Tag and confirm that the exact one is coming from the Cron Job call.

![specify LTI tag to limit os](images/specify-LTI-tag-to-limit-os.png)

There is no need to update the code each time. That means, If we remove `24.0.9` from the **LTI** tags in the **cronJob.yaml** file, there is no need to modify anything in the **build.yaml** file. The variable `excludeLTITag` will calculate its value and will return `false` always since it is not there in **LTI** tag. This ensures it works as expected for both **normal** and **cron job** builds.
In the future, if there is a need to limit the LTI tag `24.0.12` to Linux only, the only change required in the **build.yaml** file is to update `24.0.9` to `24.0.12`.
For cases where the **LTI** tag `24.0.12` should be limited to **macOS** only, the exclude section can be updated as follows:
```java
exclude:
- excludeLTITag: true
runtime:
name: linux
- excludeLTITag: true
runtime
name: windows
```

> Note: Whatever we are currently running for normal builds will continue to work as expected, even after removing the specified excludeLTITag from the LTI tag array in the cron job build.
Binary file modified docs/images/result-cron-job.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added docs/images/specify-LTI-tag-to-limit-os.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading