-
Notifications
You must be signed in to change notification settings - Fork 169
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
test: Move MacOS (x86) pipelines to post-commit #122
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,6 +44,11 @@ jobs: | |
os: [ubuntu-latest] | ||
java_version: [8, 11, 17] | ||
test-target: [rust, java] | ||
is_push_event: | ||
- ${{ github.event_name == 'push' }} | ||
exclude: # exclude java 11 for pull_request event | ||
- java_version: 11 | ||
is_push_event: false | ||
fail-fast: false | ||
name: ${{ matrix.test-target }} test on ${{ matrix.os }} with java ${{ matrix.java_version }} | ||
runs-on: ${{ matrix.os }} | ||
|
@@ -73,12 +78,8 @@ jobs: | |
os: [macos-13] | ||
java_version: [8, 11, 17] | ||
test-target: [rust, java] | ||
is_push_event: | ||
- ${{ github.event_name == 'push' }} | ||
exclude: # exclude java 11 for pull_request event | ||
- java_version: 11 | ||
is_push_event: false | ||
fail-fast: false | ||
if: github.event_name == 'push' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we can consider to split tests to several pipelines later so each one won't take too long time. Then we probably can have MacOS pipeline in pull request. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea, I plan to split shuffle tests into a separate job later. But I think we can still keep the MacOS (x86) pipelines as post-commit because later on we need to add more pipelines for Spark SQL tests. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, I think MacOS x86 pipelines maybe less important as we are moving to Apple silicon mostly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yea I think so too |
||
name: ${{ matrix.test-target }} test on ${{ matrix.os }} with java ${{ matrix.java_version }} | ||
runs-on: ${{ matrix.os }} | ||
env: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can still run Java 11 on
ubuntu-latest
pre-commit?It seems that there's no quota limit for Linux runners and it doesn't slow down the overall CI runtime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can keep it for now but will need to re-consider this later when we add Spark SQL tests which increase the number of total jobs by 5X.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair point. I'm ok to move it to post-commit.