From 43e450334323fde3097aa55d428ce79e938a1346 Mon Sep 17 00:00:00 2001 From: Dale Hui Date: Tue, 30 Oct 2018 18:43:46 -0700 Subject: [PATCH] Update golangci-lint config and implore contributors to use it - Run golangci-lint on tests and fix found issues --- .golangci.yml | 9 +------- CONTRIBUTING.md | 3 +++ database/mongodb/mongodb_test.go | 3 +++ database/parse_test.go | 10 ++++----- database/stub/stub_test.go | 3 +++ migrate_test.go | 37 ++++++++++++++++++-------------- testing/testing_test.go | 2 +- 7 files changed, 37 insertions(+), 30 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index c809d52..48350ee 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,10 +1,3 @@ -# options for analysis running -run: - tests: false - -output: - print-issued-lines: false - linters: enable: #- golint @@ -31,4 +24,4 @@ issues: exclude-use-default: false exclude: # gosec: Duplicated errcheck checks - - G104 \ No newline at end of file + - G104 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8214484..475540a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -5,8 +5,11 @@ 1. Use a version of Go that supports [modules](https://golang.org/cmd/go/#hdr-Modules__module_versions__and_more) (e.g. Go 1.11+) 1. Fork this repo and `git clone` somewhere to `$GOPATH/src/github.com/golang-migrate/migrate` * Ensure that [Go modules are enabled](https://golang.org/cmd/go/#hdr-Preliminary_module_support) (e.g. your repo path or the `GO111MODULE` environment variable are set correctly) + 1. Install [golangci-lint](https://github.com/golangci/golangci-lint#install) + 1. Run the linter: `golangci-lint run` 1. Confirm tests are working: `make test-short` 1. Write awesome code ... + 1. 1. `make test` to run all tests against all database versions 1. Push code and open Pull Request diff --git a/database/mongodb/mongodb_test.go b/database/mongodb/mongodb_test.go index 5f73a6f..d3d95f1 100644 --- a/database/mongodb/mongodb_test.go +++ b/database/mongodb/mongodb_test.go @@ -190,6 +190,9 @@ func TestTransaction(t *testing.T) { d, err := WithInstance(client, &Config{ DatabaseName: "testMigration", }) + if err != nil { + t.Fatal(err) + } defer d.Close() //We have to create collection //transactions don't support operations with creating new dbs, collections diff --git a/database/parse_test.go b/database/parse_test.go index bfbc0ba..6558e25 100644 --- a/database/parse_test.go +++ b/database/parse_test.go @@ -9,11 +9,12 @@ import ( const reservedChars = "!#$%&'()*+,/:;=?@[]" +const baseUsername = "username" + // TestUserUnencodedReservedURLChars documents the behavior of using unencoded reserved characters in usernames with // net/url Parse() func TestUserUnencodedReservedURLChars(t *testing.T) { scheme := "database://" - baseUsername := "username" urlSuffix := "password@localhost:12345/myDB?someParam=true" urlSuffixAndSep := ":" + urlSuffix @@ -46,7 +47,7 @@ func TestUserUnencodedReservedURLChars(t *testing.T) { encodedURL: scheme + baseUsername + "," + urlSuffixAndSep}, {char: "/", parses: true, expectedUsername: "", encodedURL: scheme + baseUsername + "/" + urlSuffixAndSep}, - {char: ":", parses: true, expectedUsername: "username", + {char: ":", parses: true, expectedUsername: baseUsername, encodedURL: scheme + baseUsername + ":%3A" + urlSuffix}, {char: ";", parses: true, expectedUsername: "username;", encodedURL: scheme + baseUsername + ";" + urlSuffixAndSep}, @@ -98,7 +99,6 @@ func TestUserUnencodedReservedURLChars(t *testing.T) { func TestUserEncodedReservedURLChars(t *testing.T) { scheme := "database://" - baseUsername := "username" urlSuffix := "password@localhost:12345/myDB?someParam=true" urlSuffixAndSep := ":" + urlSuffix @@ -125,7 +125,7 @@ func TestUserEncodedReservedURLChars(t *testing.T) { // TestPasswordUnencodedReservedURLChars documents the behavior of using unencoded reserved characters in passwords // with net/url Parse() func TestPasswordUnencodedReservedURLChars(t *testing.T) { - username := "username" + username := baseUsername schemeAndUsernameAndSep := "database://" + username + ":" basePassword := "password" urlSuffixAndSep := "@localhost:12345/myDB?someParam=true" @@ -215,7 +215,7 @@ func TestPasswordUnencodedReservedURLChars(t *testing.T) { } func TestPasswordEncodedReservedURLChars(t *testing.T) { - username := "username" + username := baseUsername schemeAndUsernameAndSep := "database://" + username + ":" basePassword := "password" urlSuffixAndSep := "@localhost:12345/myDB?someParam=true" diff --git a/database/stub/stub_test.go b/database/stub/stub_test.go index f755be6..fdf74c7 100644 --- a/database/stub/stub_test.go +++ b/database/stub/stub_test.go @@ -30,6 +30,9 @@ func TestMigrate(t *testing.T) { stubMigrations.Append(&source.Migration{Version: 1, Direction: source.Down, Identifier: "DROP 1"}) src := &stub.Stub{} srcDrv, err := src.Open("") + if err != nil { + t.Fatal(err) + } srcDrv.(*stub.Stub).Migrations = stubMigrations m, err := migrate.NewWithInstance("stub", srcDrv, "", d) if err != nil { diff --git a/migrate_test.go b/migrate_test.go index 6e3a0bc..1589177 100644 --- a/migrate_test.go +++ b/migrate_test.go @@ -22,6 +22,11 @@ import ( // | u d | - | u | u d | d | - | u d | var sourceStubMigrations *source.Migrations +const ( + srcDrvNameStub = "stub" + dbDrvNameStub = "stub" +) + func init() { sourceStubMigrations = source.NewMigrations() sourceStubMigrations.Append(&source.Migration{Version: 1, Direction: source.Up, Identifier: "CREATE 1"}) @@ -42,14 +47,14 @@ func TestNew(t *testing.T) { t.Fatal(err) } - if m.sourceName != "stub" { + if m.sourceName != srcDrvNameStub { t.Errorf("expected stub, got %v", m.sourceName) } if m.sourceDrv == nil { t.Error("expected sourceDrv not to be nil") } - if m.databaseName != "stub" { + if m.databaseName != dbDrvNameStub { t.Errorf("expected stub, got %v", m.databaseName) } if m.databaseDrv == nil { @@ -77,19 +82,19 @@ func TestNewWithDatabaseInstance(t *testing.T) { t.Fatal(err) } - m, err := NewWithDatabaseInstance("stub://", "stub", dbInst) + m, err := NewWithDatabaseInstance("stub://", dbDrvNameStub, dbInst) if err != nil { t.Fatal(err) } - if m.sourceName != "stub" { + if m.sourceName != srcDrvNameStub { t.Errorf("expected stub, got %v", m.sourceName) } if m.sourceDrv == nil { t.Error("expected sourceDrv not to be nil") } - if m.databaseName != "stub" { + if m.databaseName != dbDrvNameStub { t.Errorf("expected stub, got %v", m.databaseName) } if m.databaseDrv == nil { @@ -132,19 +137,19 @@ func TestNewWithSourceInstance(t *testing.T) { t.Fatal(err) } - m, err := NewWithSourceInstance("stub", sInst, "stub://") + m, err := NewWithSourceInstance(srcDrvNameStub, sInst, "stub://") if err != nil { t.Fatal(err) } - if m.sourceName != "stub" { + if m.sourceName != srcDrvNameStub { t.Errorf("expected stub, got %v", m.sourceName) } if m.sourceDrv == nil { t.Error("expected sourceDrv not to be nil") } - if m.databaseName != "stub" { + if m.databaseName != dbDrvNameStub { t.Errorf("expected stub, got %v", m.databaseName) } if m.databaseDrv == nil { @@ -164,7 +169,7 @@ func ExampleNewWithSourceInstance() { } // Read migrations from Stub and connect to a local postgres database. - m, err := NewWithSourceInstance("stub", instance, "postgres://mattes:secret@localhost:5432/database?sslmode=disable") + m, err := NewWithSourceInstance(srcDrvNameStub, instance, "postgres://mattes:secret@localhost:5432/database?sslmode=disable") if err != nil { log.Fatal(err) } @@ -188,19 +193,19 @@ func TestNewWithInstance(t *testing.T) { t.Fatal(err) } - m, err := NewWithInstance("stub", sInst, "stub", dbInst) + m, err := NewWithInstance(srcDrvNameStub, sInst, dbDrvNameStub, dbInst) if err != nil { t.Fatal(err) } - if m.sourceName != "stub" { + if m.sourceName != srcDrvNameStub { t.Errorf("expected stub, got %v", m.sourceName) } if m.sourceDrv == nil { t.Error("expected sourceDrv not to be nil") } - if m.databaseName != "stub" { + if m.databaseName != dbDrvNameStub { t.Errorf("expected stub, got %v", m.databaseName) } if m.databaseDrv == nil { @@ -1313,12 +1318,12 @@ func TestLock(t *testing.T) { func migrationsFromChannel(ret chan interface{}) ([]*Migration, error) { slice := make([]*Migration, 0) for r := range ret { - switch r.(type) { + switch t := r.(type) { case error: - return slice, r.(error) + return slice, t case *Migration: - slice = append(slice, r.(*Migration)) + slice = append(slice, t) } } return slice, nil @@ -1330,7 +1335,7 @@ func newMigSeq(migr ...*Migration) migrationSequence { return migr } -func (m *migrationSequence) add(migr ...*Migration) migrationSequence { +func (m *migrationSequence) add(migr ...*Migration) migrationSequence { // nolint:unused *m = append(*m, migr...) return *m } diff --git a/testing/testing_test.go b/testing/testing_test.go index 8217dec..d5e7ce1 100644 --- a/testing/testing_test.go +++ b/testing/testing_test.go @@ -4,7 +4,7 @@ import ( "testing" ) -func ExampleParallelTest(t *testing.T) { +func ExampleParallelTest(t *testing.T) { // nolint:govet var isReady = func(i Instance) bool { // Return true if Instance is ready to run tests. // Don't block here though.