-
Notifications
You must be signed in to change notification settings - Fork 228
Run full tests only on Wednesday scheduled jobs #1833
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
Conversation
It should work, but I can't test it. |
I was thinking of running these full tests on a less regular basis, maybe weekly or monthly instead? |
I agree. Changing the following lines to weekly? pygmt/.github/workflows/ci_tests.yaml Line 21 in 813142b
|
But this means the daily unit tests will become weekly. Or is there a way to just run the doctests, and not the unit tests? That way we could put it in the docs workflow as a weekly/monthly scheduled run (in addition to the current docs build). |
We have PRs merging into the main branch every few days, so the unit tests are also run every few days, making the daily unit tests less necessary. So a weekly job is still acceptable to me.
Sphinx has its own doctest extension, which can run doctests.
The above command works, but it reports many "errors" that are not reported by pytest. |
.github/workflows/ci_tests.yaml
Outdated
run: make test PYTEST_EXTRA="-r P" | ||
|
||
# Run full tests including doctests | ||
- name: Run full tests | ||
if: github.event_name == 'schedule' |
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.
Not sure if GitHub Actions uses &
or and
, but try something like this to run full tests on Wednesday only. Xref https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#schedule
if: github.event_name == 'schedule' | |
if: github.event_name == 'schedule' && github.event.schedule == '0 0 * * 3' |
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.
Not sure if GitHub Actions uses
&
orand
,
It's &&
(https://docs.github.com/en/actions/learn-github-actions/expressions#operators).
Co-authored-by: Wei Ji <[email protected]>
.github/workflows/ci_tests.yaml
Outdated
@@ -134,9 +134,15 @@ jobs: | |||
pip install dist/* | |||
|
|||
# Run the tests | |||
- name: Test with pytest | |||
- name: Run tests | |||
if: github.event_name != 'schedule' |
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.
This if
statement means the tests won't be run in daily jobs, which is not what we want. However, removing this if
statement means on Wednesday job, most tests will be run twice by both make test
and make fulltest
.
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.
What about something like
if: github.event_name != 'schedule' | |
if: github.event.schedule != '0 0 * * 3' |
Though not sure if it will error for regular (non-scheduled) tests, e.g. in PRs.
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.
No errors in PRs.
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.
Also need to update doc/maintenance.md
to mention the new full tests schedule.
Co-authored-by: Wei Ji <[email protected]>
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.
Nice! Just one last tiny suggestion, and wait for second approval to merge.
Co-authored-by: Wei Ji <[email protected]>
…1833) Co-authored-by: Wei Ji <[email protected]>
Previous expression was matching too strictly on the crontab, so using endsWith instead. xref https://docs.github.com/en/actions/learn-github-actions/expressions#endswith. Patches #1833.
Description of proposed changes
Address comment #1686 (comment).
Reminders
make format
andmake check
to make sure the code follows the style guide.doc/api/index.rst
.Slash Commands
You can write slash commands (
/command
) in the first line of a comment to performspecific operations. Supported slash commands are:
/format
: automatically format and lint the code/test-gmt-dev
: run full tests on the latest GMT development version