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

Reduce double nightly tests #1911

Merged
merged 4 commits into from
Apr 11, 2023
Merged

Reduce double nightly tests #1911

merged 4 commits into from
Apr 11, 2023

Conversation

miguelgfierro
Copy link
Collaborator

Description

I noticed that when there is a merge to staging and a PR to main, we are doing twice the nightly tests. They are the exact same tests so there is no need to have it twice.

image

By removing the trigger when there is a PR to main, we are still checking that the nightly have passed, because all new code that is merged to staging passes the nightly.

In addition, every 5 days we are scheduling the nightly in main

Related Issues

References

Checklist:

  • I have followed the contribution guidelines and code style for this project.
  • I have added tests covering my contributions.
  • I have updated the documentation accordingly.
  • This PR is being made to staging branch and not to main branch.

@wutaomsft
Copy link
Collaborator

wutaomsft commented Mar 31, 2023

@miguelgfierro the picture above - do you mean multiple PRs are now triggering multiple nightly tests while the correct behavior is only one nightly test should be run for all the PRs of that day?

@miguelgfierro
Copy link
Collaborator Author

When Pradnyesh did the solution, he found that there is an issue with GitHub that it only allows to schedule cron jobs in main https://github.com/microsoft/recommenders/blob/main/.github/workflows/azureml-cpu-nightly.yml#L18
So instead of doing a schedule job in staging, we run the nightlys when we merge into staging.

The code we had before executed nightly when there was a PR to main (in staging branch). So we are repeating the same tests with the same code twice every time we merge to staging and there is a PR to main.

This PR removes the nightly in the PR to main, because it is actually repeating the same test twice.

If you notice, in the screenshot, the 3 nightlys are executed at the same time.

Also in the figure you'll notice that when we do a PR to main we are executing the unit tests. This makes sense and are not repeated.

Let me know if you need additional details

@wutaomsft wutaomsft merged commit d2fe2eb into staging Apr 11, 2023
@miguelgfierro miguelgfierro deleted the reduce_double_nightly branch April 11, 2023 12:52
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