Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add golangci lint to github action workflow #1068

Merged
merged 15 commits into from
Aug 31, 2023
Prev Previous commit
Next Next commit
update based on githubci lint
  • Loading branch information
suchen-sci committed Aug 28, 2023
commit 26ad18a62ad1c0041168974ab6996c852ac95b65
10 changes: 3 additions & 7 deletions pkg/filters/headerlookup/headerlookup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
"net/http"
"os"
"sort"
"sync"
"testing"
"time"

Expand Down Expand Up @@ -79,9 +78,8 @@
assert := assert.New(t)
clusterInstance, _ := createClusterAndSyncer()

var mockMap sync.Map
supervisor := supervisor.NewMock(
nil, clusterInstance, mockMap, mockMap, nil, nil, false, nil, nil)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sync.Map contains lock, can't be copy.

nil, clusterInstance, nil, nil, false, nil, nil)

const validYaml = `
name: headerLookup
Expand Down Expand Up @@ -191,9 +189,8 @@

clusterInstance, syncerChannel := createClusterAndSyncer()

var mockMap sync.Map
supervisor := supervisor.NewMock(
nil, clusterInstance, mockMap, mockMap, nil, nil, false, nil, nil)
nil, clusterInstance, nil, nil, false, nil, nil)

// let's put data to 'foobar'
foobar := `
Expand Down Expand Up @@ -224,7 +221,7 @@
assert.Equal(hdr1, hdr2)
assert.Equal("123456789", hdr1)

hl, err = createHeaderLookup(config, hl, supervisor)

Check failure on line 224 in pkg/filters/headerlookup/headerlookup_test.go

View workflow job for this annotation

GitHub Actions / analysis

ineffectual assignment to err (ineffassign)
ctx, header = prepareCtxAndHeader(t)

// update key-value store
Expand Down Expand Up @@ -274,9 +271,8 @@
headerKey: "user-ext-id"
`
clusterInstance, _ := createClusterAndSyncer()
var mockMap sync.Map
supervisor := supervisor.NewMock(
nil, clusterInstance, mockMap, mockMap, nil, nil, false, nil, nil)
nil, clusterInstance, nil, nil, false, nil, nil)
bobbanana := `
ext-id: "333"
extra-entry: "extra"
Expand Down
7 changes: 3 additions & 4 deletions pkg/filters/proxies/httpproxy/pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package httpproxy

import (
"net/http"
"sync"
"testing"

"github.com/megaease/easegress/v2/pkg/context"
Expand Down Expand Up @@ -93,7 +92,7 @@ servers:
assert.NoError(spec.Validate())

p := &Proxy{}
p.super = supervisor.NewMock(option.New(), nil, sync.Map{}, sync.Map{}, nil,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sync.Map{} cant be copied because of lock.

p.super = supervisor.NewMock(option.New(), nil, nil,
nil, false, nil, nil)
sp := NewServerPool(p, spec, "test")
policies := map[string]resilience.Policy{}
Expand Down Expand Up @@ -135,7 +134,7 @@ servers:
assert.NoError(spec.Validate())

p := &Proxy{}
p.super = supervisor.NewMock(option.New(), nil, sync.Map{}, sync.Map{}, nil,
p.super = supervisor.NewMock(option.New(), nil, nil,
nil, false, nil, nil)
sp := NewServerPool(p, spec, "test")
spCtx := &serverPoolContext{
Expand Down Expand Up @@ -178,7 +177,7 @@ func TestCopyCORSHeaders(t *testing.T) {
dst.Add("X-Dst", "dst")

p := &Proxy{}
p.super = supervisor.NewMock(option.New(), nil, sync.Map{}, sync.Map{}, nil,
p.super = supervisor.NewMock(option.New(), nil, nil,
nil, false, nil, nil)
sp := NewServerPool(p, &ServerPoolSpec{}, "test")
dst = sp.mergeResponseHeader(dst, src)
Expand Down
3 changes: 1 addition & 2 deletions pkg/filters/proxies/httpproxy/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"net/http/httptest"
"os"
"strings"
"sync"
"sync/atomic"
"testing"
"time"
Expand Down Expand Up @@ -58,7 +57,7 @@ func newTestProxy(yamlConfig string, assert *assert.Assertions) *Proxy {

proxy := kind.CreateInstance(spec).(*Proxy)

proxy.super = supervisor.NewMock(option.New(), nil, sync.Map{}, sync.Map{}, nil,
proxy.super = supervisor.NewMock(option.New(), nil, nil,
nil, false, nil, nil)

proxy.Init()
Expand Down
3 changes: 1 addition & 2 deletions pkg/filters/validator/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"net/http"
"os"
"strings"
"sync"
"testing"
"time"

Expand Down Expand Up @@ -536,7 +535,7 @@ basicAuth:
}

supervisor := supervisor.NewMock(
nil, clusterInstance, sync.Map{}, sync.Map{}, nil, nil, false, nil, nil)
nil, clusterInstance, nil, nil, false, nil, nil)

yamlConfig := `
kind: Validator
Expand Down
11 changes: 3 additions & 8 deletions pkg/graceupdate/graceupdate.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,21 +71,16 @@ func NotifySigUsr2(closeCls func(), restartCls func()) error {
// Reset signal usr2 notify
NotifySigUsr2(closeCls, restartCls)
} else {
childdone := make(chan error, 1)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

redundant channel.

go func() {
process, err := os.FindProcess(pid)
if err != nil {
restartCls()
NotifySigUsr2(closeCls, restartCls)
} else {
_, werr := process.Wait()
childdone <- werr
select {
case err := <-childdone:
logger.Errorf("child proc exited: %v", err)
restartCls()
NotifySigUsr2(closeCls, restartCls)
}
logger.Errorf("child proc exited: %v", werr)
restartCls()
NotifySigUsr2(closeCls, restartCls)
}
}()
}
Expand Down
3 changes: 1 addition & 2 deletions pkg/object/httpserver/httpserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package httpserver

import (
"os"
"sync"
"testing"
"time"

Expand All @@ -46,7 +45,7 @@ port: 38081
keepAlive: true
https: false
`
super := supervisor.NewMock(option.New(), nil, sync.Map{}, sync.Map{}, nil,
super := supervisor.NewMock(option.New(), nil, nil,
nil, false, nil, nil)
superSpec, err := super.NewSpec(yamlConfig)
assert.NoError(err)
Expand Down
3 changes: 1 addition & 2 deletions pkg/object/httpserver/runtime_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
package httpserver

import (
"sync"
"testing"
"time"

Expand All @@ -38,7 +37,7 @@ port: 38081
keepAlive: true
https: false
`
super := supervisor.NewMock(option.New(), nil, sync.Map{}, sync.Map{}, nil,
super := supervisor.NewMock(option.New(), nil, nil,
nil, false, nil, nil)
superSpec, err := super.NewSpec(yamlConfig)
assert.NoError(err)
Expand Down
5 changes: 2 additions & 3 deletions pkg/object/mqttproxy/mqtt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -751,10 +751,9 @@ type testServer struct {

func newServer(addr string) *testServer {
mux := http.NewServeMux()
srv := http.Server{Addr: addr, Handler: mux}
ts := &testServer{
mux: mux,
srv: srv,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copy lock.

srv: http.Server{Addr: addr, Handler: mux},
addr: addr,
}
return ts
Expand Down Expand Up @@ -1389,7 +1388,7 @@ func TestMQTTProxy(t *testing.T) {
name: mqtt-proxy
kind: MQTTProxy
`
super := supervisor.NewMock(option.New(), nil, sync.Map{}, sync.Map{}, nil, nil, false, nil, nil)
super := supervisor.NewMock(option.New(), nil, nil, nil, false, nil, nil)
super.Options()
superSpec, err := super.NewSpec(yamlStr)
assert.Nil(err)
Expand Down
21 changes: 8 additions & 13 deletions pkg/supervisor/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,21 @@
package supervisor

import (
"sync"

"github.com/megaease/easegress/v2/pkg/cluster"
"github.com/megaease/easegress/v2/pkg/option"
)

// NewMock return a mock supervisor for testing purpose
func NewMock(options *option.Options, cls cluster.Cluster, businessControllers sync.Map,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove sync.Map.

systemControllers sync.Map, objectRegistry *ObjectRegistry, watcher *ObjectEntityWatcher,
func NewMock(options *option.Options, cls cluster.Cluster, objectRegistry *ObjectRegistry, watcher *ObjectEntityWatcher,
firstHandle bool, firstHandleDone chan struct{}, done chan struct{}) *Supervisor {
return &Supervisor{
options: options,
cls: cls,
businessControllers: businessControllers,
systemControllers: systemControllers,
objectRegistry: objectRegistry,
watcher: watcher,
firstHandle: firstHandle,
firstHandleDone: firstHandleDone,
done: done,
options: options,
cls: cls,
objectRegistry: objectRegistry,
watcher: watcher,
firstHandle: firstHandle,
firstHandleDone: firstHandleDone,
done: done,
}
}

Expand Down
8 changes: 4 additions & 4 deletions pkg/util/prometheushelper/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,13 +150,13 @@ func NewSummary(opt prometheus.SummaryOpts, labels []string) *prometheus.Summary
}

func getAndValidate(metricName string, labels []string) (string, error) {
if ValidateMetricName(metricName) == false {
return "", fmt.Errorf("Invalid metric name: %s", metricName)
if !ValidateMetricName(metricName) {
return "", fmt.Errorf("invalid metric name: %s", metricName)
}

for _, l := range labels {
if ValidateLabelName(l) == false {
return "", fmt.Errorf("Invalid label name: %s", l)
if !ValidateLabelName(l) {
return "", fmt.Errorf("invalid label name: %s", l)
}
}
return metricName, nil
Expand Down
Loading