From 30ef6b10268b645008b9360c32fe1829752db99d Mon Sep 17 00:00:00 2001 From: AdamKorcz <44787359+AdamKorcz@users.noreply.github.com> Date: Mon, 11 Dec 2023 18:15:50 +0000 Subject: [PATCH] :seedling: convert CI-Tests check to probes (#3621) * :seedling: convert CITest check to probes Signed-off-by: AdamKorcz * fix lint issues Signed-off-by: Adam Korczynski * debug failing integration test Signed-off-by: Adam Korczynski * Add negative outcome to test Signed-off-by: Adam Korczynski * remove 'totalTested' and 'totalMerged' values from findings Signed-off-by: Adam Korczynski * Log at debug level Signed-off-by: Adam Korczynski --------- Signed-off-by: AdamKorcz Signed-off-by: Adam Korczynski --- checks/ci_tests.go | 18 +- checks/evaluation/ci_tests.go | 134 +++----- checks/evaluation/ci_tests_test.go | 470 ++++------------------------ probes/entries.go | 4 + probes/testsRunInCI/def.yml | 28 ++ probes/testsRunInCI/impl.go | 169 ++++++++++ probes/testsRunInCI/impl_test.go | 486 +++++++++++++++++++++++++++++ 7 files changed, 806 insertions(+), 503 deletions(-) create mode 100644 probes/testsRunInCI/def.yml create mode 100644 probes/testsRunInCI/impl.go create mode 100644 probes/testsRunInCI/impl_test.go diff --git a/checks/ci_tests.go b/checks/ci_tests.go index c2e577385af..b5d37405710 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 8dc9311abe6..4c55fcc31c9 100644 --- a/checks/evaluation/ci_tests.go +++ b/checks/evaluation/ci_tests.go @@ -16,122 +16,72 @@ 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 - } +const CheckCITests = "CI-Tests" - // GitHub Check Runs. - prCheckSuccessful, err := prHasSuccessfulCheck(r, dl) - if err != nil { - return checker.CreateRuntimeErrorResult(CheckCITests, err) - } - if prCheckSuccessful { - totalTested++ - foundCI = true - } - - 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 +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) + } + // Debug PRs that were merged without CI tests + for i := range findings { + f := &findings[i] + if f.Outcome == finding.OutcomeNegative || f.Outcome == finding.OutcomePositive { dl.Debug(&checker.LogMessage{ - Text: fmt.Sprintf("merged PR %d without CI test at HEAD: %s", r.PullRequestNumber, r.HeadSHA), + Text: f.Message, }) } } - if totalMerged == 0 { + // check that the project has pull requests + if noPullRequestsFound(findings) { return checker.CreateInconclusiveResult(CheckCITests, "no pull request found") } + totalMerged, totalTested := getMergedAndTested(findings) + + if totalMerged < totalTested || len(findings) < totalTested { + e := sce.WithMessage(sce.ErrScorecardInternal, "invalid finding values") + return checker.CreateRuntimeErrorResult(name, e) + } + reason := fmt.Sprintf("%d out of %d merged PRs checked by a CI test", totalTested, totalMerged) return checker.CreateProportionalScoreResult(CheckCITests, reason, totalTested, totalMerged) } -// 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 - } - } - return false, nil -} +func getMergedAndTested(findings []finding.Finding) (int, int) { + totalMerged := 0 + totalTested := 0 -// 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 + for i := range findings { + f := &findings[i] + totalMerged++ + if f.Outcome == finding.OutcomePositive { + totalTested++ } } - return false, nil -} -// isTest returns true if the given string is a CI test. -func isTest(s string) bool { - l := strings.ToLower(s) + return totalMerged, totalTested +} - // 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 19e20d3668c..825ad5c6142 100644 --- a/checks/evaluation/ci_tests_test.go +++ b/checks/evaluation/ci_tests_test.go @@ -16,441 +16,103 @@ package evaluation import ( "testing" - "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" ) -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() - 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", + 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, + NumberOfDebug: 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", + 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{}, - }, - }, - }, - { - 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", + 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", - }, - }, + { + Outcome: finding.OutcomePositive, + Probe: "testsRunInCI", + Message: "CI test found: pr: 1, context: e2e", + Location: &finding.Location{Type: 4}, }, - }, - }, - { - name: "status is not success", - args: args{ - r: checker.RevisionCIInfo{ - CheckRuns: []clients.CheckRun{ - { - Status: "completed", - Conclusion: "failure", - }, - }, + { + Outcome: finding.OutcomeNegative, + Probe: "testsRunInCI", + Message: "CI test found: pr: 1, context: e2e", + Location: &finding.Location{Type: 4}, }, }, - }, - { - name: "conclusion is success", - args: args{ - r: checker.RevisionCIInfo{ - CheckRuns: []clients.CheckRun{ - { - Status: "completed", - Conclusion: "success", - }, - }, - }, + result: scut.TestReturn{ + Score: 7, + NumberOfDebug: 4, }, }, { - 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: "Tests debugging", + findings: []finding.Finding{ + { + Outcome: finding.OutcomeNegative, + Probe: "testsRunInCI", + Message: "merged PR 1 without CI test at HEAD: 1", + 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.OutcomeNegative, + Probe: "testsRunInCI", + Message: "merged PR 1 without CI test at HEAD: 1", + 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.OutcomeNegative, + Probe: "testsRunInCI", + Message: "merged PR 1 without CI test at HEAD: 1", + 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{ + NumberOfDebug: 3, + Score: 0, }, - 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 4486ecd8913..9f4f8ede19a 100644 --- a/probes/entries.go +++ b/probes/entries.go @@ -50,6 +50,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" @@ -129,6 +130,9 @@ var ( Webhook = []ProbeImpl{ webhooksUseSecrets.Run, } + CITests = []ProbeImpl{ + testsRunInCI.Run, + } ) //nolint:gochecknoinits diff --git a/probes/testsRunInCI/def.yml b/probes/testsRunInCI/def.yml new file mode 100644 index 00000000000..447dbeaa78b --- /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 PR that ran CI tests and one OutcomeNegative for each PR that did not run CI tests. + - 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 00000000000..4ae9576943d --- /dev/null +++ b/probes/testsRunInCI/impl.go @@ -0,0 +1,169 @@ +// 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 + } + + for i := range c.CIInfo { + r := c.CIInfo[i] + // 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) + 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) + } + + if !prSuccessStatus && !prCheckSuccessful { + f, err := finding.NewWith(fs, Probe, + fmt.Sprintf("merged PR %d without CI test at HEAD: %s", r.PullRequestNumber, r.HeadSHA), + nil, finding.OutcomeNegative) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + findings = append(findings, *f) + } + } + + 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 00000000000..670be5dfddb --- /dev/null +++ b/probes/testsRunInCI/impl_test.go @@ -0,0 +1,486 @@ +// 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", + 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", + 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() + + 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 { + r checker.RevisionCIInfo + } + tests := []struct { + 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) + 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", + }, + }, + }, + }, + }, + { + 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 { + t.Errorf("prHasSuccessfulCheck() got = %v, want %v", got, tt.want) + } + }) + } +}