Skip to content

Commit 0ce54b5

Browse files
JosephKavJosephKav
authored
refactor(go): stop using pointers to channels (#728)
- `chan` is already a reference type; `*chan` provides no benefit
1 parent 556abb8 commit 0ce54b5

47 files changed

Lines changed: 204 additions & 205 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎command/announce_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ func TestController_AnnounceCommand(t *testing.T) {
7676
test.StringPtr("11m"))
7777
if !tc.nilChannel {
7878
announceChannel := make(chan []byte, 4)
79-
controller.ServiceStatus.AnnounceChannel = &announceChannel
79+
controller.ServiceStatus.AnnounceChannel = announceChannel
8080
}
8181
if tc.failed != nil {
8282
controller.Failed.Set(tc.index, *tc.failed)
@@ -90,7 +90,7 @@ func TestController_AnnounceCommand(t *testing.T) {
9090
if controller.ServiceStatus.AnnounceChannel == nil {
9191
return
9292
}
93-
m := <-*controller.ServiceStatus.AnnounceChannel
93+
m := <-controller.ServiceStatus.AnnounceChannel
9494
var parsed apitype.WebSocketMessage
9595
json.Unmarshal(m, &parsed)
9696

‎command/command_test.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -114,10 +114,10 @@ func TestCommand_Exec(t *testing.T) {
114114

115115
func TestController_ExecIndex(t *testing.T) {
116116
// GIVEN a Controller with different Commands to execute.
117-
announce := make(chan []byte, 8)
117+
announceChannel := make(chan []byte, 8)
118118
controller := Controller{}
119119
svcStatus := status.New(
120-
&announce, nil, nil,
120+
announceChannel, nil, nil,
121121
"",
122122
"", "",
123123
"", "",
@@ -179,9 +179,9 @@ func TestController_ExecIndex(t *testing.T) {
179179
if !tc.noAnnounce {
180180
runNumber++
181181
}
182-
if len(announce) != runNumber {
182+
if len(announceChannel) != runNumber {
183183
t.Fatalf("%s\nCommand run not announced\nwant: %d\ngot: %d",
184-
packageName, runNumber, len(announce))
184+
packageName, runNumber, len(announceChannel))
185185
}
186186
})
187187
}
@@ -220,8 +220,8 @@ func TestController_Exec(t *testing.T) {
220220
// t.Parallel() - Cannot run in parallel since we're using stdout.
221221
releaseStdout := test.CaptureStdout()
222222

223-
announce := make(chan []byte, 8)
224-
controller := testController(&announce)
223+
announceChannel := make(chan []byte, 8)
224+
controller := testController(announceChannel)
225225

226226
// WHEN the Command @index is executed.
227227
controller.Command = tc.commands
@@ -247,9 +247,9 @@ func TestController_Exec(t *testing.T) {
247247
if !tc.noAnnounce {
248248
runNumber = len(*controller.Command)
249249
}
250-
if len(announce) != runNumber {
250+
if len(announceChannel) != runNumber {
251251
t.Fatalf("%s\nCommand run not announced\nwant: %d\ngot: %d",
252-
packageName, runNumber, len(announce))
252+
packageName, runNumber, len(announceChannel))
253253
}
254254
})
255255
}

‎command/help_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,10 @@ func TestMain(m *testing.M) {
3939
os.Exit(exitCode)
4040
}
4141

42-
func testController(announce *chan []byte) (control *Controller) {
42+
func testController(announceChannel chan []byte) (control *Controller) {
4343
control = &Controller{}
4444
svcStatus := status.New(
45-
announce, nil, nil,
45+
announceChannel, nil, nil,
4646
"",
4747
"", "",
4848
"", "",

‎config/edit.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,12 +74,12 @@ func (c *Config) AddService(oldServiceID string, newService *service.Service) er
7474

7575
// Trigger a save if the Service has changed.
7676
if changedService {
77-
*c.HardDefaults.Service.Status.SaveChannel <- true
77+
c.HardDefaults.Service.Status.SaveChannel <- true
7878
}
7979

8080
// Update the database if the service is new, or the versions changed.
8181
if changedDB {
82-
*c.HardDefaults.Service.Status.DatabaseChannel <- dbtype.Message{
82+
c.HardDefaults.Service.Status.DatabaseChannel <- dbtype.Message{
8383
ServiceID: newService.ID,
8484
Cells: []dbtype.Cell{
8585
{Column: "latest_version", Value: newService.Status.LatestVersion()},
@@ -129,7 +129,7 @@ func (c *Config) RenameService(oldService string, newService *service.Service) {
129129
c.Order = util.ReplaceElement(c.Order, oldService, newService.ID)
130130
c.Service[newService.ID] = newService
131131
// Rename the primary key for this service in the database.
132-
*c.HardDefaults.Service.Status.DatabaseChannel <- dbtype.Message{
132+
c.HardDefaults.Service.Status.DatabaseChannel <- dbtype.Message{
133133
ServiceID: oldService,
134134
Cells: []dbtype.Cell{
135135
{Column: "id", Value: newService.ID}}}
@@ -162,5 +162,5 @@ func (c *Config) DeleteService(serviceID string) {
162162
delete(c.Service, serviceID)
163163

164164
// Trigger save.
165-
*c.HardDefaults.Service.Status.SaveChannel <- true
165+
c.HardDefaults.Service.Status.SaveChannel <- true
166166
}

‎config/edit_test.go‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -118,11 +118,11 @@ func TestConfig_AddService(t *testing.T) {
118118
packageName, tc.wantOrder, cfg.Order)
119119
}
120120
// AND the DatabaseChannel should have a message waiting if the service was added.
121-
if len(*cfg.HardDefaults.Service.Status.DatabaseChannel) != tc.dbMessages {
121+
if len(cfg.HardDefaults.Service.Status.DatabaseChannel) != tc.dbMessages {
122122
t.Errorf("%s\nDatabaseChannel mismatch\nwant: %d messages\ngot: %d",
123-
packageName, tc.dbMessages, len(*cfg.HardDefaults.Service.Status.DatabaseChannel))
124-
for i := 0; i <= len(*cfg.HardDefaults.Service.Status.DatabaseChannel); i++ {
125-
msg := <-*cfg.HardDefaults.Service.Status.DatabaseChannel
123+
packageName, tc.dbMessages, len(cfg.HardDefaults.Service.Status.DatabaseChannel))
124+
for i := 0; i <= len(cfg.HardDefaults.Service.Status.DatabaseChannel); i++ {
125+
msg := <-cfg.HardDefaults.Service.Status.DatabaseChannel
126126
t.Log(msg)
127127
}
128128
}
@@ -285,11 +285,11 @@ func TestConfig_RenameService(t *testing.T) {
285285
if !tc.fail {
286286
want = 1
287287
}
288-
if len(*cfg.HardDefaults.Service.Status.DatabaseChannel) != want {
288+
if len(cfg.HardDefaults.Service.Status.DatabaseChannel) != want {
289289
t.Errorf("%s\nDatabaseChannel mismatch\nwant: %d messages\ngot: %d",
290-
packageName, want, len(*cfg.HardDefaults.Service.Status.DatabaseChannel))
291-
for i := 0; i <= len(*cfg.HardDefaults.Service.Status.DatabaseChannel); i++ {
292-
msg := <-*cfg.HardDefaults.Service.Status.DatabaseChannel
290+
packageName, want, len(cfg.HardDefaults.Service.Status.DatabaseChannel))
291+
for i := 0; i <= len(cfg.HardDefaults.Service.Status.DatabaseChannel); i++ {
292+
msg := <-cfg.HardDefaults.Service.Status.DatabaseChannel
293293
t.Log(msg)
294294
}
295295
}
@@ -347,11 +347,11 @@ func TestConfig_DeleteService(t *testing.T) {
347347
if tc.dbMessage {
348348
want = 1
349349
}
350-
if len(*cfg.HardDefaults.Service.Status.DatabaseChannel) != want {
350+
if len(cfg.HardDefaults.Service.Status.DatabaseChannel) != want {
351351
t.Errorf("%s\nDatabaseChannel mismatch\nwant: %d messages\ngot: %d",
352-
packageName, want, len(*cfg.HardDefaults.Service.Status.DatabaseChannel))
353-
for i := 0; i <= len(*cfg.HardDefaults.Service.Status.DatabaseChannel); i++ {
354-
msg := <-*cfg.HardDefaults.Service.Status.DatabaseChannel
352+
packageName, want, len(cfg.HardDefaults.Service.Status.DatabaseChannel))
353+
for i := 0; i <= len(cfg.HardDefaults.Service.Status.DatabaseChannel); i++ {
354+
msg := <-cfg.HardDefaults.Service.Status.DatabaseChannel
355355
t.Log(msg)
356356
}
357357
}

‎config/help_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,9 @@ func testConfig() Config {
5858
HardDefaults: Defaults{
5959
Service: service.Defaults{
6060
Status: status.NewDefaults(
61-
nil, &databaseChannel, &saveChannel)}},
62-
DatabaseChannel: &databaseChannel,
63-
SaveChannel: &saveChannel,
61+
nil, databaseChannel, saveChannel)}},
62+
DatabaseChannel: databaseChannel,
63+
SaveChannel: saveChannel,
6464
}
6565
}
6666

@@ -118,11 +118,11 @@ func testLoadBasic(file string, t *testing.T) *Config {
118118
logutil.LogFrom{}, err != nil)
119119

120120
saveChannel := make(chan bool, 32)
121-
config.SaveChannel = &saveChannel
121+
config.SaveChannel = saveChannel
122122
config.HardDefaults.Service.Status.SaveChannel = config.SaveChannel
123123

124124
databaseChannel := make(chan dbtype.Message, 32)
125-
config.DatabaseChannel = &databaseChannel
125+
config.DatabaseChannel = databaseChannel
126126
config.HardDefaults.Service.Status.DatabaseChannel = config.DatabaseChannel
127127

128128
config.GetOrder(data)
@@ -188,7 +188,7 @@ func testServiceURL(id string) *service.Service {
188188
&dashboard.OptionsDefaults{}, &dashboard.OptionsDefaults{}),
189189
Options: *options,
190190
Status: *status.New(
191-
&announceChannel, &databaseChannel, &saveChannel,
191+
announceChannel, databaseChannel, saveChannel,
192192
"",
193193
"", "",
194194
"", "",

‎config/init.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,10 +97,10 @@ func (c *Config) Load(file string, flagset *map[string]bool) {
9797
c.GetOrder(data)
9898

9999
databaseChannel := make(chan dbtype.Message, 32)
100-
c.DatabaseChannel = &databaseChannel
100+
c.DatabaseChannel = databaseChannel
101101

102102
saveChannel := make(chan bool, 32)
103-
c.SaveChannel = &saveChannel
103+
c.SaveChannel = saveChannel
104104

105105
for _, svc := range c.Service {
106106
svc.Status = *status.New(

‎config/save.go‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,27 +33,29 @@ import (
3333
// when it receives a message.
3434
func (c *Config) SaveHandler() {
3535
for {
36-
<-*c.SaveChannel
37-
waitChannelTimeout(c.SaveChannel)
36+
<-c.SaveChannel
37+
drainAndDebounce(c.SaveChannel)
3838
c.Save()
3939
}
4040
}
4141

42-
// waitChannelTimeout will remove from `channel` and wait 30 seconds.
42+
var debounceDuration = 30 * time.Second
43+
44+
// drainAndDebounce will clear the message queue from `channel` and wait `debounceDuration`.
4345
//
44-
// Repeat until the channel is empty at the end of the 30 seconds.
45-
func waitChannelTimeout(channel *chan bool) {
46+
// Repeat until the channel is empty at the end of the debounce.
47+
func drainAndDebounce(channel chan bool) {
4648
for {
4749
// Clear queue.
48-
for len(*channel) != 0 {
49-
<-*channel
50+
for len(channel) != 0 {
51+
<-channel
5052
}
5153

52-
// Sleep 30s.
53-
time.Sleep(30 * time.Second)
54+
// Sleep.
55+
time.Sleep(debounceDuration)
5456

55-
// End if channel is still empty.
56-
if len(*channel) == 0 {
57+
// End if channel still empty.
58+
if len(channel) == 0 {
5759
break
5860
}
5961
}

‎config/save_test.go‎

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -31,43 +31,41 @@ import (
3131
"github.com/release-argus/Argus/webhook"
3232
)
3333

34-
var TIMEOUT time.Duration = 30 * time.Second
35-
3634
func TestConfig_SaveHandler(t *testing.T) {
3735
// GIVEN a message is sent to the SaveHandler.
3836
config := testConfig()
3937
// Disable fatal panics.
4038
defer func() { recover() }()
4139
go func() {
42-
*config.SaveChannel <- true
40+
config.SaveChannel <- true
4341
}()
4442

4543
// WHEN the SaveHandler is running for a Config with an inaccessible file.
4644
config.SaveHandler()
4745

48-
// THEN it should have panic'd after TIMEOUT and not reach this.
49-
time.Sleep(TIMEOUT * time.Second)
46+
// THEN it should have panic'd after `debounceDuration` and not reach this.
47+
time.Sleep(debounceDuration * time.Second)
5048
t.Errorf("%s\nSave should panic'd on inaccessible file location %q",
5149
packageName, config.File)
5250
}
5351

54-
func TestWaitChannelTimeout(t *testing.T) {
52+
func TestDrainAndDebounce(t *testing.T) {
5553
// GIVEN a Config.SaveChannel and messages to send/not send.
5654
tests := map[string]struct {
5755
messages int
5856
timeTaken time.Duration
5957
}{
6058
"no messages": {
6159
messages: 0,
62-
timeTaken: TIMEOUT,
60+
timeTaken: debounceDuration,
6361
},
6462
"one message": {
6563
messages: 1,
66-
timeTaken: 2 * TIMEOUT,
64+
timeTaken: 2 * debounceDuration,
6765
},
6866
"two messages": {
6967
messages: 2,
70-
timeTaken: 2 * TIMEOUT,
68+
timeTaken: 2 * debounceDuration,
7169
},
7270
}
7371

@@ -80,16 +78,15 @@ func TestWaitChannelTimeout(t *testing.T) {
8078
// WHEN those messages are sent to the channel mid-way through the wait.
8179
go func() {
8280
for tc.messages != 0 {
83-
time.Sleep(10 * time.Second)
84-
*config.SaveChannel <- true
81+
time.Sleep(4 * (debounceDuration / 10))
82+
config.SaveChannel <- true
8583
tc.messages--
8684
}
8785
}()
88-
time.Sleep(time.Second)
8986
start := time.Now().UTC()
90-
waitChannelTimeout(config.SaveChannel)
87+
drainAndDebounce(config.SaveChannel)
9188

92-
// THEN after `TIMEOUT`, it would have tried to Save.
89+
// THEN after `debounceDuration`, it would have tried to Save.
9390
elapsed := time.Since(start)
9491
if elapsed < tc.timeTaken-100*time.Millisecond ||
9592
elapsed > tc.timeTaken+100*time.Millisecond {

‎config/test/main.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,9 @@ func BareConfig(nilFlags bool) (cfg *config.Config) {
6262

6363
// Announce channel.
6464
announceChannel := make(chan []byte, 16)
65-
cfg.HardDefaults.Service.Status.AnnounceChannel = &announceChannel
65+
cfg.HardDefaults.Service.Status.AnnounceChannel = announceChannel
6666
// Save channel.
6767
saveChannel := make(chan bool, 16)
68-
cfg.HardDefaults.Service.Status.SaveChannel = &saveChannel
68+
cfg.HardDefaults.Service.Status.SaveChannel = saveChannel
6969
return
7070
}

0 commit comments

Comments
 (0)