Skip to content
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

Feature: accept neutral conclusion in CI-Test #1238

Closed
laurentsimon opened this issue Nov 10, 2021 · 5 comments
Closed

Feature: accept neutral conclusion in CI-Test #1238

laurentsimon opened this issue Nov 10, 2021 · 5 comments
Labels
kind/enhancement New feature or request
Milestone

Comments

@laurentsimon
Copy link
Contributor

laurentsimon commented Nov 10, 2021

We may be missing check status in the CI-Test. See this change that was made for the SAST check #1237 (comment). I think we should make the same change in CI-Test check.

@evverx wdut?

@laurentsimon laurentsimon added the kind/enhancement New feature or request label Nov 10, 2021
@evverx
Copy link
Contributor

evverx commented Nov 10, 2021

I think it would make sense to include "neutral" checks there. Though I think the "skipped" conclusion is much more likely to appear (at least when custom GHActions like https://github.com/systemd/systemd/blob/main/.github/workflows/cifuzz.yml are skipped conditionally). Anyway, to be absolutely certain I think I need to take a look at what the check does and try to figure out where it lost another commit checked by CI:

./scorecard --show-details  --checks=CI-Tests --repo=https://github.com/systemd/systemd
Starting [CI-Tests]
Finished [CI-Tests]

RESULTS
-------
Aggregate score: 9.0 / 10

Check scores:
|--------|----------|--------------------------------|---------|---------------------------------------------------------------------------------------------------------|
| SCORE  |   NAME   |             REASON             | DETAILS |                                        DOCUMENTATION/REMEDIATION                                        |
|--------|----------|--------------------------------|---------|---------------------------------------------------------------------------------------------------------|
| 9 / 10 | CI-Tests | 29 out of 30 merged PRs        |         | https://github.com/ossf/scorecard/blob/795505fd7f1989ed2620149e204c32d5cb1b43f7/docs/checks.md#ci-tests |
|        |          | checked by a CI test -- score  |         |                                                                                                         |
|        |          | normalized to 9                |         |                                                                                                         |
|--------|----------|--------------------------------|---------|---------------------------------------------------------------------------------------------------------|

I'm pretty sure all the commits are tested there one way or another

@evverx
Copy link
Contributor

evverx commented Nov 10, 2021

I need to take a look at what the check does and try to figure out where it lost another commit checked by CI

Judging by

Debug: merged PR without CI test: 21261 Debug: CI test found:

Looks like systemd/systemd#21261 was excluded. I don't think it has anything to do with the check conclusions though.

@laurentsimon
Copy link
Contributor Author

So do you think it's safe to add the neutral?
Do you know what skipped conclusion means? How is it different from neutral? Does GitHub figure some checks don't need to be re-run if they passed and the code they act upon has not changed between commits?

@evverx
Copy link
Contributor

evverx commented Nov 18, 2021

I haven't been able to find any other app apart from LGTM using the "neutral" status so I think it would probably make sense to wait for bug reports with examples of specific CI services that can be "neutral" from time to time.

"skipped" usually appear when some conditions specified in GH Actions aren't met. For example, in https://github.com/evverx/systemd/runs/4249276023?check_suite_focus=true CIFuzz was skipped because it's just a fork of the systemd repository and if: github.repository == 'systemd/systemd' is false there. It's hard to tell why exactly checks are skipped automatically so probably it's safe to say if a check is skipped it isn't run.

I'd probably close this issue.

@laurentsimon
Copy link
Contributor Author

Thank for looking into this!
Closing as you suggested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants