-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
CRIB-542: crib-integration-tests GH workflow running on push to develop #15221
Conversation
cf49bb2
to
3502efa
Compare
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 |
37c9801
to
6786640
Compare
6786640
to
cb98d95
Compare
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
3613228
to
e3c8879
Compare
e3c8879
to
b63e15d
Compare
58b467a
to
21ed52c
Compare
workflow_call: | ||
inputs: |
There was a problem hiding this comment.
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.
closing it after recent discussions. we'll stick with the scheduled run for now. |
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
embedscalls the crib-integration-test.yml workflowinto the already existingfrom build-publish-develop-pr.yml, sinceworkflow_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.