Enable test ownership validation test in pre-submit#87310
Enable test ownership validation test in pre-submit#87310fluttergithubbot merged 3 commits intoflutter:masterfrom
Conversation
.ci.yaml
Outdated
|
|
||
| - name: Linux test_ownership | ||
| builder: Linux test_ownership | ||
| postsubmit: false |
There was a problem hiding this comment.
In general, there shouldn't be presubmit only targets. Presubmit only runs the tree of the PR, and not what the merged tree will be.
Also, if we had to mark this as flaky, we'd lose all coverage.
There was a problem hiding this comment.
Is it the plan to deprecate the postsubmit flag in the long run?
Updated to run both presubmit and postsubmit, and marked it as flaky.
Here is the recipe cl: https://flutter-review.googlesource.com/c/infra/+/16400 to add prod builders. PTAL.
There was a problem hiding this comment.
It's a valid point to remove the postsubmit field, but i'm not sure. I need more time to think about it, as there may be some need to do presubmit only \cc @christopherfujino
There was a problem hiding this comment.
i don't have a strong opinion, but it would be nice to have the option, i can imagine we might in the future have need for a pre-submit only test. off the top of my head, i can't think of an example, though.
There was a problem hiding this comment.
I thought about this more, and there is a case where you want postsubmit: false.
I'd like to add an FYI check for infra that validates ci.yaml changes won't get the tree into a bad state. If someone adds a new target without bringup: true, it would block the PR. However, they could override it (in the case of renames).
4 participants
#87144