From 55898aa60042be0a0fd565ab950ff9bdbe4dfbd0 Mon Sep 17 00:00:00 2001
From: DownerCase <119755054+DownerCase@users.noreply.github.com>
Date: Wed, 18 Dec 2024 14:51:00 +0000
Subject: [PATCH] CI: Enable and fix checks - Pt3 (#33)

* fix: funlen

* fix: gochecknoglobals

* fix: nonamedreturns

* fix: testpackage
---
 .golangci.yaml                                | 22 +++++++------
 cmd/monitor/common.go                         |  2 +-
 cmd/monitor/config_page.go                    |  4 ++-
 cmd/monitor/hosts_list.go                     |  5 +--
 cmd/monitor/page_processes_main.go            |  6 ++--
 cmd/monitor/page_services_main.go             |  6 ++--
 ecal/core_test.go                             | 32 ++++++++++---------
 ecal/logging/logging_test.go                  | 13 ++++----
 ecal/monitoring/callback.go                   | 12 +++----
 ecal/monitoring/monitoring_test.go            | 17 +++++-----
 .../subscriber/protobuf_subscriber_test.go    |  7 ++--
 .../subscriber/string_subscriber_test.go      | 10 +++---
 ecal/subscriber/subscriber_test.go            |  7 +++-
 internal/protobuf/protobuf_test.go            |  5 +--
 main.go                                       | 24 +++++++++-----
 15 files changed, 99 insertions(+), 73 deletions(-)

diff --git a/.golangci.yaml b/.golangci.yaml
index aba326f..86eb30b 100644
--- a/.golangci.yaml
+++ b/.golangci.yaml
@@ -10,13 +10,14 @@ linters:
     - godox # Yes. The project is in progress
     - depguard # Not desired
     - varnamelen # Not desired
-    - gochecknoglobals
-    - nonamedreturns
-    - testpackage
-    - cyclop
-    - funlen
+    - cyclop # Flags big simple switch statements
 
 linters-settings:
+  gci:
+    sections:
+      - standard
+      - default
+      - localmodule
   ireturn:
     allow:
       - tea.Model
@@ -24,11 +25,6 @@ linters-settings:
       - stdlib
       - empty
       - generic
-  gci:
-    sections:
-      - standard
-      - default
-      - localmodule
   mnd:
     ignored-files:
       - 'monitor/.*.go' # UI code has many magic layout numbers...
@@ -36,3 +32,9 @@ linters-settings:
   paralleltest:
     # eCAL uses global state, any test using Initialize can't be run in parallel
     ignore-missing: true
+
+issues:
+  exclude-rules:
+    - path: monitor/styles.go
+      linters:
+        - gochecknoglobals
diff --git a/cmd/monitor/common.go b/cmd/monitor/common.go
index 65f2aaf..03af39c 100644
--- a/cmd/monitor/common.go
+++ b/cmd/monitor/common.go
@@ -26,7 +26,7 @@ func NewTable(columns []table.Column) table.Model {
 
 type NavKeyMap map[string]func() tea.Cmd
 
-func (navKeys NavKeyMap) HandleMsg(msg tea.Msg) (cmd tea.Cmd, navigated bool) {
+func (navKeys NavKeyMap) HandleMsg(msg tea.Msg) (tea.Cmd, bool) {
 	if msg, ok := msg.(tea.KeyMsg); ok {
 		if f, ok := navKeys[msg.String()]; ok {
 			return f(), true
diff --git a/cmd/monitor/config_page.go b/cmd/monitor/config_page.go
index 105dd96..5715e31 100644
--- a/cmd/monitor/config_page.go
+++ b/cmd/monitor/config_page.go
@@ -23,8 +23,10 @@ func NewConfigModel() *ModelConfig {
 
 func (m *ModelConfig) Refresh() {}
 
-func (m *ModelConfig) Update(msg tea.Msg) (cmd tea.Cmd) {
+func (m *ModelConfig) Update(msg tea.Msg) tea.Cmd {
+	var cmd tea.Cmd
 	m.viewport, cmd = m.viewport.Update(msg)
+
 	return cmd
 }
 
diff --git a/cmd/monitor/hosts_list.go b/cmd/monitor/hosts_list.go
index 9cd2315..b6ebd2c 100644
--- a/cmd/monitor/hosts_list.go
+++ b/cmd/monitor/hosts_list.go
@@ -87,7 +87,8 @@ func (m *ModelHosts) updateTable(msg tea.Msg) {
 	m.table, _ = m.table.Update(msg)
 }
 
-func hostsToRows(hosts map[string]hostInfo) (rows []table.Row) {
+func hostsToRows(hosts map[string]hostInfo) []table.Row {
+	rows := make([]table.Row, 0)
 	for hostName, hostInfo := range hosts {
 		rows = append(rows, table.Row{
 			hostName,
@@ -99,5 +100,5 @@ func hostsToRows(hosts map[string]hostInfo) (rows []table.Row) {
 		})
 	}
 
-	return
+	return rows
 }
diff --git a/cmd/monitor/page_processes_main.go b/cmd/monitor/page_processes_main.go
index f20be55..757925f 100644
--- a/cmd/monitor/page_processes_main.go
+++ b/cmd/monitor/page_processes_main.go
@@ -51,7 +51,7 @@ func (m *ModelProcessesMain) getSelectedPid() (int32, error) {
 	return int32(pid), err
 }
 
-func (m *ModelProcessesMain) updateTable(msg tea.Msg) (cmd tea.Cmd) {
+func (m *ModelProcessesMain) updateTable(msg tea.Msg) tea.Cmd {
 	rows := []table.Row{}
 
 	mon := monitoring.GetMonitoring(monitoring.MonitorProcess)
@@ -59,10 +59,12 @@ func (m *ModelProcessesMain) updateTable(msg tea.Msg) (cmd tea.Cmd) {
 		rows = append(rows, procToRow(proc))
 	}
 
+	var cmd tea.Cmd
+
 	m.table.SetRows(rows)
 	m.table, cmd = m.table.Update(msg)
 
-	return
+	return cmd
 }
 
 func procToRow(proc monitoring.ProcessMon) table.Row {
diff --git a/cmd/monitor/page_services_main.go b/cmd/monitor/page_services_main.go
index c0a82c4..820256f 100644
--- a/cmd/monitor/page_services_main.go
+++ b/cmd/monitor/page_services_main.go
@@ -48,7 +48,7 @@ func (m *ModelServicesMain) GetSelectedID() (string, bool, error) {
 	return row[0], row[1] == "S", nil
 }
 
-func (m *ModelServicesMain) updateTable(msg tea.Msg) (cmd tea.Cmd) {
+func (m *ModelServicesMain) updateTable(msg tea.Msg) tea.Cmd {
 	rows := []table.Row{}
 
 	mon := monitoring.GetMonitoring(monitoring.MonitorClient | monitoring.MonitorServer)
@@ -60,10 +60,12 @@ func (m *ModelServicesMain) updateTable(msg tea.Msg) (cmd tea.Cmd) {
 		rows = append(rows, serverToRow(server))
 	}
 
+	var cmd tea.Cmd
+
 	m.table.SetRows(rows)
 	m.table, cmd = m.table.Update(msg)
 
-	return
+	return cmd
 }
 
 func serviceToRow(service monitoring.ServiceBase) table.Row {
diff --git a/ecal/core_test.go b/ecal/core_test.go
index 12c4239..cb86b41 100644
--- a/ecal/core_test.go
+++ b/ecal/core_test.go
@@ -1,13 +1,15 @@
-package ecal
+package ecal_test
 
 import (
 	"testing"
+
+	"github.com/DownerCase/ecal-go/ecal"
 )
 
 func TestVersionString(t *testing.T) {
 	t.Parallel()
 
-	version := GetVersionString()
+	version := ecal.GetVersionString()
 	if version == "" {
 		t.Error("GetVersionString returned empty string")
 	}
@@ -18,7 +20,7 @@ func TestVersionString(t *testing.T) {
 func TestVersionDateString(t *testing.T) {
 	t.Parallel()
 
-	buildDate := GetVersionDateString()
+	buildDate := ecal.GetVersionDateString()
 	if buildDate == "" {
 		t.Error("GetVersionDateString returned empty string")
 	}
@@ -29,16 +31,16 @@ func TestVersionDateString(t *testing.T) {
 func TestGetVersion(t *testing.T) {
 	t.Parallel()
 
-	version := GetVersion()
+	version := ecal.GetVersion()
 	t.Log(version)
 }
 
 func TestInitializeFinalize(t *testing.T) {
-	if IsInitialized() {
+	if ecal.IsInitialized() {
 		t.Error("eCAL pre-initialized...")
 	}
 
-	initResult := Initialize(NewConfig(), "go_test", CDefault)
+	initResult := ecal.Initialize(ecal.NewConfig(), "go_test", ecal.CDefault)
 	if initResult == 1 {
 		t.Fatal("eCAL already initialized")
 	} else if initResult != 0 {
@@ -46,44 +48,44 @@ func TestInitializeFinalize(t *testing.T) {
 	}
 
 	// Test double initialization
-	secondInit := Initialize(NewConfig(), "go_test2", CPublisher)
+	secondInit := ecal.Initialize(ecal.NewConfig(), "go_test2", ecal.CPublisher)
 	if secondInit != 1 {
 		t.Errorf("Second initialize returned %v", secondInit)
 	}
 
-	if !IsInitialized() {
+	if !ecal.IsInitialized() {
 		t.Error("IsInitialized return false, expected true")
 	}
 
-	if !IsComponentInitialized(CPublisher) {
+	if !ecal.IsComponentInitialized(ecal.CPublisher) {
 		t.Error("Expected publisheCPublisher to be initialised")
 	}
 
-	if !SetUnitName("go_test_set_name") {
+	if !ecal.SetUnitName("go_test_set_name") {
 		t.Error("Failed to set unit name")
 	}
 
-	if !Ok() {
+	if !ecal.Ok() {
 		t.Error("eCAL not Ok")
 	}
 
-	finalizeResult := Finalize()
+	finalizeResult := ecal.Finalize()
 	if finalizeResult != 0 {
 		t.Errorf("Failed to finalize with error %v", finalizeResult)
 	}
 
-	secondFinalize := Finalize()
+	secondFinalize := ecal.Finalize()
 	// We've called Initialize twice so 2 calls to Finalize are needed
 	if secondFinalize != 0 {
 		t.Errorf("Second finalize returned %v", secondFinalize)
 	}
 
-	thirdFinalize := Finalize()
+	thirdFinalize := ecal.Finalize()
 	if thirdFinalize != 1 {
 		t.Errorf("Expected Finalize to already be done, recevied %v", thirdFinalize)
 	}
 
-	if Ok() {
+	if ecal.Ok() {
 		t.Error("eCAL Ok after being finalized")
 	}
 }
diff --git a/ecal/logging/logging_test.go b/ecal/logging/logging_test.go
index cc1f76d..22bff17 100644
--- a/ecal/logging/logging_test.go
+++ b/ecal/logging/logging_test.go
@@ -1,4 +1,4 @@
-package logging
+package logging_test
 
 import (
 	"os"
@@ -6,11 +6,12 @@ import (
 	"time"
 
 	"github.com/DownerCase/ecal-go/ecal"
+	"github.com/DownerCase/ecal-go/ecal/logging"
 	_ "github.com/DownerCase/ecal-go/ecal/types"
 	"github.com/DownerCase/ecal-go/internal/ecaltest"
 )
 
-func expectMessageIsFromHost(t *testing.T, msg LogMessage) {
+func expectMessageIsFromHost(t *testing.T, msg logging.LogMessage) {
 	t.Helper()
 
 	host, err := os.Hostname()
@@ -23,10 +24,10 @@ func expectMessageIsFromHost(t *testing.T, msg LogMessage) {
 	}
 }
 
-func receiveMessage(t *testing.T, msg string, level Level) bool {
+func receiveMessage(t *testing.T, msg string, level logging.Level) bool {
 	t.Helper()
 
-	logs := GetLogging()
+	logs := logging.GetLogging()
 
 	for _, rmsg := range logs.Messages {
 		if rmsg.Content == msg {
@@ -50,8 +51,8 @@ func TestGetLogging(t *testing.T) {
 
 	// When: Logging a message
 	testMessage := "This is a test log message"
-	level := LevelError
-	Log(level, testMessage)
+	level := logging.LevelError
+	logging.Log(level, testMessage)
 
 	// Expect: To receieve that message
 	time.Sleep(5 * time.Millisecond)
diff --git a/ecal/monitoring/callback.go b/ecal/monitoring/callback.go
index 56174f3..1cb4e01 100644
--- a/ecal/monitoring/callback.go
+++ b/ecal/monitoring/callback.go
@@ -88,8 +88,8 @@ func copyToServiceBase(cbase C.struct_CServiceCommon) ServiceBase {
 	}
 }
 
-func copyToServerMons(cservers []C.struct_CServerMon) (servers []ServerMon) {
-	servers = make([]ServerMon, len(cservers))
+func copyToServerMons(cservers []C.struct_CServerMon) []ServerMon {
+	servers := make([]ServerMon, len(cservers))
 	for idx, cserver := range cservers {
 		servers[idx] = ServerMon{
 			ServiceBase: copyToServiceBase(cserver.base),
@@ -97,17 +97,17 @@ func copyToServerMons(cservers []C.struct_CServerMon) (servers []ServerMon) {
 			PortV1:      uint32(cserver.port_v1),
 		}
 	}
-	return
+	return servers
 }
 
-func copyToClientMons(cclients []C.struct_CClientMon) (clients []ClientMon) {
-	clients = make([]ClientMon, len(cclients))
+func copyToClientMons(cclients []C.struct_CClientMon) []ClientMon {
+	clients := make([]ClientMon, len(cclients))
 	for idx, cclient := range cclients {
 		clients[idx] = ClientMon{
 			ServiceBase: copyToServiceBase(cclient.base),
 		}
 	}
-	return
+	return clients
 }
 
 //export goCopyMonitoring
diff --git a/ecal/monitoring/monitoring_test.go b/ecal/monitoring/monitoring_test.go
index b573e8f..128a510 100644
--- a/ecal/monitoring/monitoring_test.go
+++ b/ecal/monitoring/monitoring_test.go
@@ -1,4 +1,4 @@
-package monitoring
+package monitoring_test
 
 import (
 	"os"
@@ -6,6 +6,7 @@ import (
 	"time"
 
 	"github.com/DownerCase/ecal-go/ecal"
+	"github.com/DownerCase/ecal-go/ecal/monitoring"
 	"github.com/DownerCase/ecal-go/ecal/registration"
 	"github.com/DownerCase/ecal-go/internal/ecaltest"
 	"github.com/DownerCase/ecal-go/internal/ecaltest/regtest"
@@ -13,7 +14,7 @@ import (
 	testutilsubscriber "github.com/DownerCase/ecal-go/internal/ecaltest/testutil_subscriber"
 )
 
-func expectTopicPresent(t *testing.T, ts []TopicMon, topicName string) {
+func expectTopicPresent(t *testing.T, ts []monitoring.TopicMon, topicName string) {
 	t.Helper()
 
 	if len(ts) == 0 {
@@ -41,7 +42,7 @@ func TestPublisherMonitoring(t *testing.T) {
 	pub := testutilpublisher.NewGenericPublisher(t, topic)
 	defer pub.Delete()
 
-	mon := GetMonitoring(MonitorHost)
+	mon := monitoring.GetMonitoring(monitoring.MonitorHost)
 	if len(mon.Publishers) > 0 {
 		t.Error("Monitoring returned publishers when not requested")
 	}
@@ -50,11 +51,11 @@ func TestPublisherMonitoring(t *testing.T) {
 	regtest.ExpectNew(t, topic, channel)
 
 	// Get its info
-	mon = GetMonitoring(MonitorPublisher)
+	mon = monitoring.GetMonitoring(monitoring.MonitorPublisher)
 	expectTopicPresent(t, mon.Publishers, topic)
 }
 
-func expectPid(t *testing.T, pid int, procs []ProcessMon) {
+func expectPid(t *testing.T, pid int, procs []monitoring.ProcessMon) {
 	t.Helper()
 
 	hostname, err := os.Hostname()
@@ -87,7 +88,7 @@ func TestSubscriberMonitoring(t *testing.T) {
 	sub := testutilsubscriber.NewGenericSubscriber(t, topic)
 	defer sub.Delete()
 
-	mon := GetMonitoring(MonitorHost)
+	mon := monitoring.GetMonitoring(monitoring.MonitorHost)
 	if len(mon.Publishers) > 0 {
 		t.Error("Monitoring returned publishers when not requested")
 	}
@@ -96,7 +97,7 @@ func TestSubscriberMonitoring(t *testing.T) {
 	regtest.ExpectNew(t, topic, channel)
 
 	// Get its info
-	mon = GetMonitoring(MonitorSubscriber)
+	mon = monitoring.GetMonitoring(monitoring.MonitorSubscriber)
 	expectTopicPresent(t, mon.Subscribers, topic)
 }
 
@@ -109,7 +110,7 @@ func TestProcessMonitoring(t *testing.T) {
 	time.Sleep(1500 * time.Millisecond) // Propagation delay...
 
 	// When: Requesting the processes
-	mon := GetMonitoring(MonitorProcess)
+	mon := monitoring.GetMonitoring(monitoring.MonitorProcess)
 
 	// Expect: This program
 	expectPid(t, os.Getpid(), mon.Processes)
diff --git a/ecal/protobuf/subscriber/protobuf_subscriber_test.go b/ecal/protobuf/subscriber/protobuf_subscriber_test.go
index bcdb054..86b6506 100644
--- a/ecal/protobuf/subscriber/protobuf_subscriber_test.go
+++ b/ecal/protobuf/subscriber/protobuf_subscriber_test.go
@@ -1,4 +1,4 @@
-package subscriber
+package subscriber_test
 
 import (
 	"testing"
@@ -6,15 +6,16 @@ import (
 
 	"github.com/DownerCase/ecal-go/ecal"
 	"github.com/DownerCase/ecal-go/ecal/protobuf/publisher"
+	"github.com/DownerCase/ecal-go/ecal/protobuf/subscriber"
 	"github.com/DownerCase/ecal-go/internal/ecaltest"
 	testutilpublisher "github.com/DownerCase/ecal-go/internal/ecaltest/protobuf/testutil_publisher"
 	"github.com/DownerCase/ecal-go/protos"
 )
 
-func newSubscriber[U any, T Msg[U]](t *testing.T, topic string) *Subscriber[U, T] {
+func newSubscriber[U any, T subscriber.Msg[U]](t *testing.T, topic string) *subscriber.Subscriber[U, T] {
 	t.Helper()
 
-	sub, err := New[U, T]()
+	sub, err := subscriber.New[U, T]()
 	if err != nil {
 		t.Error(err)
 	}
diff --git a/ecal/string/subscriber/string_subscriber_test.go b/ecal/string/subscriber/string_subscriber_test.go
index 7b45d48..577dc84 100644
--- a/ecal/string/subscriber/string_subscriber_test.go
+++ b/ecal/string/subscriber/string_subscriber_test.go
@@ -1,4 +1,4 @@
-package subscriber
+package subscriber_test
 
 import (
 	"reflect"
@@ -7,16 +7,17 @@ import (
 
 	"github.com/DownerCase/ecal-go/ecal"
 	"github.com/DownerCase/ecal-go/ecal/string/publisher"
+	"github.com/DownerCase/ecal-go/ecal/string/subscriber"
 	"github.com/DownerCase/ecal-go/internal/ecaltest"
 	testutilpublisher "github.com/DownerCase/ecal-go/internal/ecaltest/string/testutil_publisher"
 )
 
 const TestMessage = "Test string"
 
-func newSubscriber(t *testing.T, topic string) *Subscriber {
+func newSubscriber(t *testing.T, topic string) *subscriber.Subscriber {
 	t.Helper()
 
-	sub, err := New()
+	sub, err := subscriber.New()
 	if err != nil {
 		t.Error(err)
 	}
@@ -28,9 +29,6 @@ func newSubscriber(t *testing.T, topic string) *Subscriber {
 	return sub
 }
 
-// Export for testing.
-var NewSubscriber = newSubscriber
-
 func TestSubscriber(t *testing.T) {
 	ecaltest.InitEcal(t)
 
diff --git a/ecal/subscriber/subscriber_test.go b/ecal/subscriber/subscriber_test.go
index 10d8d4c..4dbc1a9 100644
--- a/ecal/subscriber/subscriber_test.go
+++ b/ecal/subscriber/subscriber_test.go
@@ -12,7 +12,9 @@ import (
 	testutilsubscriber "github.com/DownerCase/ecal-go/internal/ecaltest/testutil_subscriber"
 )
 
-var TestMessage = []byte{4, 15, 80}
+func GetTestMessage() []byte {
+	return []byte{4, 15, 80}
+}
 
 func TestSubscriber(t *testing.T) {
 	ecaltest.InitEcal(t)
@@ -27,6 +29,8 @@ func TestSubscriber(t *testing.T) {
 
 	go sendMessages(pub)
 
+	TestMessage := GetTestMessage()
+
 	for range 10 {
 		// TODO: Reduce the propagation delay for when the subscriber gets
 		// connected to the publisher
@@ -64,6 +68,7 @@ func TestSubscriberTimeout(t *testing.T) {
 }
 
 func sendMessages(p *publisher.Publisher) {
+	TestMessage := GetTestMessage()
 	for !p.IsStopped() {
 		p.Messages <- TestMessage
 
diff --git a/internal/protobuf/protobuf_test.go b/internal/protobuf/protobuf_test.go
index 51b5ba8..7e77c38 100644
--- a/internal/protobuf/protobuf_test.go
+++ b/internal/protobuf/protobuf_test.go
@@ -1,8 +1,9 @@
-package protobuf
+package protobuf_test
 
 import (
 	"testing"
 
+	"github.com/DownerCase/ecal-go/internal/protobuf"
 	"github.com/DownerCase/ecal-go/protos"
 )
 
@@ -10,7 +11,7 @@ func TestFullName(t *testing.T) {
 	t.Parallel()
 
 	expectedName := "pb.People.Person"
-	if fn := GetFullName(&protos.Person{}); fn != expectedName {
+	if fn := protobuf.GetFullName(&protos.Person{}); fn != expectedName {
 		t.Error("Expected: ", expectedName, " Actual: ", fn)
 	}
 }
diff --git a/main.go b/main.go
index d2c9416..9e7868e 100644
--- a/main.go
+++ b/main.go
@@ -51,12 +51,6 @@ func main() {
 	}
 	defer pub.Delete() // Don't forget to delete the publisher when done!
 
-	person := &protos.Person{
-		Id: 0, Name: "John", Email: "john@doe.net",
-		Dog:   &protos.Dog{Name: "Pluto"},
-		House: &protos.House{Rooms: 5},
-	}
-
 	if pub.Create("person") != nil {
 		panic("Failed to Create protobuf publisher")
 	}
@@ -73,7 +67,21 @@ func main() {
 
 	go receiveMessages(sub)
 
-	for idx := range int32(100) {
+	sendMessages(100, stringPublisher, pub)
+}
+
+func sendMessages(
+	numToSend int32,
+	stringPublisher *string_publisher.Publisher,
+	pub *publisher.Publisher[*protos.Person],
+) {
+	person := &protos.Person{
+		Id: 0, Name: "John", Email: "john@doe.net",
+		Dog:   &protos.Dog{Name: "Pluto"},
+		House: &protos.House{Rooms: 5},
+	}
+
+	for idx := range numToSend {
 		// Check if program has been requested to stop
 		if !ecal.Ok() {
 			logging.Warn("eCAL.Ok() is false; shutting down")
@@ -90,7 +98,7 @@ func main() {
 			logging.Error(err)
 		}
 
-		if err = stringPublisher.Send("Message ", idx); err != nil {
+		if err := stringPublisher.Send("Message ", idx); err != nil {
 			logging.Error(err)
 		}