Skip to content

Commit

Permalink
Fix various golint warnings (bazelbuild#180)
Browse files Browse the repository at this point in the history
  • Loading branch information
fweikert authored Oct 7, 2020
1 parent e86203d commit cfabe3c
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 27 deletions.
1 change: 1 addition & 0 deletions bazelisk.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// Package main is the entry point for Bazelisk.
package main

import (
Expand Down
16 changes: 8 additions & 8 deletions bazelisk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,28 +23,28 @@ func TestScanIssuesForIncompatibleFlags(t *testing.T) {
if flags == nil {
t.Errorf("Could not parse sample issues")
}
expected_flagnames := []string{
expectedFlagnames := []string{
"--//some/path:incompatible_user_defined_flag",
"--incompatible_always_check_depset_elements",
"--incompatible_no_implicit_file_export",
"--incompatible_remove_enabled_toolchain_types",
"--incompatible_remove_ram_utilization_factor",
"--incompatible_validate_top_level_header_inclusions",
}
var got_flags []string
var gotFlags []string
for _, flag := range flags {
fmt.Printf("%s\n", flag.String())
got_flags = append(got_flags, flag.Name)
gotFlags = append(gotFlags, flag.Name)
}
sort.Strings(got_flags)
sort.Strings(gotFlags)
mismatch := false
for i, got := range got_flags {
if expected_flagnames[i] != got {
for i, got := range gotFlags {
if expectedFlagnames[i] != got {
mismatch = true
break
}
}
if mismatch || len(expected_flagnames) != len(got_flags) {
t.Errorf("Expected %s, got %s", expected_flagnames, got_flags)
if mismatch || len(expectedFlagnames) != len(gotFlags) {
t.Errorf("Expected %s, got %s", expectedFlagnames, gotFlags)
}
}
18 changes: 9 additions & 9 deletions bazelisk_version_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func TestMain(m *testing.M) {
}

func TestResolveLatestRcVersion(t *testing.T) {
s := SetUp(t)
s := setUp(t)
s.AddVersion("4.0.0", false)
s.AddVersion("10.0.0", false)
s.AddVersion("11.0.0", true)
Expand All @@ -60,7 +60,7 @@ func TestResolveLatestRcVersion(t *testing.T) {
}

func TestResolveLatestVersion_TwoLatestVersionsDoNotHaveAReleaseYet(t *testing.T) {
s := SetUp(t)
s := setUp(t)
s.AddVersion("4.0.0", true)
s.AddVersion("5.0.0", false)
s.AddVersion("6.0.0", false)
Expand All @@ -80,7 +80,7 @@ func TestResolveLatestVersion_TwoLatestVersionsDoNotHaveAReleaseYet(t *testing.T
}

func TestResolveLatestVersion_FilterReleaseCandidates(t *testing.T) {
s := SetUp(t)
s := setUp(t)
s.AddVersion("3.0.0", true)
s.AddVersion("4.0.0", false)
s.AddVersion("5.0.0", false)
Expand All @@ -101,7 +101,7 @@ func TestResolveLatestVersion_FilterReleaseCandidates(t *testing.T) {
}

func TestResolveLatestVersion_ShouldFailIfNotEnoughReleases(t *testing.T) {
s := SetUp(t)
s := setUp(t)
s.AddVersion("3.0.0", true)
s.AddVersion("4.0.0", false)
s.Finish()
Expand All @@ -120,7 +120,7 @@ func TestResolveLatestVersion_ShouldFailIfNotEnoughReleases(t *testing.T) {
}

func TestResolveLatestVersion_GCSIsDown(t *testing.T) {
SetUp(t).WithError().Finish()
setUp(t).WithError().Finish()

transport.AddResponse("https://www.googleapis.com/storage/v1/b/bazel/o?delimiter=/", 500, "")

Expand Down Expand Up @@ -190,7 +190,7 @@ func (g *gcsSetup) addURL(prefix string, containsItem bool, childPrefixes ...str
transport.AddResponse(fmt.Sprintf("%s&prefix=%s", g.baseURL, prefix), 200, resp)
}

func SetUp(t *testing.T) *gcsSetup {
func setUp(t *testing.T) *gcsSetup {
return &gcsSetup{
baseURL: "https://www.googleapis.com/storage/v1/b/bazel/o?delimiter=/",
status: 200,
Expand Down Expand Up @@ -237,10 +237,10 @@ func buildGCSResponseOrFail(t *testing.T, prefixes []string, items []interface{}
Prefixes: prefixes,
Items: items,
}
if bytes, err := json.Marshal(r); err != nil {
byteValue, err := json.Marshal(r)
if err != nil {
t.Fatalf("Could not build GCS json response: %v", err)
return ""
} else {
return string(bytes)
}
return string(byteValue)
}
21 changes: 11 additions & 10 deletions core/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -432,17 +432,18 @@ type issueList struct {
Items []issue `json:"items"`
}

type flagDetails struct {
// Visible for testing
type FlagDetails struct {
Name string
ReleaseToFlip string
IssueURL string
}

func (f *flagDetails) String() string {
func (f *FlagDetails) String() string {
return fmt.Sprintf("%s (Bazel %s: %s)", f.Name, f.ReleaseToFlip, f.IssueURL)
}

func getIncompatibleFlags(bazeliskHome, resolvedBazelVersion string) (map[string]*flagDetails, error) {
func getIncompatibleFlags(bazeliskHome, resolvedBazelVersion string) (map[string]*FlagDetails, error) {
// GitHub labels use only major and minor version, we ignore the patch number (and any other suffix).
re := regexp.MustCompile(`^\d+\.\d+`)
version := re.FindString(resolvedBazelVersion)
Expand All @@ -461,23 +462,23 @@ func getIncompatibleFlags(bazeliskHome, resolvedBazelVersion string) (map[string

// ScanIssuesForIncompatibleFlags is visible for testing.
// TODO: move this function and its test into a dedicated package.
func ScanIssuesForIncompatibleFlags(issuesJSON []byte) (map[string]*flagDetails, error) {
result := make(map[string]*flagDetails)
func ScanIssuesForIncompatibleFlags(issuesJSON []byte) (map[string]*FlagDetails, error) {
result := make(map[string]*FlagDetails)
var issueList issueList
if err := json.Unmarshal(issuesJSON, &issueList); err != nil {
return nil, fmt.Errorf("could not parse JSON into list of issues: %v", err)
}

re := regexp.MustCompile(`^incompatible_\w+`)
s_re := regexp.MustCompile(`^//.*[^/]:incompatible_\w+`)
sRe := regexp.MustCompile(`^//.*[^/]:incompatible_\w+`)
for _, issue := range issueList.Items {
flag := re.FindString(issue.Title)
if len(flag) <= 0 {
flag = s_re.FindString(issue.Title)
flag = sRe.FindString(issue.Title)
}
if len(flag) > 0 {
name := "--" + flag
result[name] = &flagDetails{
result[name] = &FlagDetails{
Name: name,
ReleaseToFlip: getBreakingRelease(issue.Labels),
IssueURL: issue.URL,
Expand Down Expand Up @@ -554,7 +555,7 @@ func cleanIfNeeded(bazelPath string) {
}

// migrate will run Bazel with each newArgs separately and report which ones are failing.
func migrate(bazelPath string, baseArgs []string, flags map[string]*flagDetails) {
func migrate(bazelPath string, baseArgs []string, flags map[string]*FlagDetails) {
newArgs := getSortedKeys(flags)
// 1. Try with all the flags.
args := insertArgs(baseArgs, newArgs)
Expand Down Expand Up @@ -623,7 +624,7 @@ func migrate(bazelPath string, baseArgs []string, flags map[string]*flagDetails)
os.Exit(1)
}

func getSortedKeys(data map[string]*flagDetails) []string {
func getSortedKeys(data map[string]*FlagDetails) []string {
result := make([]string, 0)
for key := range data {
result = append(result, key)
Expand Down
1 change: 1 addition & 0 deletions core/repositories.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
)

const (
// BaseURLEnv is the name of the environment variable that stores the base URL for downloads.
BaseURLEnv = "BAZELISK_BASE_URL"
)

Expand Down

0 comments on commit cfabe3c

Please sign in to comment.