diff --git a/checks/ci_tests.go b/checks/ci_tests.go index c2e577385afe..b5d374057106 100644 --- a/checks/ci_tests.go +++ b/checks/ci_tests.go @@ -19,9 +19,10 @@ import ( "github.com/ossf/scorecard/v4/checks/evaluation" "github.com/ossf/scorecard/v4/checks/raw" sce "github.com/ossf/scorecard/v4/errors" + "github.com/ossf/scorecard/v4/probes" + "github.com/ossf/scorecard/v4/probes/zrunner" ) -// CheckCodeReview is the registered name for DoesCodeReview. const CheckCITests = "CI-Tests" //nolint:gochecknoinits @@ -35,7 +36,6 @@ func init() { } } -// CodeReview will check if the maintainers perform code review. func CITests(c *checker.CheckRequest) checker.CheckResult { rawData, err := raw.CITests(c.RepoClient) if err != nil { @@ -43,11 +43,15 @@ func CITests(c *checker.CheckRequest) checker.CheckResult { return checker.CreateRuntimeErrorResult(CheckCITests, e) } - // Return raw results. - if c.RawResults != nil { - c.RawResults.CITestResults = rawData + pRawResults := getRawResults(c) + pRawResults.CITestResults = rawData + + // Evaluate the probes. + findings, err := zrunner.Run(pRawResults, probes.CITests) + if err != nil { + e := sce.WithMessage(sce.ErrScorecardInternal, err.Error()) + return checker.CreateRuntimeErrorResult(CheckCITests, e) } - // Return the score evaluation. - return evaluation.CITests(CheckCITests, &rawData, c.Dlogger) + return evaluation.CITests(CheckCITests, findings, c.Dlogger) } diff --git a/checks/evaluation/ci_tests.go b/checks/evaluation/ci_tests.go index 8dc9311abe61..5a27275ba03c 100644 --- a/checks/evaluation/ci_tests.go +++ b/checks/evaluation/ci_tests.go @@ -16,122 +16,52 @@ package evaluation import ( "fmt" - "strings" "github.com/ossf/scorecard/v4/checker" + sce "github.com/ossf/scorecard/v4/errors" "github.com/ossf/scorecard/v4/finding" + "github.com/ossf/scorecard/v4/probes/testsRunInCI" ) -const ( - // CheckCITests is the registered name for CITests. - CheckCITests = "CI-Tests" - success = "success" -) - -func CITests(_ string, c *checker.CITestData, dl checker.DetailLogger) checker.CheckResult { - totalMerged := 0 - totalTested := 0 - for i := range c.CIInfo { - r := c.CIInfo[i] - totalMerged++ - - var foundCI bool - - // GitHub Statuses. - prSuccessStatus, err := prHasSuccessStatus(r, dl) - if err != nil { - return checker.CreateRuntimeErrorResult(CheckCITests, err) - } - if prSuccessStatus { - totalTested++ - foundCI = true - continue - } - - // GitHub Check Runs. - prCheckSuccessful, err := prHasSuccessfulCheck(r, dl) - if err != nil { - return checker.CreateRuntimeErrorResult(CheckCITests, err) - } - if prCheckSuccessful { - totalTested++ - foundCI = true - } +const CheckCITests = "CI-Tests" - if !foundCI { - // Log message says commit, but really we only care about PRs, and - // use only one commit (branch HEAD) to refer to all commits in a PR - - dl.Debug(&checker.LogMessage{ - Text: fmt.Sprintf("merged PR %d without CI test at HEAD: %s", r.PullRequestNumber, r.HeadSHA), - }) - } +func CITests(name string, + findings []finding.Finding, + dl checker.DetailLogger, +) checker.CheckResult { + expectedProbes := []string{ + testsRunInCI.Probe, + } + if !finding.UniqueProbesEqual(findings, expectedProbes) { + e := sce.WithMessage(sce.ErrScorecardInternal, "invalid probe results") + return checker.CreateRuntimeErrorResult(name, e) } - if totalMerged == 0 { + // check whether there are any negative findings + if noPullRequestsFound(findings) { return checker.CreateInconclusiveResult(CheckCITests, "no pull request found") } - reason := fmt.Sprintf("%d out of %d merged PRs checked by a CI test", totalTested, totalMerged) - return checker.CreateProportionalScoreResult(CheckCITests, reason, totalTested, totalMerged) -} + totalMerged := findings[0].Values["totalMerged"] + totalTested := findings[0].Values["totalTested"] -// PR has a status marked 'success' and a CI-related context. -// -//nolint:unparam -func prHasSuccessStatus(r checker.RevisionCIInfo, dl checker.DetailLogger) (bool, error) { - for _, status := range r.Statuses { - if status.State != success { - continue - } - if isTest(status.Context) || isTest(status.TargetURL) { - dl.Debug(&checker.LogMessage{ - Path: status.URL, - Type: finding.FileTypeURL, - Text: fmt.Sprintf("CI test found: pr: %s, context: %s", r.HeadSHA, - status.Context), - }) - return true, nil - } + if totalMerged < totalTested || len(findings) != totalTested { + e := sce.WithMessage(sce.ErrScorecardInternal, "invalid probe results") + return checker.CreateRuntimeErrorResult(name, e) } - return false, nil -} -// PR has a successful CI-related check. -// -//nolint:unparam -func prHasSuccessfulCheck(r checker.RevisionCIInfo, dl checker.DetailLogger) (bool, error) { - for _, cr := range r.CheckRuns { - if cr.Status != "completed" { - continue - } - if cr.Conclusion != success { - continue - } - if isTest(cr.App.Slug) { - dl.Debug(&checker.LogMessage{ - Path: cr.URL, - Type: finding.FileTypeURL, - Text: fmt.Sprintf("CI test found: pr: %d, context: %s", r.PullRequestNumber, - cr.App.Slug), - }) - return true, nil - } - } - return false, nil -} + // Log all findings + checker.LogFindings(nonNegativeFindings(findings), dl) -// isTest returns true if the given string is a CI test. -func isTest(s string) bool { - l := strings.ToLower(s) + // All findings (should) have "totalMerged" and "totalTested" + reason := fmt.Sprintf("%d out of %d merged PRs checked by a CI test", totalTested, totalMerged) + return checker.CreateProportionalScoreResult(CheckCITests, reason, totalTested, totalMerged) +} - // Add more patterns here! - for _, pattern := range []string{ - "appveyor", "buildkite", "circleci", "e2e", "github-actions", "jenkins", - "mergeable", "packit-as-a-service", "semaphoreci", "test", "travis-ci", - "flutter-dashboard", "Cirrus CI", "azure-pipelines", - } { - if strings.Contains(l, pattern) { +func noPullRequestsFound(findings []finding.Finding) bool { + for i := range findings { + f := &findings[i] + if f.Outcome == finding.OutcomeNotApplicable { return true } } diff --git a/checks/evaluation/ci_tests_test.go b/checks/evaluation/ci_tests_test.go index 19e20d3668c3..604a44679d12 100644 --- a/checks/evaluation/ci_tests_test.go +++ b/checks/evaluation/ci_tests_test.go @@ -16,441 +16,124 @@ package evaluation import ( "testing" - "github.com/ossf/scorecard/v4/checker" - "github.com/ossf/scorecard/v4/clients" + sce "github.com/ossf/scorecard/v4/errors" + "github.com/ossf/scorecard/v4/finding" scut "github.com/ossf/scorecard/v4/utests" ) -func Test_isTest(t *testing.T) { - t.Parallel() - type args struct { - s string - } - tests := []struct { - name string - args args - want bool - }{ - { - name: "appveyor", - args: args{ - s: "appveyor", - }, - want: true, - }, - { - name: "circleci", - args: args{ - s: "circleci", - }, - want: true, - }, - { - name: "jenkins", - args: args{ - s: "jenkins", - }, - want: true, - }, - { - name: "e2e", - args: args{ - s: "e2e", - }, - want: true, - }, - { - name: "github-actions", - args: args{ - s: "github-actions", - }, - want: true, - }, - { - name: "mergeable", - args: args{ - s: "mergeable", - }, - want: true, - }, - { - name: "packit-as-a-service", - args: args{ - s: "packit-as-a-service", - }, - want: true, - }, - { - name: "semaphoreci", - args: args{ - s: "semaphoreci", - }, - want: true, - }, - { - name: "test", - args: args{ - s: "test", - }, - want: true, - }, - { - name: "travis-ci", - args: args{ - s: "travis-ci", - }, - want: true, - }, - { - name: "azure-pipelines", - args: args{ - s: "azure-pipelines", - }, - want: true, - }, - { - name: "non-existing", - args: args{ - s: "non-existing", - }, - want: false, - }, - } - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - if got := isTest(tt.args.s); got != tt.want { - t.Errorf("isTest() = %v, want %v for test %v", got, tt.want, tt.name) - } - }) - } -} - -func Test_prHasSuccessfulCheck(t *testing.T) { +// Tip: If you add new findings to this test, else +// add a unit test to the probes with the same findings. +func TestCITests(t *testing.T) { t.Parallel() - + //nolint tests := []struct { - name string - args checker.RevisionCIInfo - want bool - wantErr bool + name string + findings []finding.Finding + result scut.TestReturn }{ { - name: "check run with conclusion success", - args: checker.RevisionCIInfo{ - PullRequestNumber: 1, - HeadSHA: "sha", - CheckRuns: []clients.CheckRun{ - { - App: clients.CheckRunApp{Slug: "test"}, - Conclusion: "success", - URL: "url", - Status: "completed", - }, + name: "Has CI tests. 1 tested out of 1 merged", + findings: []finding.Finding { + { + Outcome: finding.OutcomePositive, + Probe: "testsRunInCI", + Message: "CI test found: pr: 1, context: e2e", + Values: map[string]int{"totalMerged": 1, "totalTested": 1}, + Location: &finding.Location{Type: 4}, }, }, - want: true, - wantErr: false, - }, - { - name: "check run with conclusion not success", - args: checker.RevisionCIInfo{ - PullRequestNumber: 1, - HeadSHA: "sha", - CheckRuns: []clients.CheckRun{ - { - App: clients.CheckRunApp{Slug: "test"}, - Conclusion: "failed", - URL: "url", - Status: "completed", - }, - }, + result: scut.TestReturn{ + Score: 10, + NumberOfInfo: 1, }, - want: false, - wantErr: false, }, { - name: "check run with conclusion not success", - args: checker.RevisionCIInfo{ - PullRequestNumber: 1, - HeadSHA: "sha", - CheckRuns: []clients.CheckRun{ - { - App: clients.CheckRunApp{Slug: "test"}, - Conclusion: "success", - URL: "url", - Status: "notcompleted", - }, + name: "Has CI tests. 3 tested out of 4 merged", + findings: []finding.Finding { + { + Outcome: finding.OutcomePositive, + Probe: "testsRunInCI", + Message: "CI test found: pr: 1, context: e2e", + Values: map[string]int{"totalMerged": 4, "totalTested": 3}, + Location: &finding.Location{Type: 4}, }, - }, - want: false, - wantErr: false, - }, - } - for _, tt := range tests { - tt := tt - dl := &scut.TestDetailLogger{} - - got, err := prHasSuccessfulCheck(tt.args, dl) - if err != nil { - t.Fatalf("unexpected err: %v", err) - } - if got != tt.want { - t.Errorf("prHasSuccessfulCheck() = %v, want %v", got, tt.want) - } - } -} - -func Test_prHasSuccessStatus(t *testing.T) { - t.Parallel() - type args struct { //nolint:govet - r checker.RevisionCIInfo - dl checker.DetailLogger - } - tests := []struct { //nolint:govet - name string - args args - want bool - wantErr bool - }{ - { - name: "empty revision", - args: args{ - r: checker.RevisionCIInfo{}, - }, - want: false, - wantErr: false, - }, - { - name: "no statuses", - args: args{ - r: checker.RevisionCIInfo{ - Statuses: []clients.Status{}, + { + Outcome: finding.OutcomePositive, + Probe: "testsRunInCI", + Message: "CI test found: pr: 1, context: e2e", + Values: map[string]int{"totalMerged": 4, "totalTested": 3}, + Location: &finding.Location{Type: 4}, }, - }, - }, - { - name: "status is not success", - args: args{ - r: checker.RevisionCIInfo{ - Statuses: []clients.Status{ - { - State: "failure", - }, - }, - }, - }, - }, - { - name: "status is success", - args: args{ - r: checker.RevisionCIInfo{ - Statuses: []clients.Status{ - { - State: "success", - Context: CheckCITests, - }, - }, + { + Outcome: finding.OutcomePositive, + Probe: "testsRunInCI", + Message: "CI test found: pr: 1, context: e2e", + Values: map[string]int{"totalMerged": 4, "totalTested": 3}, + Location: &finding.Location{Type: 4}, }, - dl: &scut.TestDetailLogger{}, - }, - want: true, - wantErr: false, - }, - } - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - got, err := prHasSuccessStatus(tt.args.r, tt.args.dl) - if (err != nil) != tt.wantErr { - t.Errorf("prHasSuccessStatus() error = %v, wantErr %v", err, tt.wantErr) - return - } - if got != tt.want { - t.Errorf("prHasSuccessStatus() got = %v, want %v", got, tt.want) - } - }) - } -} - -func Test_prHasSuccessfulCheckAdditional(t *testing.T) { - t.Parallel() - type args struct { //nolint:govet - r checker.RevisionCIInfo - dl checker.DetailLogger - } - tests := []struct { //nolint:govet - name string - args args - want bool - wantErr bool - }{ - { - name: "empty revision", - args: args{ - r: checker.RevisionCIInfo{}, }, - want: false, - wantErr: false, - }, - { - name: "status is not completed", - args: args{ - r: checker.RevisionCIInfo{ - CheckRuns: []clients.CheckRun{ - { - Status: "notcompleted", - }, - }, - }, + result: scut.TestReturn{ + Score: 7, + NumberOfInfo: 3, }, }, { - name: "status is not success", - args: args{ - r: checker.RevisionCIInfo{ - CheckRuns: []clients.CheckRun{ - { - Status: "completed", - Conclusion: "failure", - }, - }, + name: "More tested than there are findings = error", + findings: []finding.Finding { + { + Outcome: finding.OutcomePositive, + Probe: "testsRunInCI", + Message: "CI test found: pr: 1, context: e2e", + Values: map[string]int{"totalMerged": 2, "totalTested": 2}, + Location: &finding.Location{Type: 4}, }, }, - }, - { - name: "conclusion is success", - args: args{ - r: checker.RevisionCIInfo{ - CheckRuns: []clients.CheckRun{ - { - Status: "completed", - Conclusion: "success", - }, - }, - }, + result: scut.TestReturn{ + Error: sce.ErrScorecardInternal, + NumberOfInfo: 0, + Score: -1, }, }, { - name: "conclusion is succesls with a valid app slug", - args: args{ - r: checker.RevisionCIInfo{ - CheckRuns: []clients.CheckRun{ - { - Status: "completed", - Conclusion: "success", - App: clients.CheckRunApp{Slug: "e2e"}, - }, - }, + name: "More tested than merged = error", + findings: []finding.Finding { + { + Outcome: finding.OutcomePositive, + Probe: "testsRunInCI", + Message: "CI test found: pr: 1, context: e2e", + Values: map[string]int{"totalMerged": 2, "totalTested": 3}, + Location: &finding.Location{Type: 4}, }, - dl: &scut.TestDetailLogger{}, - }, - want: true, - wantErr: false, - }, - } - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - got, err := prHasSuccessfulCheck(tt.args.r, tt.args.dl) - if (err != nil) != tt.wantErr { - t.Errorf("prHasSuccessfulCheck() error = %v, wantErr %v", err, tt.wantErr) - return - } - if got != tt.want { - t.Errorf("prHasSuccessfulCheck() got = %v, want %v", got, tt.want) - } - }) - } -} - -func TestCITests(t *testing.T) { - t.Parallel() - type args struct { //nolint:govet - in0 string - c *checker.CITestData - dl checker.DetailLogger - } - tests := []struct { //nolint:govet - name string - args args - want int - }{ - { - name: "Status completed with failure", - args: args{ - in0: "", - c: &checker.CITestData{ - CIInfo: []checker.RevisionCIInfo{ - { - CheckRuns: []clients.CheckRun{ - { - Status: "completed", - App: clients.CheckRunApp{Slug: "e2e"}, - }, - }, - Statuses: []clients.Status{ - { - State: "failure", - Context: CheckCITests, - TargetURL: "e2e", - }, - }, - }, - }, + { + Outcome: finding.OutcomePositive, + Probe: "testsRunInCI", + Message: "CI test found: pr: 1, context: e2e", + Values: map[string]int{"totalMerged": 2, "totalTested": 3}, + Location: &finding.Location{Type: 4}, }, - dl: &scut.TestDetailLogger{}, - }, - want: 0, - }, - { - name: "valid", - args: args{ - in0: "", - c: &checker.CITestData{ - CIInfo: []checker.RevisionCIInfo{ - { - CheckRuns: []clients.CheckRun{ - { - Status: "completed", - Conclusion: "success", - App: clients.CheckRunApp{Slug: "e2e"}, - }, - }, - Statuses: []clients.Status{ - { - State: "success", - Context: CheckCITests, - TargetURL: "e2e", - }, - }, - }, - }, + { + Outcome: finding.OutcomePositive, + Probe: "testsRunInCI", + Message: "CI test found: pr: 1, context: e2e", + Values: map[string]int{"totalMerged": 2, "totalTested": 3}, + Location: &finding.Location{Type: 4}, }, - dl: &scut.TestDetailLogger{}, }, - want: 10, - }, - { - name: "no ci info", - args: args{ - in0: "", - c: &checker.CITestData{}, - dl: &scut.TestDetailLogger{}, + result: scut.TestReturn{ + Error: sce.ErrScorecardInternal, + NumberOfInfo: 0, + Score: -1, }, - want: -1, }, } - for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - if got := CITests(tt.args.in0, tt.args.c, tt.args.dl); got.Score != tt.want { - t.Errorf("CITests() = %v, want %v", got.Score, tt.want) + dl := scut.TestDetailLogger{} + got := CITests(tt.name, tt.findings, &dl) + if !scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl) { + t.Errorf("got %v, expected %v", got, tt.result) } }) } diff --git a/probes/entries.go b/probes/entries.go index b5600ebe7e2e..30b26c3fb015 100644 --- a/probes/entries.go +++ b/probes/entries.go @@ -49,6 +49,7 @@ import ( "github.com/ossf/scorecard/v4/probes/securityPolicyContainsText" "github.com/ossf/scorecard/v4/probes/securityPolicyContainsVulnerabilityDisclosure" "github.com/ossf/scorecard/v4/probes/securityPolicyPresent" + "github.com/ossf/scorecard/v4/probes/testsRunInCI" "github.com/ossf/scorecard/v4/probes/toolDependabotInstalled" "github.com/ossf/scorecard/v4/probes/toolPyUpInstalled" "github.com/ossf/scorecard/v4/probes/toolRenovateInstalled" @@ -121,6 +122,9 @@ var ( CIIBestPractices = []ProbeImpl{ hasOpenSSFBadge.Run, } + CITests = []ProbeImpl{ + testsRunInCI.Run, + } ) //nolint:gochecknoinits diff --git a/probes/testsRunInCI/def.yml b/probes/testsRunInCI/def.yml new file mode 100644 index 000000000000..f2b5f4f3a255 --- /dev/null +++ b/probes/testsRunInCI/def.yml @@ -0,0 +1,28 @@ +# Copyright 2023 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +id: testsRunInCI +short: Checks that the project runs tests in the CI for example with Github Actions or Prow. +motivation: > + Running tests helps developers catch mistakes early on, which can reduce the number of vulnerabilities that find their way into a project. +implementation: > + The probe checks for tests in the projects CI jobs in the recent commits (~30). +outcome: + - The probe returns one OutcomePositive for each successful test or check that ran in the projects CI. All findings include the number of merged pull requests as well as how many of these pull requests were tested by tests in the CI. These fields are called "totalMerged" and "totalTested". + - The probe returns a single OutcomeNotApplicable if the projects has had no pull requests. +remediation: + effort: Medium + text: + - Check-in scripts that run all the tests in your repository. + - Integrate those scripts with a CI/CD platform that runs it on every pull request (e.g. if hosted on GitHub, [GitHub Actions](https://docs.github.com/en/actions/learn-github-actions/introduction-to-github-actions), [Prow](https://github.com/kubernetes/test-infra/tree/master/prow), etc). \ No newline at end of file diff --git a/probes/testsRunInCI/impl.go b/probes/testsRunInCI/impl.go new file mode 100644 index 000000000000..9e10c0b8c5f7 --- /dev/null +++ b/probes/testsRunInCI/impl.go @@ -0,0 +1,173 @@ +// Copyright 2023 OpenSSF Scorecard Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// nolint:stylecheck +package testsRunInCI + +import ( + "embed" + "fmt" + "strings" + + "github.com/ossf/scorecard/v4/checker" + "github.com/ossf/scorecard/v4/finding" + "github.com/ossf/scorecard/v4/probes/internal/utils/uerror" +) + +//go:embed *.yml +var fs embed.FS + +const ( + Probe = "testsRunInCI" + success = "success" +) + +func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { + if raw == nil { + return nil, "", fmt.Errorf("%w: raw", uerror.ErrNil) + } + + var findings []finding.Finding + + c := raw.CITestResults + + if len(c.CIInfo) == 0 { + f, err := finding.NewWith(fs, Probe, + "no pull requests found", nil, + finding.OutcomeNotApplicable) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + findings = append(findings, *f) + return findings, Probe, nil + } + + totalMerged := 0 + totalTested := 0 + for i := range c.CIInfo { + r := c.CIInfo[i] + totalMerged++ + + // GitHub Statuses. + prSuccessStatus, f, err := prHasSuccessStatus(r) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + if prSuccessStatus { + findings = append(findings, *f) + totalTested++ + continue + } + + // GitHub Check Runs. + prCheckSuccessful, f, err := prHasSuccessfulCheck(r) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + if prCheckSuccessful { + findings = append(findings, *f) + totalTested++ + } + } + + for i := range findings { + f := &findings[i] + f.WithValues(map[string]int{ + "totalMerged": totalMerged, + "totalTested": totalTested, + }) + } + + return findings, Probe, nil +} + +// PR has a status marked 'success' and a CI-related context. +// +//nolint:unparam +func prHasSuccessStatus(r checker.RevisionCIInfo) (bool, *finding.Finding, error) { + for _, status := range r.Statuses { + if status.State != success { + continue + } + if isTest(status.Context) || isTest(status.TargetURL) { + msg := fmt.Sprintf("CI test found: pr: %s, context: %s", r.HeadSHA, + status.Context) + + f, err := finding.NewWith(fs, Probe, + msg, nil, + finding.OutcomePositive) + if err != nil { + return false, nil, fmt.Errorf("create finding: %w", err) + } + + loc := &finding.Location{ + Path: status.URL, + Type: finding.FileTypeURL, + } + f = f.WithLocation(loc) + return true, f, nil + } + } + return false, nil, nil +} + +// PR has a successful CI-related check. +// +//nolint:unparam +func prHasSuccessfulCheck(r checker.RevisionCIInfo) (bool, *finding.Finding, error) { + for _, cr := range r.CheckRuns { + if cr.Status != "completed" { + continue + } + if cr.Conclusion != success { + continue + } + if isTest(cr.App.Slug) { + msg := fmt.Sprintf("CI test found: pr: %d, context: %s", r.PullRequestNumber, + cr.App.Slug) + + f, err := finding.NewWith(fs, Probe, + msg, nil, + finding.OutcomePositive) + if err != nil { + return false, nil, fmt.Errorf("create finding: %w", err) + } + + loc := &finding.Location{ + Path: cr.URL, + Type: finding.FileTypeURL, + } + f = f.WithLocation(loc) + return true, f, nil + } + } + return false, nil, nil +} + +// isTest returns true if the given string is a CI test. +func isTest(s string) bool { + l := strings.ToLower(s) + + // Add more patterns here! + for _, pattern := range []string{ + "appveyor", "buildkite", "circleci", "e2e", "github-actions", "jenkins", + "mergeable", "packit-as-a-service", "semaphoreci", "test", "travis-ci", + "flutter-dashboard", "Cirrus CI", "azure-pipelines", + } { + if strings.Contains(l, pattern) { + return true + } + } + return false +} diff --git a/probes/testsRunInCI/impl_test.go b/probes/testsRunInCI/impl_test.go new file mode 100644 index 000000000000..d09bafae2789 --- /dev/null +++ b/probes/testsRunInCI/impl_test.go @@ -0,0 +1,490 @@ +// Copyright 2023 OpenSSF Scorecard Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// nolint:stylecheck +package testsRunInCI + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + + "github.com/ossf/scorecard/v4/checker" + "github.com/ossf/scorecard/v4/clients" + "github.com/ossf/scorecard/v4/finding" + scut "github.com/ossf/scorecard/v4/utests" +) + +const ( + // CheckCITests is the registered name for CITests. + CheckCITests = "CI-Tests" +) + +// Important: tests must include findings with values. +// Testing only for the outcome is insufficient, because the +// values of the findings are important to the probe. +func Test_Run(t *testing.T) { + t.Parallel() + // nolint:govet + tests := []struct { + name string + raw *checker.RawResults + findings []*finding.Finding + err error + }{ + { + name: "Has 1 CIInfo which has a successful CheckRun.", + raw: &checker.RawResults{ + CITestResults: checker.CITestData{ + CIInfo: []checker.RevisionCIInfo{ + { + HeadSHA: "HeadSHA", + PullRequestNumber: 1, + CheckRuns: []clients.CheckRun{ + { + Status: "completed", + Conclusion: "success", + App: clients.CheckRunApp{Slug: "e2e"}, + }, + }, + Statuses: []clients.Status{ + { + State: "not successful", + Context: CheckCITests, + TargetURL: "e2e", + }, + }, + }, + }, + }, + }, + findings: []*finding.Finding{ + { + Outcome: finding.OutcomePositive, + Probe: Probe, + Message: "CI test found: pr: 1, context: e2e", + Values: map[string]int{"totalMerged": 1, "totalTested": 1}, + Location: &finding.Location{Type: 4}, + }, + }, + }, + { + name: "Has 1 CIInfo which has a successful Status.", + raw: &checker.RawResults{ + CITestResults: checker.CITestData{ + CIInfo: []checker.RevisionCIInfo{ + { + HeadSHA: "HeadSHA", + PullRequestNumber: 1, + CheckRuns: []clients.CheckRun{ + { + Status: "incomplete", + Conclusion: "not successful", + App: clients.CheckRunApp{Slug: "e2e"}, + }, + }, + Statuses: []clients.Status{ + { + State: "success", + Context: CheckCITests, + TargetURL: "e2e", + }, + }, + }, + }, + }, + }, + findings: []*finding.Finding{ + { + Outcome: finding.OutcomePositive, + Probe: Probe, + Message: "CI test found: pr: HeadSHA, context: CI-Tests", + Values: map[string]int{"totalMerged": 1, "totalTested": 1}, + Location: &finding.Location{Type: 4}, + }, + }, + }, + } + for _, tt := range tests { + tt := tt // Re-initializing variable so it is not changed while executing the closure below + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + findings, s, err := Run(tt.raw) + if !cmp.Equal(tt.err, err, cmpopts.EquateErrors()) { + t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(tt.err, err, cmpopts.EquateErrors())) + } + if err != nil { + return + } + if diff := cmp.Diff(Probe, s); diff != "" { + t.Errorf("mismatch (-want +got):\n%s", diff) + } + if diff := cmp.Diff(len(tt.findings), len(findings)); diff != "" { + t.Errorf("mismatch (-want +got):\n%s", diff) + } + for i := range tt.findings { + outcome := &tt.findings[i] + f := &findings[i] + if diff := cmp.Diff(*outcome, f); diff != "" { + t.Errorf("mismatch (-want +got):\n%s", diff) + } + } + }) + } +} + +func Test_isTest(t *testing.T) { + t.Parallel() + type args struct { + s string + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "appveyor", + args: args{ + s: "appveyor", + }, + want: true, + }, + { + name: "circleci", + args: args{ + s: "circleci", + }, + want: true, + }, + { + name: "jenkins", + args: args{ + s: "jenkins", + }, + want: true, + }, + { + name: "e2e", + args: args{ + s: "e2e", + }, + want: true, + }, + { + name: "github-actions", + args: args{ + s: "github-actions", + }, + want: true, + }, + { + name: "mergeable", + args: args{ + s: "mergeable", + }, + want: true, + }, + { + name: "packit-as-a-service", + args: args{ + s: "packit-as-a-service", + }, + want: true, + }, + { + name: "semaphoreci", + args: args{ + s: "semaphoreci", + }, + want: true, + }, + { + name: "test", + args: args{ + s: "test", + }, + want: true, + }, + { + name: "travis-ci", + args: args{ + s: "travis-ci", + }, + want: true, + }, + { + name: "azure-pipelines", + args: args{ + s: "azure-pipelines", + }, + want: true, + }, + { + name: "non-existing", + args: args{ + s: "non-existing", + }, + want: false, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if got := isTest(tt.args.s); got != tt.want { + t.Errorf("isTest() = %v, want %v for test %v", got, tt.want, tt.name) + } + }) + } +} + +func Test_prHasSuccessfulCheck(t *testing.T) { + t.Parallel() + + //enabled nolint because this is a test + //nolint + tests := []struct { + name string + args checker.RevisionCIInfo + want bool + wantErr bool + }{ + { + name: "check run with conclusion success", + args: checker.RevisionCIInfo{ + PullRequestNumber: 1, + HeadSHA: "sha", + CheckRuns: []clients.CheckRun{ + { + App: clients.CheckRunApp{Slug: "test"}, + Conclusion: "success", + URL: "url", + Status: "completed", + }, + }, + }, + want: true, + wantErr: false, + }, + { + name: "check run with conclusion not success", + args: checker.RevisionCIInfo{ + PullRequestNumber: 1, + HeadSHA: "sha", + CheckRuns: []clients.CheckRun{ + { + App: clients.CheckRunApp{Slug: "test"}, + Conclusion: "failed", + URL: "url", + Status: "completed", + }, + }, + }, + want: false, + wantErr: false, + }, + { + name: "check run with conclusion not success", + args: checker.RevisionCIInfo{ + PullRequestNumber: 1, + HeadSHA: "sha", + CheckRuns: []clients.CheckRun{ + { + App: clients.CheckRunApp{Slug: "test"}, + Conclusion: "success", + URL: "url", + Status: "notcompleted", + }, + }, + }, + want: false, + wantErr: false, + }, + } + for _, tt := range tests { + tt := tt + + //nolint:errcheck + got, _, _ := prHasSuccessfulCheck(tt.args) + if got != tt.want { + t.Errorf("prHasSuccessfulCheck() = %v, want %v", got, tt.want) + } + } +} + +func Test_prHasSuccessStatus(t *testing.T) { + t.Parallel() + type args struct { //nolint:govet + r checker.RevisionCIInfo + } + tests := []struct { //nolint:govet + name string + args args + want bool + wantErr bool + }{ + { + name: "empty revision", + args: args{ + r: checker.RevisionCIInfo{}, + }, + want: false, + wantErr: false, + }, + { + name: "no statuses", + args: args{ + r: checker.RevisionCIInfo{ + Statuses: []clients.Status{}, + }, + }, + }, + { + name: "status is not success", + args: args{ + r: checker.RevisionCIInfo{ + Statuses: []clients.Status{ + { + State: "failure", + }, + }, + }, + }, + }, + { + name: "status is success", + args: args{ + r: checker.RevisionCIInfo{ + Statuses: []clients.Status{ + { + State: "success", + Context: CheckCITests, + }, + }, + }, + }, + want: true, + wantErr: false, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got, _, err := prHasSuccessStatus(tt.args.r) //nolint:govet + if (err != nil) != tt.wantErr { //nolint:govet + t.Errorf("prHasSuccessStatus() error = %v, wantErr %v", err, tt.wantErr) //nolint:govet + return + } + if got != tt.want { //nolint:govet + t.Errorf("prHasSuccessStatus() got = %v, want %v", got, tt.want) //nolint:govet + } + }) + } +} + +func Test_prHasSuccessfulCheckAdditional(t *testing.T) { + t.Parallel() + type args struct { //nolint:govet + r checker.RevisionCIInfo + dl checker.DetailLogger + } + tests := []struct { //nolint:govet + name string + args args + want bool + wantErr bool + }{ + { + name: "empty revision", + args: args{ + r: checker.RevisionCIInfo{}, + }, + want: false, + wantErr: false, + }, + { + name: "status is not completed", + args: args{ + r: checker.RevisionCIInfo{ + CheckRuns: []clients.CheckRun{ + { + Status: "notcompleted", + }, + }, + }, + }, + }, + { + name: "status is not success", + args: args{ + r: checker.RevisionCIInfo{ + CheckRuns: []clients.CheckRun{ + { + Status: "completed", + Conclusion: "failure", + }, + }, + }, + }, + }, + { + name: "conclusion is success", + args: args{ + r: checker.RevisionCIInfo{ + CheckRuns: []clients.CheckRun{ + { + Status: "completed", + Conclusion: "success", + }, + }, + }, + }, + }, + { + name: "conclusion is succesls with a valid app slug", + args: args{ + r: checker.RevisionCIInfo{ + CheckRuns: []clients.CheckRun{ + { + Status: "completed", + Conclusion: "success", + App: clients.CheckRunApp{Slug: "e2e"}, + }, + }, + }, + dl: &scut.TestDetailLogger{}, + }, + want: true, + wantErr: false, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got, _, err := prHasSuccessfulCheck(tt.args.r) + if (err != nil) != tt.wantErr { + t.Errorf("prHasSuccessfulCheck() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { //nolint:govet + t.Errorf("prHasSuccessfulCheck() got = %v, want %v", got, tt.want) + } + }) + } +}