test: Add comprehensive unit tests for goodLicense with dependency injection (#136)

* Add comprehensive unit tests for goodLicense with dependency injection

* chore: mock http client

- add interface so we can mock http client and its response or error
- this allows us to still test the content of MakeApiCall and not mock out the entire function (aka valuable)

Signed-off-by: jmeridth <jmeridth@gmail.com>

* chore: cleanup the http mock creation into a helper function

Signed-off-by: jmeridth <jmeridth@gmail.com>

* fix: linting

Signed-off-by: jmeridth <jmeridth@gmail.com>

---------

Signed-off-by: jmeridth <jmeridth@gmail.com>
Co-authored-by: jmeridth <jmeridth@gmail.com>

Author: zohayb23

data/rest-data.go

@@ -15,6 +15,10 @@ import (
 	"github.com/privateerproj/privateer-sdk/config"
 )
 
+type HttpClient interface {
+    Do(req *http.Request) (*http.Response, error)
+}
+
 type RestData struct {
 	owner               string
 	repo                string
@@ -26,6 +30,7 @@ type RestData struct {
 	Rulesets            []Ruleset
 	contents            RepoContent
 	ghClient            *github.Client
+	HttpClient          HttpClient
 }
 
 type RepoContent struct {
@@ -75,16 +80,20 @@ func (r *RestData) Setup() error {
 }
 
 func (r *RestData) MakeApiCall(endpoint string, isGithub bool) (body []byte, err error) {
-	r.Config.Logger.Trace(fmt.Sprintf("GET %s", endpoint))
+	if r.Config != nil && r.Config.Logger != nil {
+		r.Config.Logger.Trace(fmt.Sprintf("GET %s", endpoint))
+	}
 	request, err := http.NewRequest("GET", endpoint, nil)
 	if err != nil {
 		return nil, err
 	}
 	if isGithub {
 		request.Header.Set("Authorization", "Bearer "+r.token)
 	}
-	client := &http.Client{}
-	response, err := client.Do(request)
+	if r.HttpClient == nil {
+		r.HttpClient = &http.Client{}
+	}
+	response, err := r.HttpClient.Do(request)
 	if err != nil {
 		err = fmt.Errorf("error making http call: %s", err.Error())
 		return nil, err

data/rest_data_mock.go

@@ -0,0 +1,34 @@
+package data
+
+import (
+	"bytes"
+	"io"
+	"net/http"
+)
+
+type ClientMock struct {
+	Response *http.Response
+	Err      error
+}
+
+func (c *ClientMock) Do(req *http.Request) (*http.Response, error) {
+    return c.Response, c.Err
+}
+
+func NewPayloadWithHTTPMock(base Payload, body []byte, statusCode int, httpErr error) Payload {
+	if statusCode == 0 {
+		statusCode = http.StatusOK
+	}
+	mock := &ClientMock{
+		Response: &http.Response{
+			StatusCode: statusCode,
+			Body:       io.NopCloser(bytes.NewReader(body)),
+		},
+		Err: httpErr,
+	}
+	if base.RestData == nil {
+		base.RestData = &RestData{}
+	}
+	base.HttpClient = mock
+	return base
+}

evaluation_plans/osps/legal/steps.go

@@ -84,6 +84,7 @@ func goodLicense(payloadData any, _ map[string]*layer4.Change) (result layer4.Re
 	}
 
 	licenses, errString := getLicenseList(data, nil)
+
 	if errString != "" {
 		return layer4.Unknown, errString
 	}
@@ -116,8 +117,10 @@ func goodLicense(payloadData any, _ map[string]*layer4.Change) (result layer4.Re
 		}
 	}
 	approvedLicenses := strings.Join(spdx_ids, ", ")
-	data.Config.Logger.Trace(fmt.Sprintf("Requested licenses: %s", approvedLicenses))
-	data.Config.Logger.Trace(fmt.Sprintf("Non-approved licenses: %s", badLicenses))
+	if data.Config.Logger != nil {
+		data.Config.Logger.Trace(fmt.Sprintf("Requested licenses: %s", approvedLicenses))
+		data.Config.Logger.Trace(fmt.Sprintf("Non-approved licenses: %s", badLicenses))
+	}
 
 	if len(badLicenses) > 0 {
 		return layer4.Failed, fmt.Sprintf("These licenses are not OSI or FSF approved: %s", strings.Join(badLicenses, ", "))

evaluation_plans/osps/legal/steps_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/ossf/gemara/layer4"
+	"github.com/privateerproj/privateer-sdk/config"
 	"github.com/revanite-io/pvtr-github-repo/data"
 	"github.com/stretchr/testify/assert"
 )
@@ -186,3 +187,105 @@ func TestSplitSpdxExpression(t *testing.T) {
 		})
 	}
 }
+
+func TestGoodLicense(t *testing.T) {
+	tests := []struct {
+		name            string
+		payloadData     any
+		apiResponse		 []byte
+		apiError        error
+		expectedResult  layer4.Result
+		expectedMessage string
+	}{
+		{
+			name:            "Invalid payload",
+			payloadData:     "invalid",
+			expectedResult:  layer4.Unknown,
+			expectedMessage: "Malformed assessment: expected payload type data.Payload, got string (invalid)",
+		},
+		{
+			name: "No license identifiers found",
+			payloadData: data.Payload{
+				GraphqlRepoData: &data.GraphqlRepoData{},
+				Config:          &config.Config{},
+			},
+			apiResponse: []byte(`{"licenses":[{"licenseId":"MIT","isOsiApproved":true,"isFsfLibre":false}]}`),
+			apiError: nil,
+			expectedResult:  layer4.Failed,
+			expectedMessage: "License SPDX identifier was not found in Security Insights data or via GitHub API",
+		},
+		{
+			name: "OSI approved license (MIT)",
+			payloadData: data.Payload{
+				GraphqlRepoData: func() *data.GraphqlRepoData {
+					repo := stubGraphqlRepo("")
+					repo.Repository.LicenseInfo.SpdxId = "MIT"
+					return repo
+				}(),
+				Config: &config.Config{},
+			},
+			apiResponse: []byte(`{"licenses":[{"licenseId":"MIT","isOsiApproved":true,"isFsfLibre":false}]}`),
+			apiError: nil,
+			expectedResult:  layer4.NeedsReview,
+			expectedMessage: "All license found are OSI or FSF approved",
+		},
+		{
+			name: "Non-approved license",
+			payloadData: data.Payload{
+				GraphqlRepoData: func() *data.GraphqlRepoData {
+					repo := stubGraphqlRepo("")
+					repo.Repository.LicenseInfo.SpdxId = "BadLicense"
+					return repo
+				}(),
+				Config: &config.Config{},
+			},
+			apiResponse: []byte(`{"licenses":[{"licenseId":"BadLicense","isOsiApproved":false,"isFsfLibre":false}]}`),
+			apiError: nil,
+			expectedResult:  layer4.Failed,
+			expectedMessage: "These licenses are not OSI or FSF approved: BadLicense",
+		},
+		{
+			name: "Multiple licenses with mixed approval",
+			payloadData: data.Payload{
+				GraphqlRepoData: func() *data.GraphqlRepoData {
+					repo := stubGraphqlRepo("")
+					repo.Repository.LicenseInfo.SpdxId = "MIT AND BadLicense"
+					return repo
+				}(),
+				Config: &config.Config{},
+			},
+			apiResponse: []byte(`{"licenses":[{"licenseId":"MIT","isOsiApproved":true,"isFsfLibre":false},{"licenseId":"BadLicense","isOsiApproved":false,"isFsfLibre":false}]}`),
+			apiError: nil,
+			expectedResult:  layer4.Failed,
+			expectedMessage: "These licenses are not OSI or FSF approved: BadLicense",
+		},
+		{
+			name: "Unknown license ID",
+			payloadData: data.Payload{
+				GraphqlRepoData: func() *data.GraphqlRepoData {
+					repo := stubGraphqlRepo("")
+					repo.Repository.LicenseInfo.SpdxId = "UnknownLicense"
+					return repo
+				}(),
+				Config: &config.Config{},
+			},
+			apiResponse: []byte(`{"licenses":[{"licenseId":"MIT","isOsiApproved":true,"isFsfLibre":false}]}`),
+			apiError: nil,
+			expectedResult:  layer4.Failed,
+			expectedMessage: "These licenses are not OSI or FSF approved: UnknownLicense",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			if payload, ok := test.payloadData.(data.Payload); ok {
+				payload = data.NewPayloadWithHTTPMock(payload, test.apiResponse, 200, test.apiError)
+				test.payloadData = payload
+			}
+
+			result, message := goodLicense(test.payloadData, nil)
+			assert.Equal(t, test.expectedResult, result)
+			assert.Equal(t, test.expectedMessage, message)
+		})
+	}
+}