From 0d099eb3ad970628536bf5ded8e6590187102582 Mon Sep 17 00:00:00 2001 From: jaskaransarkaria Date: Tue, 24 Sep 2024 15:21:35 +0100 Subject: [PATCH] =?UTF-8?q?fix:=20=F0=9F=90=9B=20static=20code=20error=20a?= =?UTF-8?q?nd=20mock=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- pkg/environment/createPR.go | 20 ++++++++++--------- pkg/environment/isRdsVersionMismatched.go | 12 +++++------ .../isRdsVersionMismatched_test.go | 6 +++--- pkg/github/client_test.go | 7 ++++--- 4 files changed, 24 insertions(+), 21 deletions(-) diff --git a/pkg/environment/createPR.go b/pkg/environment/createPR.go index a1844622..95b247fb 100644 --- a/pkg/environment/createPR.go +++ b/pkg/environment/createPR.go @@ -13,29 +13,31 @@ import ( func createPR(description, namespace string) func(github.GithubIface, []string) (string, error) { b := make([]byte, 2) - rand.Read(b) + + rand.Read(b) //nolint:errcheck + fourCharUid := hex.EncodeToString(b) branchName := namespace + "-rds-minor-version-bump-" + fourCharUid return func(gh github.GithubIface, filenames []string) (string, error) { checkCmd := exec.Command("/bin/sh", "-c", "git checkout -b "+branchName) - checkCmd.Start() - checkCmd.Wait() + checkCmd.Start() //nolint:errcheck + checkCmd.Wait() //nolint:errcheck strFiles := strings.Join(filenames, " ") cmd := exec.Command("/bin/sh", "-c", "git add "+strFiles) cmd.Dir = "namespaces/live.cloud-platform.service.justice.gov.uk/" + namespace + "/resources" - cmd.Start() - cmd.Wait() + cmd.Start() //nolint:errcheck + cmd.Wait() //nolint:errcheck commitCmd := exec.Command("/bin/sh", "-c", "git commit -m 'concourse: correcting rds version drift'") commitCmd.Dir = "namespaces/live.cloud-platform.service.justice.gov.uk/" + namespace + "/resources" - commitCmd.Start() - commitCmd.Wait() + commitCmd.Start() //nolint:errcheck + commitCmd.Wait() //nolint:errcheck pushCmd := exec.Command("/bin/sh", "-c", "git push --set-upstream origin "+branchName) - pushCmd.Start() - pushCmd.Wait() + pushCmd.Start() //nolint:errcheck + pushCmd.Wait() //nolint:errcheck return gh.CreatePR(branchName, namespace, description) } diff --git a/pkg/environment/isRdsVersionMismatched.go b/pkg/environment/isRdsVersionMismatched.go index 7009b04a..cf43c0de 100644 --- a/pkg/environment/isRdsVersionMismatched.go +++ b/pkg/environment/isRdsVersionMismatched.go @@ -18,7 +18,7 @@ func IsRdsVersionMismatched(outputTerraform string) (*RdsVersionResults, error) match, _ := regexp.MatchString("Error: updating RDS DB Instance .* api error InvalidParameterCombination:.* from .* to .*", outputTerraform) if !match { - return nil, errors.New("terraform is failing but it doesn't look like a rds version mismatch.") + return nil, errors.New("terraform is failing but it doesn't look like a rds version mismatch") } versionRe := regexp.MustCompile(`from (?P\d+\.\d+) to (?P\d+\.\d+)`) @@ -32,11 +32,11 @@ func IsRdsVersionMismatched(outputTerraform string) (*RdsVersionResults, error) sanitisedNames := removeInputStr(moduleMatches) if !checkVersionDowngrade(sanitisedVersions) { - return nil, errors.New("terraform is failing, but it isn't trying to downgrade the RDS versions so it needs more investigation.") + return nil, errors.New("terraform is failing, but it isn't trying to downgrade the RDS versions so it needs more investigation") } if len(sanitisedVersions) != len(sanitisedNames) { - return nil, fmt.Errorf("Error: there is an inconistent number of versions vs module names, there should be an even amount but we have %d sets of versions and %d module names", len(sanitisedVersions), len(sanitisedNames)) + return nil, fmt.Errorf("error: there is an inconistent number of versions vs module names, there should be an even amount but we have %d sets of versions and %d module names", len(sanitisedVersions), len(sanitisedNames)) } return &RdsVersionResults{ @@ -57,12 +57,12 @@ func checkVersionDowngrade(versions [][]string) bool { adjustedAcc := strings.Join(splitAcc, "") adjustedTf := strings.Join(splitTf, "") - acc, err := strconv.ParseInt(adjustedAcc, 0, 64) - tf, err := strconv.ParseInt(adjustedTf, 0, 64) + acc, accErr := strconv.ParseInt(adjustedAcc, 0, 64) + tf, tfErr := strconv.ParseInt(adjustedTf, 0, 64) isUpgrade := tf > acc - if err != nil || isUpgrade { + if accErr != nil || tfErr != nil || isUpgrade { isValid = false break } diff --git a/pkg/environment/isRdsVersionMismatched_test.go b/pkg/environment/isRdsVersionMismatched_test.go index 55f1a469..e9656776 100644 --- a/pkg/environment/isRdsVersionMismatched_test.go +++ b/pkg/environment/isRdsVersionMismatched_test.go @@ -94,8 +94,8 @@ func TestIsRdsVersionMismatched(t *testing.T) { }, nil, }, - {"GIVEN an output WITHOUT a version mismatch THEN return nil", nomatchOutput, nil, errors.New("terraform is failing but it doesn't look like a rds version mismatch.")}, - {"GIVEN an output WITH a version mismatch BUT the mismatch would cause a db UPGRADE THEN return nil", isVersionUpgradeMismatch, nil, errors.New("terraform is failing, but it isn't trying to downgrade the RDS versions so it needs more investigation.")}, + {"GIVEN an output WITHOUT a version mismatch THEN return nil", nomatchOutput, nil, errors.New("terraform is failing but it doesn't look like a rds version mismatch")}, + {"GIVEN an output WITH a version mismatch BUT the mismatch would cause a db UPGRADE THEN return nil", isVersionUpgradeMismatch, nil, errors.New("terraform is failing, but it isn't trying to downgrade the RDS versions so it needs more investigation")}, { "GIVEN an output WITH MULTIPLE version mismatches THEN return the correct string slices", multipleMismatchOutput, &environment.RdsVersionResults{ Versions: [][]string{{"14.12", "14.10"}, {"16.18", "16.16"}}, @@ -104,7 +104,7 @@ func TestIsRdsVersionMismatched(t *testing.T) { }, nil, }, - {"GIVEN an output with an inconsistent number of versions and modules THEN return nil and an error", inconsistentOutput, nil, errors.New("Error: there is an inconistent number of versions vs module names, there should be an even amount but we have 1 sets of versions and 3 module names")}, + {"GIVEN an output with an inconsistent number of versions and modules THEN return nil and an error", inconsistentOutput, nil, errors.New("error: there is an inconistent number of versions vs module names, there should be an even amount but we have 1 sets of versions and 3 module names")}, } for _, tt := range tests { diff --git a/pkg/github/client_test.go b/pkg/github/client_test.go index 6bcc7ee0..f239430e 100644 --- a/pkg/github/client_test.go +++ b/pkg/github/client_test.go @@ -22,6 +22,10 @@ func (m *mockGithub) IsMerged(ctx context.Context, owner string, repo string, nu return true, nil, nil } +func (m *mockGithub) Create(ctx context.Context, owner string, repo string, pr *github.NewPullRequest) (*github.PullRequest, *github.Response, error) { + return nil, nil, nil +} + func TestNewGithubClient(t *testing.T) { type args struct { config *GithubClientConfig @@ -51,7 +55,6 @@ func TestNewGithubClient(t *testing.T) { } func TestGithubClient_GetChangedFiles(t *testing.T) { - mc := &mockGithub{ resp: []*github.CommitFile{ { @@ -95,7 +98,6 @@ func TestGithubClient_IsMerged(t *testing.T) { PullRequests: mc, } got, err := gh.IsMerged(8344) - if err != nil { t.Errorf("GithubClient.IsMerged() error = %v, wantErr %v", err, nil) return @@ -103,5 +105,4 @@ func TestGithubClient_IsMerged(t *testing.T) { if got != mc.merged { t.Errorf("GithubClient.IsMerged() = %v, want %v", got, true) } - }