options: Rewrite unit tests (2/n)

Signed-off-by: Stephen Augustus <foo@auggie.dev>

entrypoint/entrypoint.go

@@ -29,7 +29,11 @@ import (
 // New creates a new scorecard command which can be used as an entrypoint for
 // GitHub Actions.
 func New() (*cobra.Command, error) {
-	opts := options.New()
+	opts, err := options.New()
+	if err != nil {
+		return nil, fmt.Errorf("creating new options: %w", err)
+	}
+
 	if err := opts.Initialize(); err != nil {
 		return nil, fmt.Errorf("initializing options: %w", err)
 	}

go.mod

@@ -4,6 +4,7 @@ go 1.17
 
 require (
 	github.com/caarlos0/env/v6 v6.9.1
+	github.com/google/go-cmp v0.5.7
 	github.com/ossf/scorecard/v4 v4.1.1-0.20220303013834-3070b3ca1b59
 	github.com/spf13/cobra v1.3.0
 )
@@ -23,7 +24,6 @@ require (
 	github.com/golang-jwt/jwt/v4 v4.0.0 // indirect
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
 	github.com/golang/protobuf v1.5.2 // indirect
-	github.com/google/go-cmp v0.5.7 // indirect
 	github.com/google/go-github/v38 v38.1.0 // indirect
 	github.com/google/go-github/v41 v41.0.0 // indirect
 	github.com/google/go-querystring v1.1.0 // indirect

options/options.go

@@ -33,32 +33,20 @@ import (
 var (
 	// Errors.
 	errGithubEventPathEmpty       = errors.New("GitHub event path is empty")
+	errResultsPathEmpty           = errors.New("results path is empty")
 	errOnlyDefaultBranchSupported = errors.New("only default branch is supported")
 
 	trueStr = "true"
 )
 
-// Options TODO(lint): should have comment or be unexported (revive).
+// Options are options for running scorecard via GitHub Actions.
 type Options struct {
 	// Scorecard options.
 	ScorecardOpts *options.Options
 
 	// Scorecard command-line options.
 	EnabledChecks string `env:"ENABLED_CHECKS"`
 
-	// Input options.
-	// TODO(options): These input options shadow the some of the SCORECARD_
-	//                env vars:
-	//                export SCORECARD_RESULTS_FILE="$INPUT_RESULTS_FILE"
-	//                export SCORECARD_RESULTS_FORMAT="$INPUT_RESULTS_FORMAT"
-	//                export SCORECARD_PUBLISH_RESULTS="$INPUT_PUBLISH_RESULTS"
-	//
-	//                Let's target them for deletion, but only after confirming
-	//                that there isn't anything that surprisingly needs them.
-	InputResultsFile    string `env:"INPUT_RESULTS_FILE"`
-	InputResultsFormat  string `env:"INPUT_RESULTS_FORMAT"`
-	InputPublishResults string `env:"INPUT_PUBLISH_RESULTS"`
-
 	// Scorecard checks.
 	EnableLicense           string `env:"ENABLE_LICENSE"`
 	EnableDangerousWorkflow string `env:"ENABLE_DANGEROUS_WORKFLOW"`
@@ -84,56 +72,45 @@ const (
 	formatSarif                = options.FormatSarif
 )
 
-// New TODO(lint): should have comment or be unexported (revive).
-func New() *Options {
-	opts := &Options{}
+// New creates a new options set for running scorecard via GitHub Actions.
+func New() (*Options, error) {
+	// Enable scorecard command to use SARIF format.
+	os.Setenv(options.EnvVarEnableSarif, trueStr)
+
+	opts := &Options{
+		ScorecardOpts: options.New(),
+	}
 	if err := env.Parse(opts); err != nil {
-		// TODO(options): Consider making this an error.
-		fmt.Printf("parsing entrypoint env vars: %+v", err)
+		return opts, fmt.Errorf("parsing entrypoint env vars: %w", err)
 	}
 
-	// TODO(options): Push options into scorecard.Options once/if it supports
-	//                validation.
-	scOpts := options.New()
-
 	if err := opts.Initialize(); err != nil {
-		// TODO(options): Consider making this an error.
-		fmt.Printf("initializing scorecard-action options: %+v\n", err)
+		return opts, fmt.Errorf(
+			"initializing scorecard-action options: %w",
+			err,
+		)
 	}
 
 	// TODO(options): Move this set-or-default logic to its own function.
-	if opts.InputResultsFormat != "" {
-		scOpts.Format = opts.InputResultsFormat
-	} else {
-		scOpts.EnableSarif = true
-		scOpts.Format = formatSarif
-		os.Setenv(options.EnvVarEnableSarif, trueStr)
-		if scOpts.Format == "" {
-			// Default the scorecard command to using SARIF format.
-			if scOpts.PolicyFile == "" {
-				// TODO(policy): Should we default or error here?
-				scOpts.PolicyFile = defaultScorecardPolicyFile
-			}
+	opts.ScorecardOpts.EnableSarif = true
+	if opts.ScorecardOpts.Format == formatSarif {
+		if opts.ScorecardOpts.PolicyFile == "" {
+			// TODO(policy): Should we default or error here?
+			opts.ScorecardOpts.PolicyFile = defaultScorecardPolicyFile
 		}
 	}
 
-	if err := scOpts.Validate(); err != nil {
-		// TODO(options): Consider making this an error.
-		fmt.Printf("validating scorecard options: %+v\n", err)
+	if err := opts.ScorecardOpts.Validate(); err != nil {
+		return opts, fmt.Errorf("validating scorecard options: %w", err)
 	}
 
-	opts.ScorecardOpts = scOpts
 	opts.SetPublishResults()
-
-	if opts.ScorecardOpts.PublishResults {
-		if scOpts.ResultsFile == "" {
-			scOpts.ResultsFile = opts.InputResultsFile
-			// TODO(options): We should check if this is empty.
-		}
+	if opts.ScorecardOpts.ResultsFile == "" {
+		return opts, errResultsPathEmpty
 	}
 
 	// TODO(options): Consider running Validate() before returning.
-	return opts
+	return opts, nil
 }
 
 // Initialize initializes the environment variables required for the action.
@@ -147,8 +124,9 @@ func (o *Options) Initialize() error {
 	   GITHUB_ACTIONS is true in GitHub env.
 	*/
 
-	o.EnableLicense = "1"
-	o.EnableDangerousWorkflow = "1"
+	// TODO(checks): Do we actually expect to use these?
+	// o.EnableLicense = "1"
+	// o.EnableDangerousWorkflow = "1"
 
 	return o.SetRepoInfo()
 }
@@ -197,23 +175,6 @@ func (o *Options) Print() {
 // SetPublishResults sets whether results should be published based on a
 // repository's visibility.
 func (o *Options) SetPublishResults() {
-	// Check INPUT_PUBLISH_RESULTS
-	var inputBool bool
-	var inputErr error
-	input := os.Getenv(EnvInputPublishResults)
-	if input != "" {
-		inputBool, inputErr = strconv.ParseBool(o.InputPublishResults)
-		if inputErr != nil {
-			// TODO(options): Consider making this an error.
-			fmt.Printf(
-				"could not parse a valid bool from %s (%s): %+v\n",
-				input,
-				EnvInputPublishResults,
-				inputErr,
-			)
-		}
-	}
-
 	privateRepo, err := strconv.ParseBool(o.PrivateRepoStr)
 	if err != nil {
 		// TODO(options): Consider making this an error.
@@ -226,8 +187,6 @@ func (o *Options) SetPublishResults() {
 
 	if privateRepo {
 		o.ScorecardOpts.PublishResults = false
-	} else {
-		o.ScorecardOpts.PublishResults = o.ScorecardOpts.PublishResults || inputBool
 	}
 }
 

options/options_test.go

@@ -16,13 +16,16 @@
 package options
 
 import (
+	"os"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
 	"github.com/ossf/scorecard/v4/options"
 )
 
 const (
-	testRepo = "good/repo"
+	testRepo        = "good/repo"
+	testResultsFile = "results.sarif"
 
 	githubEventPathNonFork   = "testdata/non-fork.json"
 	githubEventPathFork      = "testdata/fork.json"
@@ -31,13 +34,90 @@ const (
 	githubEventPathBadData   = "testdata/bad-data.json"
 )
 
+func TestNew(t *testing.T) {
+	tests := []struct {
+		name            string
+		githubEventPath string
+		repo            string
+		resultsFile     string
+		resultsFormat   string
+		publishResults  string
+		want            options.Options
+		wantErr         bool
+	}{
+		{
+			name:            "SuccessFormatSARIF",
+			githubEventPath: githubEventPathNonFork,
+			repo:            testRepo,
+			resultsFormat:   "sarif",
+			resultsFile:     testResultsFile,
+			want: options.Options{
+				Repo:        testRepo,
+				EnableSarif: true,
+				Format:      formatSarif,
+				PolicyFile:  defaultScorecardPolicyFile,
+				ResultsFile: testResultsFile,
+				Commit:      options.DefaultCommit,
+				LogLevel:    options.DefaultLogLevel,
+			},
+			wantErr: false,
+		},
+		{
+			name:            "SuccessFormatJSON",
+			githubEventPath: githubEventPathNonFork,
+			repo:            testRepo,
+			resultsFormat:   "json",
+			resultsFile:     testResultsFile,
+			want: options.Options{
+				Repo:        testRepo,
+				EnableSarif: true,
+				Format:      options.FormatJSON,
+				ResultsFile: testResultsFile,
+				Commit:      options.DefaultCommit,
+				LogLevel:    options.DefaultLogLevel,
+			},
+			wantErr: false,
+		},
+		/*
+			{
+				name:    "FailureNoEnvVars",
+				want:    options.Options{},
+				wantErr: true,
+			},
+		*/
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if tt.githubEventPath != "" {
+				os.Setenv(EnvGithubEventPath, tt.githubEventPath)
+			}
+			if tt.repo != "" {
+				os.Setenv(EnvGithubRepository, tt.repo)
+			}
+			if tt.resultsFile != "" {
+				os.Setenv("SCORECARD_RESULTS_FILE", tt.resultsFile)
+			}
+			if tt.resultsFormat != "" {
+				os.Setenv("SCORECARD_RESULTS_FORMAT", tt.resultsFormat)
+			}
+
+			opts, err := New()
+			got := *opts.ScorecardOpts
+			if (err != nil) != tt.wantErr {
+				t.Errorf("New() error = %+v, wantErr %+v", err, tt.wantErr)
+				return
+			}
+			if !cmp.Equal(tt.want, got) {
+				t.Errorf("New(): -want, +got:\n%s", cmp.Diff(tt.want, got))
+			}
+		})
+	}
+}
+
 func TestInitialize(t *testing.T) {
 	type fields struct {
 		ScorecardOpts           *options.Options
 		EnabledChecks           string
-		InputResultsFile        string
-		InputResultsFormat      string
-		InputPublishResults     string
 		EnableLicense           string
 		EnableDangerousWorkflow string
 		GithubEventName         string
@@ -85,9 +165,6 @@ func TestInitialize(t *testing.T) {
 			o := &Options{
 				ScorecardOpts:           tt.fields.ScorecardOpts,
 				EnabledChecks:           tt.fields.EnabledChecks,
-				InputResultsFile:        tt.fields.InputResultsFile,
-				InputResultsFormat:      tt.fields.InputResultsFormat,
-				InputPublishResults:     tt.fields.InputPublishResults,
 				EnableLicense:           tt.fields.EnableLicense,
 				EnableDangerousWorkflow: tt.fields.EnableDangerousWorkflow,
 				GithubEventName:         tt.fields.GithubEventName,