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.

Sign up for GitHub

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

Jump to bottom

chore: only run tests that changed in regular CI if nothing else did #247

Merged
merged 1 commit into from
Sep 18, 2023
Merged

chore: only run tests that changed in regular CI if nothing else did #247

merged 1 commit into from
Sep 18, 2023

Conversation

RomanBredehoft
Copy link
Collaborator

@RomanBredehoft RomanBredehoft commented Sep 14, 2023 •
edited
Loading

Basically :

  • improve regular tests so that if a PR only changes specific test files, we only run these tests (as they are all independent). We still run all tests if pytest utils or tests data has been changed
  • fix a problem where regular pytest step would not be triggered if the CI has been manually triggered
  • make comments for skippable tests in CI more clear

closes https://github.com/zama-ai/concrete-ml-internal/issues/3893

@cla-bot cla-bot bot added the cla-signed label Sep 14, 2023
@RomanBredehoft RomanBredehoft force-pushed the chore/run_tests_that_changed_if_nothing_else_did_3893 branch from 1a81c67 to 8873b18 Compare September 14, 2023 14:40
@github-actions
Copy link

Coverage passed ✅

Coverage details 

---------- coverage: platform linux, python 3.8.18-final-0 -----------
Name    Stmts   Miss  Cover   Missing
-------------------------------------
TOTAL    5904      0   100%

50 files skipped due to complete coverage.

@RomanBredehoft
Copy link
Collaborator Author

the CI was tested in other PRs and everything went well

@RomanBredehoft RomanBredehoft marked this pull request as ready for review September 14, 2023 15:56
@RomanBredehoft RomanBredehoft requested a review from a team as a code owner September 14, 2023 15:56
RomanBredehoft
RomanBredehoft commented Sep 14, 2023
View reviewed changes
.github/workflows/continuous-integration.yaml
|| steps.changed-files-in-pr.outputs.makefile_any_changed == 'true'
)
)
|| fromJSON(env.IS_WORKFLOW_DISPATCH)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

regular tests should be run if CI is manually triggered

Copy link
Collaborator

Choose a reason for hiding this comment

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

What do you mean by manually triggered?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

through GitHub's Action interface

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

meaning we manually trigger the workflow (it is not triggered automatically when opening a PR or during a weekly release for example)

RomanBredehoft
RomanBredehoft commented Sep 14, 2023
View reviewed changes
.github/workflows/continuous-integration.yaml
# modified tests
# Note that if pytest utils or test data are changed, the regular pytest step should have been
# triggered instead
- name: PyTest on modified tests only
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

main feature

jfrery
jfrery reviewed Sep 15, 2023
View reviewed changes
Copy link
Collaborator

@jfrery jfrery left a comment

Choose a reason for hiding this comment

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

For my understanding, here we only rerun tests that have been modified unless something from src has been modified, right?

@RomanBredehoft
Copy link
Collaborator Author

For my understanding, here we only rerun tests that have been modified unless something from src has been modified, right?

yes exactly, more precisely, we rerun tests that have been modified instead of all tests unless one of the following it true :
- the current workflow does no take place in a weekly or release CI
- if the CI has been triggered manually (through GitHub's Action interface)
- the source code has been changed
- any tests utils (pytest, data) has been changed
- any dependency has been updated
- conftest.py has been changed
- Makefile has been changed

which does sometimes happen when we fix some tests or refactor

jfrery
jfrery approved these changes Sep 18, 2023
View reviewed changes
Copy link
Collaborator

@jfrery jfrery left a comment

Choose a reason for hiding this comment

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

Thanks for the answer! I like the idea let's go.

@RomanBredehoft RomanBredehoft merged commit c1899e5 into main Sep 18, 2023
@RomanBredehoft RomanBredehoft deleted the chore/run_tests_that_changed_if_nothing_else_did_3893 branch September 18, 2023 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fd0r fd0r fd0r approved these changes

@jfrery jfrery jfrery approved these changes

Assignees
No one assigned
Labels
cla-signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@RomanBredehoft @jfrery @fd0r