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

Split CI #2210

Open
wants to merge 18 commits into
base: dev-2.11
Choose a base branch
from
Open

Split CI #2210

wants to merge 18 commits into from

Conversation

ychiucco
Copy link
Collaborator

closes #2203

Checklist before merging

  • I added an appropriate entry to CHANGELOG.md
  • I added logging to new code - if appropriate.
  • I merged main into the current branch.

This comment was marked as off-topic.

Copy link

github-actions bot commented Jan 23, 2025

Coverage report

This PR does not seem to contain any modification to coverable code.

@ychiucco ychiucco marked this pull request as ready for review January 23, 2025 09:43
@ychiucco ychiucco requested a review from tcompa January 23, 2025 09:44
@tcompa
Copy link
Collaborator

tcompa commented Jan 23, 2025

The API tests are actually using the SSH containers, thus we need a better split.

For the record, here is part of the output of poetry run pytest -vv tests/v2/03_api/ --durations 0:

$ poetry run pytest -vv tests/v2/03_api/ --durations 0

[...]

======= slowest durations ====================

25.33s call     tests/v2/03_api/test_api_task_collection.py::test_task_collection_from_pypi[None]
24.44s call     tests/v2/03_api/test_api_task_collection.py::test_task_collection_from_pypi[1.3.2]
21.56s call     tests/v2/03_api/test_api_task_collection_ssh.py::test_task_collection_ssh_from_pypi
16.44s setup    tests/v2/03_api/test_api_task_collection_ssh.py::test_task_collection_ssh_from_pypi
12.38s call     tests/v2/03_api/test_api_task_lifecycle.py::test_lifecycle[slurm_ssh]
11.15s call     tests/v2/03_api/test_api_task_lifecycle.py::test_lifecycle[local]
11.09s call     tests/v2/03_api/test_api_task_collection_failures.py::test_invalid_manifest
8.08s call     tests/v2/03_api/test_api_task_collection.py::test_task_collection_from_wheel_non_canonical
6.46s call     tests/v2/03_api/test_api_task_collection_ssh.py::test_task_collection_ssh_from_wheel
6.39s call     tests/v2/03_api/test_api_task_collection_failures.py::test_failure_cleanup
6.14s call     tests/v2/03_api/test_api_task_collection.py::test_task_collection_from_wheel_file
5.34s call     tests/v2/03_api/test_api_task_collection_failures.py::test_missing_task_executable
4.53s setup    tests/v2/03_api/test_api_workflow.py::test_delete_workflow
3.00s setup    tests/v2/03_api/test_api_task_collection_custom.py::test_task_collection_custom
1.65s call     tests/v2/03_api/test_api_job.py::test_project_apply_workflow_subset
1.51s teardown tests/v2/03_api/test_unit_timezone.py::test_timezone_api
0.99s call     tests/v2/03_api/admin/test_admin_others.py::test_task_query

[...]

================= 1 failed, 143 passed, 919 warnings in 222.71s (0:03:42) ==================

Some first comments:

  1. Installing fractal-tasks-core from PyPI is taking a lot.
  2. The SSH test requires containers, and then should run from the GitHub actions where containers are already present (otherwise we have a 16-seconds overhead)
  3. TBD how can tests/v2/03_api/test_api_task_collection_failures.py::test_invalid_manifest take so long

@tcompa
Copy link
Collaborator

tcompa commented Jan 27, 2025

  • Move task-collection and lifecycle tests from 03_api to 06_tasks (to be renamed 06_tasks_lifecycle)
  • Make sure that tests 06 are part of the GHA job which does use containers
  • Make sure that tests 03 now run quite fast, because they should not involve any background task (to be checked with --durations 0). If not, a bit of profiling wouldn't hurt.

@tcompa
Copy link
Collaborator

tcompa commented Jan 27, 2025

  • API: check whether some parametrize can be moved into the test without making it unreadable
  • Review 06 durations, and look for tests that can be removed or merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants