Comments (5)
In your suggested config, the dependabot PR has passing tests
rule will be satisfied while the security checks
and tests
status checks are pending, even if they eventually will fail or time out. That is the confusing behavior.
The key name is has_successful_status
, but the behavior is more like "does not have unsuccessful status".
from policy-bot.
I think the following statement is probably where the confusion lies:
In my opinion, using policy bot is supposed to replace the need for other required status checks
When using the bot internally, we don't hold this statement as true. Policy-bot is a gate for more complex review policies, but is not the only or single status check that should be made required.
For example, we mark policy-bot, CI, and some other internal only checks as the required status checks for repo branch protection. Each repo can configure additional checks beyond just policy-bot and CI. Once all the required status checks are passing, bulldozer will merge in PRs.
This lets us decouple trying to put everything into a policy and affords a bit more flexibility in maintaining a small set of policies that can work in almost all repos. That being said, I think there is maybe some merit in adding some new functionality to policy bot around status checks but I think it goes against how we were originally holding the problem. Let us think a bit internally on a reasonable path forward.
from policy-bot.
The way we usually solve this is to directly make the tests
status a required status check in GitHub. This way, the policy only reflects approval.
If you need to combine this all in the policy for some reason (maybe you want human developers to be able to ignore failing tests if they have approval), try combining the conditions in a single rule:
policy:
approval:
- or:
- owner has approved
- depandabot PR has passing tests
disapproval:
requires:
organizations:
- "org"
approval_rules:
- name: owner has approved
requires:
count: 1
teams:
- "org/team"
- name: dependabot PR has passing tests
if:
has_author_in:
users:
- "dependabot[bot]"
- "dependabot-circleci[bot]"
has_successful_status:
- "security checks"
- "tests"
requires:
count: 0
from policy-bot.
@bluekeyes In my opinion, sing policy bot is supposed to replace the need for other required status checks. This "workaround" of making tests a required status in GitHub doesn't even work if i want conditional approval based on author.
@misund Firstly not all CI's report statuses immediately, ours in some cases will not report while CI is queued and will not report some tests until they pass or fail.
Secondly, this makes using policy-bot to set org wide standards impossible, our policy of "you must have this successful status" to be able to merge anything can just be bypassed by not reporting the status at all, fx deleting it from your CI workflow.
Another example where this behavior does not make sense (to me):
policy:
approval:
- or:
- and:
- tests passed
- owner has approved
- and:
- dependabot is making the PR
- or:
- tests passed
- owner has approved
disapproval:
requires:
organizations:
- "bestseller"
approval_rules:
- name: tests passed
if:
has_successful_status:
- "test"
- "Security Checks"
requires:
count: 0
- name: owner has approved
requires:
count: 1
teams:
- "....."
- name: dependabot is making the PR
if:
has_author_in:
users:
- "dependabot[bot]"
- "dependabot-circleci[bot]"
requires:
count: 0
In this case, the behavior we want is that if the PR is made by dependabot and the tests pass then it should not require approval and bulldozer can merge it.
In reality, the has_author_in
gets skipped if false and anyone can merge the PR without any approval, if the tests pass
from policy-bot.
I'm going to close this now that #752 is implemented and released in version 1.35.0. While the behavior of skipped checks may still be confusing, I tried to improve the documentation here and I believe there is now an alternative way (required conditions) to implement the desired behavior.
If that's not true, we can reopen this and discuss what is still missing.
from policy-bot.
Related Issues (20)
- If no rule matches can policy-bot not set a failed status on the PR? HOT 1
- Unable to run policy-bot behind a reverse-prxoy HOT 3
- `common.IsActor()` does not actually use `ctx` and can be simplified.
- Condition for not having specific label(s) HOT 6
- has_successful_status causes review requests while PR has draft status HOT 5
- Status check clarification HOT 2
- Feature Request: Predicate to skip rule if a file was changed HOT 6
- Feature Request: Option to count skipped jobs in has_successful_status HOT 5
- Clarify why users are "disqualified" when approval is ignored
- Create new production Release 🚀 HOT 1
- Connecting lines broken when hiding skipped rules with errors
- requires.conditions not working correctly in rule HOT 2
- behavior when using `invalidate_on_push` and `ignore_commits_by`? HOT 2
- Rebase invalidates approval HOT 1
- Concurrent deployments can overwrite statuses unintentionally HOT 1
- Status checks not re-run when used with merge queue HOT 2
- Paths in public_url are removed from OAuth routes
- Re-evaluate policy when PR base branch changes
- has_successful_status may use incorrect check-run HOT 2
- can I have approval rule with has_author_in but in not form HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from policy-bot.