Fix linter issues (#9)

* added initial golangci config

* fix wsl issues

* improve testcases for TestHub

* remove init funcion

* fix unparam issues

* remove old lint action and start using reviewdog action

.codecov.yml

@@ -1,7 +1,6 @@
-
 codecov:
   notify:
-    require_ci_to_pass: yes
+    require_ci_to_pass: no
 
 coverage:
   precision: 2

.github/workflows/pull_request.yml

@@ -0,0 +1,19 @@
+name: PR
+on: [pull_request]
+jobs:
+  lint:
+    name: golangci-lint
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out code into the Go module directory
+        uses: actions/checkout@v1
+      - name: golangci-lint
+        uses: docker://reviewdog/action-golangci-lint:v1 # Pre-built image
+        # uses: reviewdog/action-golangci-lint@v1 # Build with Dockerfile
+        # uses: docker://reviewdog/action-golangci-lint:v1.0.2 # Can use specific version.
+        # uses: reviewdog/[email protected] # Can use specific version.
+        with:
+          github_token: ${{ secrets.HUB_TOKEN }}
+          # Can pass --config flag to change golangci-lint behavior and target
+          # directory.
+          golangci_lint_flags: "--config=.golangci.yml"

.github/workflows/go-push.yml → .github/workflows/push.yml

@@ -1,4 +1,4 @@
-name: Go
+name: Push
 on: [push]
 jobs:
   build:

.golangci.yml

@@ -0,0 +1,88 @@
+# This file contains all available configuration options
+# with their default values.
+
+# options for analysis running
+run:
+  # timeout for analysis, e.g. 30s, 5m, default is 1m
+  deadline: 10m
+
+# output configuration options
+output:
+  # colored-line-number|line-number|json|tab|checkstyle, default is "colored-line-number"
+  format: colored-line-number
+
+  # print lines of code with issue, default is true
+  print-issued-lnes: true
+
+  # print linter name in the end of issue text, default is true
+  print-linter-name: true
+
+# all available settings of specific linters
+linters-settings:
+  golint:
+    # minimal confidence for issues, default is 0.8
+    min-confidence: 0.8
+  gocyclo:
+    # minimal code complexity to report, 30 by default (but we recommend 10-20)
+    min-complexity: 20
+  maligned:
+    # print struct with more effective memory layout or not, false by default
+    suggest-new: true
+  goimports:
+    local-prefixes: github.com/empregoligado/rabbids
+  depguard:
+    list-type: blacklist
+    include-go-root: false
+    packages:
+      - github.com/davecgh/go-spew/spew
+  lll:
+    # max line length, lines longer will be reported. Default is 120.
+    # '\t' is counted as 1 character by default, and can be changed with the tab-width option
+    line-length: 130
+    # tab width in spaces. Default to 1.
+    tab-width: 4
+  unused:
+    # treat code as a program (not a library) and report unused exported identifiers; default is false.
+    # XXX: if you enable this setting, unused will report a lot of false-positives in text editors:
+    # if it's called for subdir of a project it can't find funcs usages. All text editor integrations
+    # with golangci-lint call it on a directory with the changed file.
+    check-exported: false
+  unparam:
+    # Inspect exported functions, default is false. Set to true if no external program/library imports your code.
+    # XXX: if you enable this setting, unparam will report a lot of false-positives in text editors:
+    # if it's called for subdir of a project it can't find external interfaces. All text editor integrations
+    # with golangci-lint call it on a directory with the changed file.
+    check-exported: false
+  wsl:
+    # If true append is only allowed to be cuddled if appending value is
+    # matching variables, fields or types on line above. Default is true.
+    strict-append: true
+    # Allow calls and assignments to be cuddled as long as the lines have any
+    # matching variables, fields or types. Default is true.
+    allow-assign-and-call: true
+    # Allow multiline assignments to be cuddled. Default is true.
+    allow-multiline-assign: true
+    # Allow declarations (var) to be cuddled.
+    allow-cuddle-declarations: true
+    # Allow trailing comments in ending of blocks
+    allow-trailing-comment: false
+    # Force newlines in end of case at this limit (0 = never).
+    force-case-trailing-whitespace: 0
+
+linters:
+  enable-all: true
+  disable:
+    - maligned
+    - prealloc
+    - gosec
+    - gochecknoglobals
+    - goimports
+    - gomnd
+
+issues:
+  # List of regexps of issue texts to exclude, empty list by default.
+  # But independently from this option we use default exclude patterns,
+  # it can be disabled by `exclude-use-default: false`. To list all
+  # excluded by default patterns execute `golangci-lint run --help`
+  # exclude:
+  # - newHTTP - result 1 (error) is always nil

hub.go

@@ -27,6 +27,7 @@ func (h *Hub) Publish(m Message) {
 	for k, v := range h.fields {
 		m.Fields[k] = v
 	}
+
 	for _, sub := range h.matcher.Lookup(m.Topic()) {
 		sub.Set(m)
 	}
@@ -42,9 +43,11 @@ func (h *Hub) With(f Fields) *Hub {
 	for k, v := range h.fields {
 		hub.fields[k] = v
 	}
+
 	for k, v := range f {
 		hub.fields[k] = v
 	}
+
 	return &hub
 }
 
@@ -79,6 +82,7 @@ func (h *Hub) Close() {
 	for _, s := range subs {
 		h.matcher.Unsubscribe(s)
 	}
+
 	for _, s := range subs {
 		s.subscriber.Close()
 	}

hub_benchmark_test.go

@@ -18,19 +18,22 @@ func BenchmarkPublishOnBlockingSubscribers(b *testing.B) {
 func runBenchmark(b *testing.B, numItems, numThreads, numSubscribers int, blocking bool) {
 	h := New()
 	subs := createSubscribers(h, numSubscribers, blocking)
-	var wgPub, wgSub sync.WaitGroup
-	wgSub.Add(len(subs))
 	itemsToInsert := generateTopics(numThreads, numItems)
+	var wgPub, wgSub sync.WaitGroup
 
+	wgSub.Add(len(subs))
 	processSubscriptionsForBench(subs, &wgSub)
 	b.ResetTimer()
+
 	for i := 0; i < b.N; i++ {
 		wgPub.Add(numThreads)
+
 		for j := 0; j < numThreads; j++ {
 			go func(j int) {
 				for _, key := range itemsToInsert[j] {
 					h.Publish(Message{Name: key})
 				}
+
 				wgPub.Done()
 			}(j)
 		}
@@ -43,16 +46,20 @@ func runBenchmark(b *testing.B, numItems, numThreads, numSubscribers int, blocki
 
 func createSubscribers(h *Hub, qtd int, blocking bool) []Subscription {
 	subs := make([]Subscription, 0, qtd)
+
 	for i := 0; i < qtd; i++ {
 		topic := strconv.Itoa(rand.Intn(10)) + "." + strconv.Itoa(rand.Intn(50)) + ".*"
 		var sub Subscription
+
 		if blocking {
 			sub = h.Subscribe(20, topic)
 		} else {
 			sub = h.NonBlockingSubscribe(20, topic)
 		}
+
 		subs = append(subs, sub)
 	}
+
 	return subs
 }
 
@@ -62,6 +69,7 @@ func processSubscriptionsForBench(subs []Subscription, wg *sync.WaitGroup) {
 			for range s.Receiver {
 
 			}
+
 			wg.Done()
 		}(sub)
 	}

hub_example_test.go

@@ -13,11 +13,14 @@ func ExampleHub() {
 	// the cap param is used to create one buffered channel with cap = 10
 	// If you wan an unbuferred channel use the 0 cap
 	sub := h.Subscribe(10, "account.login.*", "account.changepassword.*")
+
 	wg.Add(1)
+
 	go func(s hub.Subscription) {
 		for msg := range s.Receiver {
 			fmt.Printf("receive msg with topic %s and id %d\n", msg.Name, msg.Fields["id"])
 		}
+
 		wg.Done()
 	}(sub)
 

hub_test.go

@@ -13,64 +13,91 @@ type messageCounter struct {
 	sub Subscription
 }
 
+// nolint:funlen
 func TestHub(t *testing.T) {
 	h := New()
+	defaultMessages := []Message{
+		{Name: "forex.eur"},
+		{Name: "forex"},
+		{Name: "trade.jpy"},
+		{Name: "forex.jpy"},
+		{Name: "trade"},
+		{Name: "trade.usd"},
+		{Name: "forex.usd"},
+		{Name: "trade.eur"},
+	}
+	testCases := []struct {
+		name          string
+		messages      []Message
+		subFN         func(h *Hub) Subscription
+		ExpectedCount int
+	}{
+		{
+			name:          "simple subscription unbuffered",
+			messages:      defaultMessages,
+			subFN:         func(h *Hub) Subscription { return h.Subscribe(0, "forex.*") },
+			ExpectedCount: 3,
+		},
+		{
+			name:          "simple subscription buffered",
+			messages:      defaultMessages,
+			subFN:         func(h *Hub) Subscription { return h.Subscribe(2, "*.usd") },
+			ExpectedCount: 2,
+		},
+		{
+			name:          "simple subscription with an invalid cap buffer",
+			messages:      defaultMessages,
+			subFN:         func(h *Hub) Subscription { return h.Subscribe(-1, "forex", "forex.eur", "forex.*") },
+			ExpectedCount: 4,
+		},
+		{
+			name:          "non blocking subscription unbuffered",
+			messages:      defaultMessages,
+			subFN:         func(h *Hub) Subscription { return h.NonBlockingSubscribe(0, "*.eur", "trade") },
+			ExpectedCount: 3,
+		},
+		{
+			name:          "non blocking subscription buffered",
+			messages:      defaultMessages,
+			subFN:         func(h *Hub) Subscription { return h.NonBlockingSubscribe(2, "forex", "forex.eur", "forex.*") },
+			ExpectedCount: 4,
+		},
+		{
+			name:          "non blocking subscription with an invalid cap buffer",
+			messages:      defaultMessages,
+			subFN:         func(h *Hub) Subscription { return h.NonBlockingSubscribe(-1, "trade") },
+			ExpectedCount: 1,
+		},
+		{
+			name:          "get all the messages",
+			messages:      defaultMessages,
+			subFN:         func(h *Hub) Subscription { return h.NonBlockingSubscribe(0, "*", "*.*") },
+			ExpectedCount: 8,
+		},
+	}
 
-	sub0 := h.Subscribe(0, "forex.*")
-	sub1 := h.Subscribe(10, "*.usd")
-	sub2 := h.Subscribe(-10, "forex.eur", "forex.*")
-	sub3 := h.NonBlockingSubscribe(0, "*.eur", "trade")
-	sub4 := h.NonBlockingSubscribe(10, "forex.*")
-	sub5 := h.NonBlockingSubscribe(-10, "trade")
-	sub6 := h.Subscribe(10, "*")
-
-	c0 := newMessageCounter(sub0)
-	c1 := newMessageCounter(sub1)
-	c2 := newMessageCounter(sub2)
-	c3 := newMessageCounter(sub3)
-	c4 := newMessageCounter(sub4)
-	c5 := newMessageCounter(sub5)
-	c6 := newMessageCounter(sub6)
-
-	h.Publish(Message{Name: "forex.eur"})
-	h.Publish(Message{Name: "forex"})
-	h.Publish(Message{Name: "trade.jpy"})
-	h.Publish(Message{Name: "forex.jpy"})
-	h.Publish(Message{Name: "trade"})
-
-	time.Sleep(time.Millisecond)
-
-	require.Equal(t, int64(2), c0.count(), "Messages processed by sub0")
-	require.Equal(t, int64(0), c1.count(), "Messages processed by sub1")
-	require.Equal(t, int64(2), c2.count(), "Messages processed by sub2")
-	require.Equal(t, int64(2), c3.count(), "Messages processed by sub3")
-	require.Equal(t, int64(2), c4.count(), "Messages processed by sub4")
-	require.Equal(t, int64(1), c5.count(), "Messages processed by sub5")
-	require.Equal(t, int64(2), c6.count(), "Messages processed by sub6")
-
-	c0.reset()
-	c1.reset()
-	c2.reset()
-	c3.reset()
-	c4.reset()
-	c5.reset()
-	c6.reset()
-
-	h.Close()
-
-	h.Publish(Message{Name: "forex.eur"})
-	h.Publish(Message{Name: "forex"})
-	h.Publish(Message{Name: "trade.jpy"})
-	h.Publish(Message{Name: "forex.jpy"})
-	h.Publish(Message{Name: "trade"})
-
-	require.Equal(t, int64(0), c0.count(), "Messages processed by sub0")
-	require.Equal(t, int64(0), c1.count(), "Messages processed by sub1")
-	require.Equal(t, int64(0), c2.count(), "Messages processed by sub2")
-	require.Equal(t, int64(0), c3.count(), "Messages processed by sub3")
-	require.Equal(t, int64(0), c4.count(), "Messages processed by sub4")
-	require.Equal(t, int64(0), c5.count(), "Messages processed by sub5")
-	require.Equal(t, int64(0), c6.count(), "Messages processed by sub6")
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			sub := tc.subFN(h)
+			counter := newMessageCounter(sub)
+			for _, m := range tc.messages {
+				time.Sleep(time.Millisecond)
+				h.Publish(m)
+			}
+
+			time.Sleep(time.Second)
+			require.EqualValues(t, tc.ExpectedCount, counter.count())
+
+			counter.reset()
+			h.Close()
+
+			for _, m := range tc.messages {
+				h.Publish(m)
+			}
+			require.EqualValues(t, 0, counter.count(), "after close the hub all the subsctibers MUST not get more events")
+		})
+	}
 }
 
 func TestNonBlockingSubscriberShouldAlertIfLoseMessages(t *testing.T) {
@@ -81,6 +108,7 @@ func TestNonBlockingSubscriberShouldAlertIfLoseMessages(t *testing.T) {
 	for i := 0; i < 11; i++ {
 		h.Publish(Message{Name: "a.c.c", Fields: Fields{"i": i}})
 	}
+
 	msg := <-subsAlert.Receiver
 	require.Equal(t, 1, msg.Fields["missed"])
 	require.Equal(t, []string{"a.*.c"}, msg.Fields["topic"])
@@ -110,6 +138,7 @@ func TestWith(t *testing.T) {
 		"something": 123,
 		"field":     456,
 	}, msg.Fields)
+
 	msg = <-subs.Receiver
 	require.Equal(t, Fields{"msg": 4, "hub": "subH2", "something": 789}, msg.Fields)
 }
@@ -121,6 +150,7 @@ func newMessageCounter(s Subscription) *messageCounter {
 			atomic.AddInt64(&ms.c, 1)
 		}
 	}(ms)
+
 	return ms
 }