-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Fixing the issue when status check per rule matches multiple actions #29631
Fixing the issue when status check per rule matches multiple actions #29631
Conversation
Related to: I guess the |
This comment was marked as off-topic.
This comment was marked as off-topic.
not sure , but I think this fix hasn't broken the expected behavior of #29143 Based on intuition, a rule can match many actions, unlike writing one rule to match one action (in which case, we only need to write the full action name). Perhaps this checking behavior can be refactored. |
I proposed some examples in #29631 (comment). I think the correct logic should be:
Sorry I think I misunderstood something. Ignore my comments .... ps: It would be helpful to add some tests to clarify the behavior. |
The problem I fixed with that condition was that the function was returning success even if a required check hasn't run at all (e.g. no status check was submitted that matches a required pattern). |
I'm not sure, but the expected behavior is that every rule needs to match at least one action? If using <, then the following case may be incorrect. actions
rules
|
Yeah I think I didn't communicate that well. The deleted To put it the other way around: Currently it is also returning The test case for the bug I fixed would look like this: actions (all successful)
rules
For that the function returned overall |
Hmm... I think perhaps one rule matching at least one action is correct. In GitHub, it seems like one rule matches just one action. But if Gitea allows wildcard characters, then it seems to allow one rule to match multiple actions. In this case, we can't simply compare the rule count and the matched action count. |
0c0ea5d
to
c2c9cd8
Compare
I've taken the time to write some tests, which I should have done beforehand, and also changed the Have a look at this commit (feel free to grab the tests into this PR): MarkusAmshove@adc885b |
c2c9cd8
to
8ab9e5b
Compare
I'm just curious about this case. I think it should pass because when allowing wildcard characters, sometimes intersections are possible. |
6d4cf88
to
583bea2
Compare
None of the contexts have a wildcard character, did you mean to add a * somewhere? |
Like this, just simulating when multiple rules match the same action. givenStatuses := createStatuses(
createSuccess("Build 1"),
createSuccess("Build 2"),
createSuccess("Build 2t"),
)
requiredContexts := createRequiredContexts(
"Build 1*",
"Build 2*",
"Build*",
"Build *",
) |
583bea2
to
16dce07
Compare
Yes, that should result in success :-) Maybe it's easier to change the logic to check each required context if it matches a check and if one doesn't, return pending. That way we don't have to count how many match |
1f4b607
to
90658f3
Compare
90658f3
to
1b0a814
Compare
…o-gitea#29631) Close go-gitea#29628 rule ``` Test / Build* Test / Build * Test / Build 2* Test / Build 1* ``` ![image](https://github.com/go-gitea/gitea/assets/30816317/19bef0a9-fa97-43c5-887b-dece76064aa8) rule2 ``` Test / Build* Test / Build 1* ``` ![image](https://github.com/go-gitea/gitea/assets/30816317/19bef0a9-fa97-43c5-887b-dece76064aa8) rule3 ``` Test / Build* Test / Build 1* NotExist* ``` ![image](https://github.com/go-gitea/gitea/assets/30816317/f6a5e832-2e1b-4049-915b-45bec5ef070c) --------- Co-authored-by: Zettat123 <[email protected]>
…29631) (#29655) Backport #29631 by @charles7668 Close #29628 rule ``` Test / Build* Test / Build * Test / Build 2* Test / Build 1* ``` ![image](https://github.com/go-gitea/gitea/assets/30816317/19bef0a9-fa97-43c5-887b-dece76064aa8) rule2 ``` Test / Build* Test / Build 1* ``` ![image](https://github.com/go-gitea/gitea/assets/30816317/19bef0a9-fa97-43c5-887b-dece76064aa8) rule3 ``` Test / Build* Test / Build 1* NotExist* ``` ![image](https://github.com/go-gitea/gitea/assets/30816317/f6a5e832-2e1b-4049-915b-45bec5ef070c) Co-authored-by: charles <[email protected]> Co-authored-by: Zettat123 <[email protected]>
* giteaofficial/main: Filter for default-branch selection (go-gitea#29388) Fixing the issue when status check per rule matches multiple actions (go-gitea#29631) Fix 500 when deleting account with incorrect password or unsupported login type (go-gitea#29579) Partially enable MSSQL case-sensitive collation support (go-gitea#29238)
Close #29628
rule
rule2
rule3