You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/09/09 09:29:46 UTC
[GitHub] [pulsar] nicoloboschi opened a new pull request, #17571: [ci] Move cpp-tests job inside Pulsar CI workflow
nicoloboschi opened a new pull request, #17571:
URL: https://github.com/apache/pulsar/pull/17571
### Motivation
There are multiple benefits to adding the cpp-tests inside the main workflow:
- Now CPP tests runner is triggered even for docs-only changes
- The build artifacts are not reused
- CPP tests can be skipped in case of integration-tests failures or build broken or checkstyle errors etc.
- Less workflows
### Modifications
* Moved the job right after the integration-tests
* Now maven artifacts are reused from the previous job
* Added dependency to the new job for the latest step `Pulsar CI checks completed`
* Removed cpp-tests from the required checks because now it's included in `Pulsar CI checks completed`
- [x] `doc-not-needed`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] tisonkun commented on a diff in pull request #17571: [ci] Move cpp-tests job inside Pulsar CI workflow
Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #17571:
URL: https://github.com/apache/pulsar/pull/17571#discussion_r967208132
##########
.asf.yaml:
##########
@@ -50,7 +50,6 @@ github:
# See ./github/workflows/README.md for more documentation on this list.
contexts:
- Pulsar CI checks completed
- - cpp-tests
Review Comment:
I'm afraid that you should remove this line alone without CI change or set up a dummy `cpp-tests` task since it's required on the master branch and doesn't respect changes in PR.
Otherwise, we may ask for help from the INFRA team to unrequire the check manually or even force merge with admin privilege.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nicoloboschi commented on a diff in pull request #17571: [ci] Move cpp-tests job inside Pulsar CI workflow
Posted by GitBox <gi...@apache.org>.
nicoloboschi commented on code in PR #17571:
URL: https://github.com/apache/pulsar/pull/17571#discussion_r968440164
##########
.github/workflows/pulsar-ci.yaml:
##########
@@ -466,6 +465,68 @@ jobs:
run: |
gh-actions-artifact-client.js delete pulsar-java-test-image.zst
+ cpp-tests:
+ name:
Review Comment:
See this conversation https://github.com/apache/pulsar/pull/17571#discussion_r967208132
I will do a follow up pr
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nicoloboschi merged pull request #17571: [ci] Move cpp-tests job inside Pulsar CI workflow
Posted by GitBox <gi...@apache.org>.
nicoloboschi merged PR #17571:
URL: https://github.com/apache/pulsar/pull/17571
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on a diff in pull request #17571: [ci] Move cpp-tests job inside Pulsar CI workflow
Posted by GitBox <gi...@apache.org>.
lhotari commented on code in PR #17571:
URL: https://github.com/apache/pulsar/pull/17571#discussion_r968440325
##########
.github/workflows/pulsar-ci.yaml:
##########
@@ -466,6 +465,68 @@ jobs:
run: |
gh-actions-artifact-client.js delete pulsar-java-test-image.zst
+ cpp-tests:
+ name:
Review Comment:
I'm taking this back. Since cpp-tests is required, it cannot be renamed here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on a diff in pull request #17571: [ci] Move cpp-tests job inside Pulsar CI workflow
Posted by GitBox <gi...@apache.org>.
lhotari commented on code in PR #17571:
URL: https://github.com/apache/pulsar/pull/17571#discussion_r966836525
##########
.github/workflows/pulsar-ci.yaml:
##########
@@ -498,6 +499,66 @@ jobs:
- name: Delete docker image from GitHub Actions Artifacts
run: |
gh-actions-artifact-client.js delete pulsar-java-test-image.zst
+
+
+ cpp-tests:
+ name: "Pulsar CPP client tests"
+ runs-on: ubuntu-20.04
+ timeout-minutes: 120
+ if: ${{ steps.check_changes.outputs.docs_only != 'true' }}
+ needs: 'integration-tests'
+ steps:
+ - name: checkout
+ uses: actions/checkout@v2
+
+ - name: Tune Runner VM
+ uses: ./.github/actions/tune-runner-vm
+
+ - name: Cache local Maven repository
+ uses: actions/cache@v2
+ with:
+ path: |
+ ~/.m2/repository/*/*/*
+ !~/.m2/repository/org/apache/pulsar
+ key: ${{ runner.os }}-m2-dependencies-core-modules-${{ hashFiles('**/pom.xml') }}
+ restore-keys: |
+ ${{ runner.os }}-m2-dependencies-core-modules-
+
+ - name: Set up JDK 17
+ uses: actions/setup-java@v2
+ with:
+ distribution: 'temurin'
+ java-version: 17
+
+ - name: clean disk
+ run: |
+ sudo apt clean
+ docker rmi $(docker images -q) -f
+ df -h
+
+ - name: Install gh-actions-artifact-client.js
+ uses: apache/pulsar-test-infra/gh-actions-artifact-client/dist@master
+
+ - name: Restore maven build results from Github artifact cache
+ run: |
+ cd $HOME
+ $GITHUB_WORKSPACE/build/pulsar_ci_tool.sh restore_tar_from_github_actions_artifacts pulsar-maven-repository-binaries
+
+ - name: build cpp artifacts
+ run: |
+ echo "Build C++ client library"
+ pulsar-client-cpp/docker-build.sh
+
+ - name: run c++ tests
+ run: pulsar-client-cpp/docker-tests.sh
+
+ - name: Upload test-logs
+ uses: actions/upload-artifact@v3
+ if: failure()
+ continue-on-error: true
+ with:
+ name: test-logs
Review Comment:
```suggestion
name: cpp-tests-logs
```
##########
.github/workflows/pulsar-ci.yaml:
##########
@@ -498,6 +499,66 @@ jobs:
- name: Delete docker image from GitHub Actions Artifacts
run: |
gh-actions-artifact-client.js delete pulsar-java-test-image.zst
+
+
+ cpp-tests:
+ name: "Pulsar CPP client tests"
Review Comment:
```suggestion
name: "CI - CPP, Python Tests"
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on a diff in pull request #17571: [ci] Move cpp-tests job inside Pulsar CI workflow
Posted by GitBox <gi...@apache.org>.
lhotari commented on code in PR #17571:
URL: https://github.com/apache/pulsar/pull/17571#discussion_r968434691
##########
.github/workflows/pulsar-ci.yaml:
##########
@@ -466,6 +465,68 @@ jobs:
run: |
gh-actions-artifact-client.js delete pulsar-java-test-image.zst
+ cpp-tests:
+ name:
Review Comment:
```suggestion
name: "CI - CPP, Python Tests"
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on a diff in pull request #17571: [ci] Move cpp-tests job inside Pulsar CI workflow
Posted by GitBox <gi...@apache.org>.
lhotari commented on code in PR #17571:
URL: https://github.com/apache/pulsar/pull/17571#discussion_r968437737
##########
.github/workflows/pulsar-ci.yaml:
##########
@@ -466,6 +465,68 @@ jobs:
run: |
gh-actions-artifact-client.js delete pulsar-java-test-image.zst
+ cpp-tests:
+ name:
+ runs-on: ubuntu-20.04
+ timeout-minutes: 120
+ needs: [
+ 'changed_files_job',
+ 'integration-tests'
+ ]
+ if: ${{ needs.changed_files_job.outputs.docs_only != 'true' }}
+ steps:
+ - name: checkout
+ uses: actions/checkout@v2
+
+ - name: Tune Runner VM
+ uses: ./.github/actions/tune-runner-vm
+
+ - name: Cache local Maven repository
+ uses: actions/cache@v2
+ with:
+ path: |
+ ~/.m2/repository/*/*/*
+ !~/.m2/repository/org/apache/pulsar
+ key: ${{ runner.os }}-m2-dependencies-core-modules-${{ hashFiles('**/pom.xml') }}
+ restore-keys: |
+ ${{ runner.os }}-m2-dependencies-core-modules-
+
+ - name: Set up JDK 17
+ uses: actions/setup-java@v2
+ with:
+ distribution: 'temurin'
+ java-version: 17
+
+ - name: clean disk
+ run: |
+ sudo apt clean
+ docker rmi $(docker images -q) -f
+ df -h
Review Comment:
this should use the shared action used in other build jobs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] tisonkun commented on pull request #17571: [ci] Move cpp-tests job inside Pulsar CI workflow
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #17571:
URL: https://github.com/apache/pulsar/pull/17571#issuecomment-1242127958
BTW, this PR triggers only part of CI tasks. I may assume that a change in `pulsar-ci.yml` should trigger almost full test cases?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nicoloboschi commented on a diff in pull request #17571: [ci] Move cpp-tests job inside Pulsar CI workflow
Posted by GitBox <gi...@apache.org>.
nicoloboschi commented on code in PR #17571:
URL: https://github.com/apache/pulsar/pull/17571#discussion_r967222595
##########
.asf.yaml:
##########
@@ -50,7 +50,6 @@ github:
# See ./github/workflows/README.md for more documentation on this list.
contexts:
- Pulsar CI checks completed
- - cpp-tests
Review Comment:
you're right. I removed the name from the job. in this way for this pull the job must pass. Once this is merged, we should be fine and we can add the job name again
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] tisonkun commented on pull request #17571: [ci] Move cpp-tests job inside Pulsar CI workflow
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #17571:
URL: https://github.com/apache/pulsar/pull/17571#issuecomment-1242202886
It seems that we still take a long time to complete CI. For example, https://github.com/apache/pulsar/pull/17062/commits/10f7209d9fcdc0b0105fd8282f82634f8962cc29 made 11 hours ago and now it's not completed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on a diff in pull request #17571: [ci] Move cpp-tests job inside Pulsar CI workflow
Posted by GitBox <gi...@apache.org>.
lhotari commented on code in PR #17571:
URL: https://github.com/apache/pulsar/pull/17571#discussion_r966845866
##########
.github/workflows/pulsar-ci.yaml:
##########
@@ -498,6 +499,66 @@ jobs:
- name: Delete docker image from GitHub Actions Artifacts
run: |
gh-actions-artifact-client.js delete pulsar-java-test-image.zst
+
+
+ cpp-tests:
+ name: "CI - CPP, Python Tests"
+ runs-on: ubuntu-20.04
+ timeout-minutes: 120
+ if: ${{ steps.check_changes.outputs.docs_only != 'true' }}
+ needs: 'integration-tests'
+ steps:
+ - name: checkout
+ uses: actions/checkout@v2
+
+ - name: Tune Runner VM
+ uses: ./.github/actions/tune-runner-vm
+
+ - name: Cache local Maven repository
+ uses: actions/cache@v2
+ with:
+ path: |
+ ~/.m2/repository/*/*/*
+ !~/.m2/repository/org/apache/pulsar
+ key: ${{ runner.os }}-m2-dependencies-core-modules-${{ hashFiles('**/pom.xml') }}
+ restore-keys: |
+ ${{ runner.os }}-m2-dependencies-core-modules-
+
+ - name: Set up JDK 17
+ uses: actions/setup-java@v2
+ with:
+ distribution: 'temurin'
+ java-version: 17
+
+ - name: clean disk
+ run: |
+ sudo apt clean
+ docker rmi $(docker images -q) -f
+ df -h
+
+ - name: Install gh-actions-artifact-client.js
+ uses: apache/pulsar-test-infra/gh-actions-artifact-client/dist@master
+
+ - name: Restore maven build results from Github artifact cache
+ run: |
+ cd $HOME
+ $GITHUB_WORKSPACE/build/pulsar_ci_tool.sh restore_tar_from_github_actions_artifacts pulsar-maven-repository-binaries
+
+ - name: build cpp artifacts
+ run: |
+ echo "Build C++ client library"
+ pulsar-client-cpp/docker-build.sh
+
+ - name: run c++ tests
+ run: pulsar-client-cpp/docker-tests.sh
+
+ - name: Upload test-logs
+ uses: actions/upload-artifact@v3
+ if: failure()
+ continue-on-error: true
+ with:
+ name: cpp-test-logs
+ path: cpp-test-logs
Review Comment:
```suggestion
name: cpp-tests-logs
path: test-logs
```
the name is for the artifact (used `cpp-tests` (id of the build job, could be something else unique too) + `-log`, the path is the directory which contains the logs and that's `test-logs`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org