Skip to content

CI Only run arm tests nightly#26996

Merged
jjerphan merged 6 commits intoscikit-learn:mainfrom
thomasjpfan:do_not_run_arm_test_by_default_pr
Aug 8, 2023
Merged

CI Only run arm tests nightly#26996
jjerphan merged 6 commits intoscikit-learn:mainfrom
thomasjpfan:do_not_run_arm_test_by_default_pr

Conversation

@thomasjpfan
Copy link
Copy Markdown
Member

Reference Issues/PRs

Closes #26879

What does this implement/fix? Explain your changes.

This PR makes the ARM test run nightly. This PR also adds a [cirrus arm] commit tag to run the arm tests on CI if they are needed.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 2, 2023

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: 0f40a88. Link to the linter CI: here

Copy link
Copy Markdown
Contributor

@Micky774 Micky774 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@Micky774 Micky774 added Quick Review For PRs that are quick to review Waiting for Second Reviewer First reviewer is done, need a second one! labels Aug 3, 2023
Copy link
Copy Markdown
Member

@jjerphan jjerphan left a comment

Choose a reason for hiding this comment

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

Thank you, @thomasjpfan.

I have a question and a suggestion.

.cirrus.star Outdated

if "[cd build]" in commit_msg or "[cd build cirrus]" in commit_msg:
return fs.read(arm_wheel_yaml) + fs.read(arm_tests_yaml)
return fs.read(arm_wheel_yaml)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Before running tests and building wheels was done together.

Are tests run when the wheels are built? If not, could we change the logic so that we can trigger both when several tags are present?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I updated the PR to run the required jobs based on the tags. This means that if both [cd build] and [cirrus arm] is present, then both jobs run.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@jjerphan jjerphan merged commit 05133a5 into scikit-learn:main Aug 8, 2023
TamaraAtanasoska pushed a commit to TamaraAtanasoska/scikit-learn that referenced this pull request Aug 21, 2023
Co-authored-by: Julien Jerphanion <git@jjerphan.xyz>
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Sep 18, 2023
Co-authored-by: Julien Jerphanion <git@jjerphan.xyz>
jeremiedbb pushed a commit that referenced this pull request Sep 20, 2023
Co-authored-by: Julien Jerphanion <git@jjerphan.xyz>
REDVM pushed a commit to REDVM/scikit-learn that referenced this pull request Nov 16, 2023
Co-authored-by: Julien Jerphanion <git@jjerphan.xyz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Build / CI No Changelog Needed Quick Review For PRs that are quick to review Waiting for Second Reviewer First reviewer is done, need a second one!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cirrus CI usage limits in the future

3 participants