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

CRIB-542: crib-integration-tests GH workflow running on push to develop #15221

Closed

Conversation

rafaelfelix
Copy link
Collaborator

@rafaelfelix rafaelfelix commented Nov 13, 2024

Because our crib-integration-tests runs only on schedule, the PR issuer never really gets the feedback that their change broke CRIB when/after merging to develop, and the CRIB team gets notified at most once a day if develop is broken. That + timezone differences between team mates can prolong the resolution of the problem.

This PR embeds calls the crib-integration-test.yml workflow into the already existing from build-publish-develop-pr.yml, since workflow_run doesn't provide feedback in the context of a PR/push as well (see https://stackoverflow.com/questions/63343937/how-to-use-the-github-actions-workflow-run-event#comment134992476_65081720) we'd have a check running per PR/push to develop but completely detached from the commit ref, which defeats the purpose of establishing a feedback loop.

Keeping these workflows split would incur in potential race conditions, such as running tests by pulling a docker tag that doesn't yet exist, or worse: exists but with a different content than the one from the actual commit ref.

@rafaelfelix rafaelfelix force-pushed the CRIB-542/crib-integration-tests-on-push-to-develop branch 2 times, most recently from cf49bb2 to 3502efa Compare November 13, 2024 14:40
@rafaelfelix rafaelfelix marked this pull request as ready for review November 13, 2024 15:09
@rafaelfelix rafaelfelix requested review from a team as code owners November 13, 2024 15:09
@HenryNguyen5
Copy link
Collaborator

Instead of re-adding all of the CRIB logic here, you could use a similar for to the breaking changes GQL check: https://github.com/smartcontractkit/chainlink/blob/develop/.github/workflows/operator-ui-ci.yml#L16

@rafaelfelix rafaelfelix force-pushed the CRIB-542/crib-integration-tests-on-push-to-develop branch 4 times, most recently from 37c9801 to 6786640 Compare November 14, 2024 15:48
@rafaelfelix rafaelfelix force-pushed the CRIB-542/crib-integration-tests-on-push-to-develop branch from 6786640 to cb98d95 Compare November 29, 2024 14:29
Because our crib-integration-tests runs only on schedule, the PR issuer never really gets the feedback that their change broke CRIB when/after merging to develop, and the CRIB team gets notified at most once a day if `develop` is broken. That + timezone differences between team mates can prolong the resolution of the problem.

This PR embeds the crib-integration-tests.yml workflow into the already existing build-publish-develop-pr.yml, since `workflow_run` doesn't provide feedback in the context of a PR/push as well (see https://stackoverflow.com/questions/63343937/how-to-use-the-github-actions-workflow-run-event#comment134992476_65081720) we'd have a check running per PR/push to develop but completely detached from the commit ref, which defeats the purpose of establishing a feedback loop.

Keeping these workflows splitted would incur in potential race conditions, such as running tests by pulling a docker tag that doesn't yet exist, or worse: exists but with a different content than the one from the actual commit ref.
running on every commit to every PR would increase our GH runner costs too much
@rafaelfelix rafaelfelix force-pushed the CRIB-542/crib-integration-tests-on-push-to-develop branch 5 times, most recently from 3613228 to e3c8879 Compare December 13, 2024 14:01
@rafaelfelix rafaelfelix force-pushed the CRIB-542/crib-integration-tests-on-push-to-develop branch from e3c8879 to b63e15d Compare December 13, 2024 14:21
@rafaelfelix rafaelfelix force-pushed the CRIB-542/crib-integration-tests-on-push-to-develop branch from 58b467a to 21ed52c Compare December 13, 2024 15:51
workflow_call:
inputs:
Copy link
Contributor

Choose a reason for hiding this comment

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

Please correct me if I'm wrong, but is this going to trigger the test every time we merge into the develop branch? If that's true, a quick check shows there are 10+ PRs daily, resulting in at least 300 runs per month. This could cost us a significant amount of money, I suppose.
My questions are:

  • Do we want to spend money on a broken test? It will spin up some pods every time, only to fail.
  • Does it really make sense to enable it for every push to develop, considering we can't make it a mandatory check and we don't expect people to fix CRIB?

Maybe it would make sense to completely disable it for now. Once it's fixed, we could run it on a schedule (e.g., once daily) or trigger it by adding a specific label to PRs.

@rafaelfelix
Copy link
Collaborator Author

closing it after recent discussions. we'll stick with the scheduled run for now.

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.

4 participants