Skip to content

Commit 0514241

Browse files
committed
Fix deprecated rand.Seed and Crontab.Save pointer receiver bug
- Remove deprecated rand.Seed() call in lib/utils.go (#21). Since Go 1.20+ the global rand is automatically seeded, and the per-call re-seeding was actually causing identical values within the same second. - Change Crontab.Save from value receiver to pointer receiver so that setting c.IsSaved = true actually persists on the caller's struct (#22). - Add tests for both fixes. https://claude.ai/code/session_01CukrzEXPeaab9By46BLrG3
1 parent 83a0837 commit 0514241

File tree

4 files changed

+58
-3
lines changed

4 files changed

+58
-3
lines changed

lib/crontab.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ func (c Crontab) Write() string {
272272
return result
273273
}
274274

275-
func (c Crontab) Save(crontabLines string) error {
275+
func (c *Crontab) Save(crontabLines string) error {
276276
if c.IsUserCrontab {
277277
cmd := c.buildCrontabCommand("-")
278278

lib/crontab_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package lib
22

33
import (
4+
"os"
45
"regexp"
56
"strings"
67
"testing"
@@ -242,6 +243,34 @@ func TestLineWriteWithEnvFlag(t *testing.T) {
242243
viper.Set("CRONITOR_ENV", originalEnv)
243244
}
244245

246+
func TestSaveSetsIsSaved(t *testing.T) {
247+
// Create a temp file to act as the crontab
248+
tmpFile, err := os.CreateTemp("", "crontab-test-*")
249+
if err != nil {
250+
t.Fatalf("Failed to create temp file: %v", err)
251+
}
252+
defer os.Remove(tmpFile.Name())
253+
tmpFile.Close()
254+
255+
crontab := &Crontab{
256+
IsUserCrontab: false, // use file-based save path
257+
Filename: tmpFile.Name(),
258+
}
259+
260+
if crontab.IsSaved {
261+
t.Fatal("IsSaved should be false before Save()")
262+
}
263+
264+
err = crontab.Save("* * * * * echo hello\n")
265+
if err != nil {
266+
t.Fatalf("Save() returned error: %v", err)
267+
}
268+
269+
if !crontab.IsSaved {
270+
t.Errorf("IsSaved should be true after Save(), but it is still false (pointer receiver bug)")
271+
}
272+
}
273+
245274
func TestLineWriteWithNoStdoutAndEnv(t *testing.T) {
246275
// Save original viper value
247276
originalEnv := viper.GetString("CRONITOR_ENV")

lib/utils.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,9 @@ import (
44
"math/rand"
55
"os"
66
"path/filepath"
7-
"time"
87
)
98

109
func randomMinute() int {
11-
rand.Seed(time.Now().Unix())
1210
return rand.Intn(59)
1311
}
1412

lib/utils_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package lib
2+
3+
import (
4+
"testing"
5+
)
6+
7+
func TestRandomMinuteRange(t *testing.T) {
8+
// Verify randomMinute returns values in the valid range [0, 59)
9+
for i := 0; i < 100; i++ {
10+
result := randomMinute()
11+
if result < 0 || result >= 59 {
12+
t.Errorf("randomMinute() returned %d, expected value in range [0, 59)", result)
13+
}
14+
}
15+
}
16+
17+
func TestRandomMinuteNotConstant(t *testing.T) {
18+
// Verify randomMinute produces varying output (not stuck on a single value).
19+
// With 20 calls, getting the same value every time is astronomically unlikely
20+
// if the RNG is properly seeded.
21+
seen := make(map[int]bool)
22+
for i := 0; i < 20; i++ {
23+
seen[randomMinute()] = true
24+
}
25+
if len(seen) < 2 {
26+
t.Errorf("randomMinute() returned the same value %d times in a row, RNG may not be working", 20)
27+
}
28+
}

0 commit comments

Comments
 (0)