BUG: CI-Tests sometimes doesn't appear in the CLI's results.

[pnacht]

Describe the bug
Sometimes (~1% of the times) CI-Tests is not included in the output from the Scorecard CLI. No exceptions are thrown, the result set simply only contains 17 checks.

Reproduction steps
This one is tough to reproduce. It happens very rarely and not always on the same repositories.

I'm using the CLI to run Scorecards on 200 projects. Usually 1-2 projects (different projects each time) simply don't have results for CI-Tests.

Each run of the CLI takes the form scorecard --repo=owner/repo --format=json. Running Scorecard on the same repositories later successfully gives a result for the check (with and without --checks=CI-Tests).

[spencerschrock]

Have you noticed duplicate Code-Review entries?
Could be related to #2686

[pnacht]

I have not, but they may have gone unnoticed given how I was parsing the results. I'll try to investigate.

[pnacht]

Just inspected a new case and yes, there's two Code-Review blocks.

        {
            "details": null,
            "score": 9,
            "reason": "27 out of last 30 changesets reviewed before merge -- score normalized to 9",
            "name": "Code-Review",
            "documentation": {
                "url": "https://github.com/ossf/scorecard/blob/376f465c111c39c6a5ad7408e8896cd790cb5219/docs/checks.md#code-review",
                "short": "Determines if the project requires code review before pull requests (aka merge requests) are merged."
            }
        },
        {
            "details": null,
            "score": -1,
            "reason": "internal error: internal error: Client.Repositories.ListStatuses: internal error: ListStatuses: GET https://api.github.com/repos/easymock/objenesis/commits/1ad0fe19f872d675d6c29ea9103122813ad7c9f1/statuses: 400  []",
            "name": "Code-Review",
            "documentation": {
                "url": "https://github.com/ossf/scorecard/blob/376f465c111c39c6a5ad7408e8896cd790cb5219/docs/checks.md#code-review",
                "short": "Determines if the project requires code review before pull requests (aka merge requests) are merged."
            }
        },

It is worth emphasizing, however, that I've been running this script of 200 repositories quite a few times now, and so far it's only ever been CI-Tests that's been "missing" from the output, while #2686 shows a case where it was Contributors instead...

[spencerschrock]

The reason and score fields look normal. The documentation field is based on the name field, so I'm guessing it's a race condition with the checkName.

I'm weary of this codeblock, even though we shadow the key and value:

scorecard/pkg/scorecard.go

Lines 48 to 60 in 23bd295

	for checkName, checkFn := range checksToRun {
	checkName := checkName
	checkFn := checkFn
	wg.Add(1)
	go func() {
	defer wg.Done()
	runner := checker.NewRunner(
	checkName,
	repo.URI(),
	&request,
	)
	resultsCh <- runner.Run(ctx, checkFn)

[spencerschrock]

I've made a small patch here that seems to have helped in my tests.
https://github.com/spencerschrock/scorecard/tree/bug/check-race
Can you build from it and test it in your next script run?

EDIT: still occurring, so I'm guessing this wasn't the problem. But it does appear to occur after Scorecard sleeps to wait for API quota

Thanks to the Chainguard Imposter Commit blog I found out this is easy to checkout in the normal ossf/scorecard repo:

git fetch origin 7f43a24e2ccc091435a841780f3b679b449106f5
git checkout FETCH_HEAD

[pnacht]

Good catch! Hadn't noticed that. Looking at my logs I can confirm that every single time this happened, it was after sleeping for the quota.

[spencerschrock]

It is worth emphasizing, however, that I've been running this script of 200 repositories quite a few times now, and so far it's only ever been CI-Tests that's been "missing" from the output, while #2686 shows a case where it was Contributors instead...

Note: #2686 was indeed the CI-Tests check missing. The issue had a copy/paste issue which accidentally included Contributors info. Checking the linked, failing workflow run showed CI-Tests missing.

[spencerschrock]

Should be fixed after merging #2756