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

ci: Optimize build + test pipeline #3300

Merged
merged 31 commits into from
Jul 3, 2023

Conversation

frascuchon
Copy link
Member

@frascuchon frascuchon commented Jun 30, 2023

Description

This PR reviews and organizes the CI pipeline. With the changes introduced here, all tests are running as expected.

Also, the cache action has been updated in order to fix a cache resolution problem which allows to reuse the cache for the rest of the jobs.

Two blocks of tests are launched separately

base tests (most of the server tests and some basic client tests) and extra tests (training, monitor, listeners...). The pipeline falls into errors if resource all tests are launched in the same job.
In the next step, those groups should be reviewed and improved.

Refs #3291

Reusable jobs extraction

The job running the tests has been extracted as a separate workflow, which helps with reusability and workflow code maintenance. The same has been applied to the build Python package.

Improve conditional executions

The IS_DEPLOYABLE step conditions have been removed. Now, this condition is executed just once at the related job level.

@codecov
Copy link

codecov bot commented Jun 30, 2023

Codecov Report

Patch coverage has no change and project coverage change: -33.84 ⚠️

Comparison is base (b7f65c3) 86.24% compared to head (834014d) 52.40%.

❗ Current head 834014d differs from pull request most recent head 5bb2dff. Consider uploading reports for the commit 5bb2dff to get more accurate results

Additional details and impacted files
@@             Coverage Diff              @@
##           develop    #3300       +/-   ##
============================================
- Coverage    86.24%   52.40%   -33.84%     
============================================
  Files          233      234        +1     
  Lines        12493    12642      +149     
============================================
- Hits         10774     6625     -4149     
- Misses        1719     6017     +4298     
Flag Coverage Δ
pytest ?

Flags with carried forward coverage won't be shown. Click here to find out more.

see 150 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@frascuchon frascuchon force-pushed the tests/review-test-workflow-in-CI branch from 93157ab to 42013d9 Compare June 30, 2023 16:39
@frascuchon frascuchon self-assigned this Jul 2, 2023
@frascuchon frascuchon changed the title tests: Uncomment failing tests ci: Optimize build + test pipeline Jul 2, 2023
@frascuchon frascuchon force-pushed the tests/review-test-workflow-in-CI branch 2 times, most recently from 8251f01 to cbbd8a6 Compare July 2, 2023 17:00
@frascuchon frascuchon force-pushed the tests/review-test-workflow-in-CI branch from cbbd8a6 to de5d908 Compare July 2, 2023 17:01
@frascuchon frascuchon force-pushed the tests/review-test-workflow-in-CI branch from 1440e16 to 13ed3a6 Compare July 2, 2023 17:19
@frascuchon frascuchon force-pushed the tests/review-test-workflow-in-CI branch from 79a0149 to efb6cd1 Compare July 2, 2023 17:35
@frascuchon frascuchon force-pushed the tests/review-test-workflow-in-CI branch 2 times, most recently from 504335b to 14f4501 Compare July 2, 2023 20:56
@frascuchon frascuchon marked this pull request as ready for review July 2, 2023 21:08
@frascuchon frascuchon force-pushed the tests/review-test-workflow-in-CI branch from 14f4501 to 339204f Compare July 2, 2023 21:09
Copy link
Member

@gabrielmbmb gabrielmbmb left a comment

Choose a reason for hiding this comment

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

LGTM!

.github/workflows/build-python-package.yml Outdated Show resolved Hide resolved
.github/workflows/build-python-package.yml Outdated Show resolved Hide resolved
.github/workflows/run-python-tests.yml Show resolved Hide resolved
@frascuchon frascuchon force-pushed the tests/review-test-workflow-in-CI branch 9 times, most recently from b4b2814 to 3971bb8 Compare July 3, 2023 16:48
@frascuchon frascuchon force-pushed the tests/review-test-workflow-in-CI branch from 834014d to 5bb2dff Compare July 3, 2023 17:04
@frascuchon frascuchon merged commit faed3fe into develop Jul 3, 2023
15 checks passed
@frascuchon frascuchon deleted the tests/review-test-workflow-in-CI branch July 3, 2023 17:22
alvarobartt pushed a commit that referenced this pull request Jul 4, 2023
<!-- Thanks for your contribution! As part of our Community Growers
initiative 🌱, we're donating Justdiggit bunds in your name to reforest
sub-Saharan Africa. To claim your Community Growers certificate, please
contact David Berenstein in our Slack community or fill in this form
https://tally.so/r/n9XrxK once your PR has been merged. -->

# Description

This PR reviews and organizes the CI pipeline. With the changes
introduced here, all tests are running as expected.

Also, the cache action has been updated in order to fix a cache
resolution problem which allows to reuse the cache for the rest of the
jobs.

## Two blocks of tests are launched separately 

**base tests** (most of the server tests and some basic client tests)
and **extra tests** (training, monitor, listeners...). The pipeline
falls into errors if resource all tests are launched in the same job.
In the next step, those groups should be reviewed and improved.  

Refs #3291 

## Reusable jobs extraction

The job running the tests has been extracted as a separate workflow,
which helps with reusability and workflow code maintenance. The same has
been applied to the build Python package.

## Improve conditional executions

The `IS_DEPLOYABLE` step conditions have been removed. Now, this
condition is executed just once at the related job level.
leiyre pushed a commit that referenced this pull request Jul 4, 2023
* develop:
  refactor: add `HuggingFaceDatasetMixIn` under `integrations` (#3326)
  feat: add list user workspaces endpoint (#3308)
  ci: Stop linking issues to team work project
  chore: add missing `greenlet` dependency in `server` extra (#3330)
  fix: unit test failing if not local db (#3307)
  ci: Optimize build + test pipeline (#3300)
  refactor: simplify old bulk endpoints to avoid create datasets if does not exists (#3306)
  📝 Update doc site link (#3299)
  🚑 Fix dependencies (#3302)
  feat: migrate to async SQLAlchemy engine (#3162)
leiyre pushed a commit that referenced this pull request Jul 5, 2023
* develop:
  fix: return all workspaces in system for owner users (#3343)
  fix: `rg.init` with argilla user using quickstart images raise an unexpected error (#3341)
  feat: add `Suggestion` endpoints (#3304)
  feat: add `list_user_workspaces` and `User.workspaces` property (#3334)
  refactor: add `HuggingFaceDatasetMixIn` under `integrations` (#3326)
  feat: add list user workspaces endpoint (#3308)
  ci: Stop linking issues to team work project
  chore: add missing `greenlet` dependency in `server` extra (#3330)
  docs: update developer docs (#3314)
  Docs/3312 docs 112 is not building correctly (#3313)
  fix: unit test failing if not local db (#3307)
  ci: Optimize build + test pipeline (#3300)
  refactor: simplify old bulk endpoints to avoid create datasets if does not exists (#3306)
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.

3 participants