Fix edx-platform required checks #34789
Comments
|
Thank you for your report! @openedx/axim-oncall will triage within a business day. Simple requests usually take 2-3 business days to resolve; more complex requests could take longer. |
|
@kdmccormick @e0d are either of you taking this, or is this a backlog eng item? |
|
I am not, and I doubt Ed is, so it's in the backlog, although I'd vouch for it to be high up in the backlog. |
|
I'm just wondering if it's an on-call ticket or not. I don't know how to address this. |
kdmccormick
changed the title
|
Fair, I guess it's more edx-platform maintenance than on-call work. @feanil and I are next up in the on-call queue, so I figure whichever one of us has time first can pick it up. |
|
@feanil LMK here when you switch us over to github hosted unit test runners. I think it should make these required checks much easier to manage. I'm reeealllllyyyy looking forward to being able to auto-merge edx-platform PRs again 😄 |
This ensures that the `success` job in unit-tests.yml always runs, allowing us to protect master from unit test failures by making the `success` job required. From https://github.com/marketplace/actions/alls-green#options: > Important: For this to work properly, it is a must to have the job always run, > otherwise GitHub will make it skipped when any of the dependencies fail. In > some contexts, skipped is interpreted as success which may lead to undersired, > unobvious and even dangerous (as in security breach "dangerous") side-effects. Closes #34789
When a shard of unit-tests.yml fails, we want the `success` job to be maked "Failed" (not "Skipped"). That's because "Failed" blocks the PR from merging, whereas "Skipped" does not. This change ensures that `success` always runs to completion rather than being cancelled as soon as a unit test shard fails or is cancelled. From https://github.com/marketplace/actions/alls-green#options: > Important: For this to work properly, it is a must to have the job always run, > otherwise GitHub will make it skipped when any of the dependencies fail. In > some contexts, skipped is interpreted as success which may lead to undersired, > unobvious and even dangerous (as in security breach "dangerous") side-effects. Closes #34789
When a shard of unit-tests.yml fails, we want the `success` job to be maked "Failed" (not "Skipped"). That's because "Failed" blocks the PR from merging, whereas "Skipped" does not. This change ensures that `success` always runs to completion rather than being cancelled as soon as a unit test shard fails or is cancelled. From https://github.com/marketplace/actions/alls-green#options: > Important: For this to work properly, it is a must to have the job always run, > otherwise GitHub will make it skipped when any of the dependencies fail. In > some contexts, skipped is interpreted as success which may lead to undersired, > unobvious and even dangerous (as in security breach "dangerous") side-effects. Closes #34789
|
Remaining tasks:
|
|
Pylint is already Several weeks have gone by, so I think we're safe to re-enable auto-merge. |
Repository
axim-engineering
Urgency
Low (2 weeks)
Requested Change
The set of required checks on edx-platform and/or the "unit tests successful" check itself need to be fixed, although we're not exactly sure how yet.
They don't currently enforce that all unit tests pass, as the Unit tests successful check seems to be getting skipped.
More details:
Tests successfulchecks? #32238We have disabled Auto-merge on edx-platform because of this. As part of resolving this ticket, please re-enable it.
Reasoning
.
The text was updated successfully, but these errors were encountered: