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 full tests (including inline doctests) in workflows #2286

Closed
2 tasks done
seisman opened this issue Dec 29, 2022 · 4 comments · Fixed by #2288 or #2456
Closed
2 tasks done

Run full tests (including inline doctests) in workflows #2286

seisman opened this issue Dec 29, 2022 · 4 comments · Fixed by #2288 or #2456
Assignees
Labels
maintenance Boring but important stuff for the core devs
Milestone

Comments

@seisman
Copy link
Member

seisman commented Dec 29, 2022

As mentioned in #2281 (comment):

@seisman seisman added the maintenance Boring but important stuff for the core devs label Dec 29, 2022
@seisman seisman added this to the 0.9.0 milestone Dec 29, 2022
@seisman seisman closed this as completed Dec 31, 2022
@seisman
Copy link
Member Author

seisman commented Mar 19, 2023

I'm reopening the issue, because it seems the "Tests" workflow still doesn't run the full tests on Wednesday (e.g., https://github.com/GenericMappingTools/pygmt/actions/runs/4421700978/jobs/7752774811).

@seisman seisman reopened this Mar 19, 2023
@seisman
Copy link
Member Author

seisman commented Mar 19, 2023

Our current strategy is to run the unit tests in daily scheduled CI jobs and only run the "full tests" (unit tests + doctests) on the Wednesday scheduled CI job.

Actually, looking at the list of daily scheduled runs (https://github.com/GenericMappingTools/pygmt/actions/workflows/ci_tests.yaml?query=event%3Aschedule), I find it very difficult to know which job runs on Wednesday. What's worse is, if all unit tests pass but some doctests fail, we will see six passing runs and one failing run every week. It's very likely that we will miss the failing run and don't know that we're having failing doctests.

I think the simplest solution is "always run the full tests in scheduled jobs".

As mentioned in #1833 (comment),

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.

If you think that running the full tests daily is a waste of CI resources, maybe we can run every two or three days or once per week.

@weiji14
Copy link
Member

weiji14 commented Mar 19, 2023

Alternatively, could we revisit a solution to run only the doctests as mentioned in #1833 (comment)? I'm thinking of a workflow that does:

rm pygmt/tests/*
pytest --doctest-modules ...

That way, we have a dedicated workflow to catch errors only in the doctests.

@seisman
Copy link
Member Author

seisman commented Mar 19, 2023

Alternatively, could we revisit a solution to run only the doctests as mentioned in #1833 (comment)? I'm thinking of a workflow that does:

rm pygmt/tests/*
pytest --doctest-modules ...

That way, we have a dedicated workflow to catch errors only in the doctests.

It sounds like a clever solution. I just tried to do some statistics about our tests.

Currently, we have 696 tests, including 628 unit tests (in the pygmt/tests directory) and 68 doctests (in Python source codes). Among the 68 doctests, 36 doctests are skipped by defining the special variable __doctest_skip__, which is provided by the pytest-doctestplus plugin.

These counts are get by running the following commands (I removed the tool.pytest.ini_options section from pyproject.toml before running the commands):

  • pytest --collect-only pygmt: 628 (all unit tests)
  • pytest --collect-only --doctest-modules pygmt: 696 (all unit tests + all doctests)
  • pytest --collect-only --doctest-plus pygmt: 660 (all unit tests + 32 "unskipped" doctests)
  • pytest --collect-only --doctest-only pygmt: 32 (32 "unskipped" doctests)
  • pytest --collect-only --doctest-modules --ignore=pygmt/tests pygmt: 68 (all doctests)

There is no way to run the 38 "skipped" doctests only, but it seems we can use the last command in a new workflow to run the 68 doctests only.

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
2 participants