Skip to content

Commit 8cb044c

Browse files
StupidScienceStupidScience
committed
Rework some flags and add tests
1 parent 4a7634a commit 8cb044c

10 files changed

Lines changed: 602 additions & 21 deletions

File tree

‎Makefile‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ RESET := $(shell tput -Txterm sgr0)
66
.PHONY: all
77
all: help
88

9+
.PHONY: cover
10+
cover:
11+
go tool cover -html ./cover.out
12+
913
.PHONY: doc
1014
doc:
1115
./hacks/run-doc-generation.sh
@@ -44,6 +48,7 @@ help:
4448
@echo ' ${YELLOW}make${RESET} ${GREEN}<target>${RESET}'
4549
@echo ''
4650
@echo 'Targets:'
51+
@echo " ${YELLOW}cover ${RESET} Open html coverage report in browser"
4752
@echo " ${YELLOW}doc ${RESET} Run doc generation"
4853
@echo " ${YELLOW}lint ${RESET} Run linters via golangci-lint"
4954
@echo " ${YELLOW}prepare ${RESET} Run all available checks and generators"

‎internal/flags/clrevents.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ func (c *CLREvents) String() string {
1616
}
1717

1818
func (c *CLREvents) Set(str string) error {
19+
if strings.TrimSpace(str) == "" {
20+
return fmt.Errorf("passed empty CLR Events")
21+
}
1922
events := strings.Split(str, CLREventsDelimiter)
2023
*c = events
2124
return nil

‎internal/flags/duration.go‎

Lines changed: 52 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,33 +4,72 @@ import (
44
"fmt"
55
"strconv"
66
"strings"
7+
"time"
78
)
89

9-
type Duration string
10+
type Duration time.Duration
1011

11-
var defaultDuration = "00:00:00:10"
12+
var (
13+
defaultDuration = 10 * time.Second
14+
day = 24 * time.Hour
15+
unitPosition = map[int]time.Duration{
16+
0: 24 * time.Hour,
17+
1: time.Hour,
18+
2: time.Minute,
19+
3: time.Second,
20+
}
21+
)
1222

1323
func (d *Duration) String() string {
14-
return string(*d)
24+
m := time.Duration(*d)
25+
days := m / day
26+
m -= days * day
27+
hours := m / time.Hour
28+
m -= hours * time.Hour
29+
minutes := m / time.Minute
30+
m -= minutes * time.Minute
31+
seconds := m / time.Second
32+
return fmt.Sprintf(
33+
"%02d:%02d:%02d:%02d",
34+
days, hours, minutes, seconds,
35+
)
1536
}
1637

1738
func (d *Duration) Set(str string) error {
39+
if strings.TrimSpace(str) == "" {
40+
return fmt.Errorf("empty duration passed")
41+
}
42+
m, err := time.ParseDuration(str)
43+
if err == nil {
44+
*d = Duration(m)
45+
return d.checkDuration()
46+
}
47+
1848
split := strings.Split(str, ":")
1949
if len(split) != 4 {
2050
return fmt.Errorf(
2151
"wrong duration format provided \"%s\", should be \"dd:hh:mm:ss\"",
2252
str,
2353
)
2454
}
25-
for _, s := range split {
26-
if _, err := strconv.Atoi(s); err != nil {
55+
for i := range split {
56+
n, err := strconv.Atoi(split[i])
57+
if err != nil {
2758
return fmt.Errorf(
28-
"wrong duration format provided \"%s\", should be \"dd:hh:mm:ss\"",
59+
"not a number value provided for duration: \"%s\"",
2960
str,
3061
)
3162
}
63+
m += time.Duration(n) * unitPosition[i]
64+
}
65+
*d = Duration(m)
66+
return d.checkDuration()
67+
}
68+
69+
func (d *Duration) checkDuration() error {
70+
if time.Duration(*d) < (1 * time.Second) {
71+
return fmt.Errorf("provided duration is to low, minimum 1 second")
3272
}
33-
*d = Duration(str)
3473
return nil
3574
}
3675

@@ -39,5 +78,10 @@ func (d *Duration) Type() string {
3978
}
4079

4180
func (d *Duration) Description() string {
42-
return "Trace for the given timespan and then automatically stop the trace. Provided in the form of dd:hh:mm:ss"
81+
return "Trace for the given timespan and then automatically stop the trace." +
82+
fmt.Sprintf(
83+
"Provided in the form of dd:hh:mm:ss or "+
84+
"corresponding time unit representation (e.g. 1s, 2m, 3h) (default %s)",
85+
defaultDuration,
86+
)
4387
}

‎internal/flags/gcdump_flagset.go‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,11 @@
11
package flags
22

33
import (
4-
"strconv"
5-
64
"github.com/spf13/pflag"
75
)
86

97
type GCDumpFlagSet struct {
10-
Timeout int
8+
Timeout Timeout
119
dt *DotnetToolsFlagSet
1210

1311
flagSet *pflag.FlagSet
@@ -23,11 +21,10 @@ func NewGCDumpFlagSet() *GCDumpFlagSet {
2321
func (gc *GCDumpFlagSet) Parse() *pflag.FlagSet {
2422
flagSet := pflag.NewFlagSet("dotnet-gcdump", pflag.ExitOnError)
2523
flagSet.AddFlagSet(gc.dt.Parse())
26-
flagSet.IntVar(
24+
flagSet.Var(
2725
&gc.Timeout,
28-
"timeout",
29-
gc.Timeout,
30-
"Give up on collecting the GC dump if it takes longer than this many seconds",
26+
gc.Timeout.Type(),
27+
gc.Timeout.Description(),
3128
)
3229

3330
gc.flagSet = flagSet
@@ -36,12 +33,10 @@ func (gc *GCDumpFlagSet) Parse() *pflag.FlagSet {
3633

3734
func (gc *GCDumpFlagSet) Args() []string {
3835
args := gc.dt.Args()
39-
if gc.flagSet.Changed("timeout") {
36+
if gc.flagSet.Changed(gc.Timeout.Type()) {
4037
args = append(
4138
args,
42-
[]string{
43-
"--timeout", strconv.Itoa(gc.Timeout),
44-
}...,
39+
FlagToArg(&gc.Timeout)...,
4540
)
4641
}
4742
return args

‎internal/flags/gcdump_flagset_test.go‎

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
package flags
2+
3+
import (
4+
"testing"
5+
6+
"github.com/spf13/pflag"
7+
"github.com/stretchr/testify/require"
8+
)
9+
10+
func Test_GCDumpFlagSet(t *testing.T) {
11+
testCases := []struct {
12+
name string
13+
args []string
14+
expArgs []string
15+
}{
16+
{
17+
name: "Defaults",
18+
args: []string{},
19+
expArgs: []string{
20+
"--process-id", "1",
21+
},
22+
},
23+
{
24+
name: "Override process ID",
25+
args: []string{
26+
"--process-id", "5",
27+
},
28+
expArgs: []string{
29+
"--process-id", "5",
30+
},
31+
},
32+
{
33+
name: "Override timeout",
34+
args: []string{
35+
"--timeout", "120",
36+
},
37+
expArgs: []string{
38+
"--process-id", "1",
39+
"--timeout", "120",
40+
},
41+
},
42+
{
43+
name: "Override timeout in seconds",
44+
args: []string{
45+
"--timeout", "120s",
46+
},
47+
expArgs: []string{
48+
"--process-id", "1",
49+
"--timeout", "120",
50+
},
51+
},
52+
{
53+
name: "Override timeout in minutes",
54+
args: []string{
55+
"--timeout", "2m",
56+
},
57+
expArgs: []string{
58+
"--process-id", "1",
59+
"--timeout", "120",
60+
},
61+
},
62+
{
63+
name: "Override timeout with milliseconds",
64+
args: []string{
65+
"--timeout", "60s50ms",
66+
},
67+
expArgs: []string{
68+
"--process-id", "1",
69+
"--timeout", "60",
70+
},
71+
},
72+
{
73+
name: "Override timeout and process id",
74+
args: []string{
75+
"--process-id", "5",
76+
"--timeout", "10m",
77+
},
78+
expArgs: []string{
79+
"--process-id", "5",
80+
"--timeout", "600",
81+
},
82+
},
83+
}
84+
85+
for _, tc := range testCases {
86+
t.Run(tc.name, func(t *testing.T) {
87+
flagSet := pflag.NewFlagSet("test", pflag.ContinueOnError)
88+
gc := NewGCDumpFlagSet()
89+
flagSet.AddFlagSet(gc.Parse())
90+
91+
require.NoError(t, flagSet.Parse(tc.args))
92+
require.Equal(t, tc.expArgs, gc.Args())
93+
})
94+
}
95+
}
96+
97+
func Test_GCDumpFlagSet_Errors(t *testing.T) {
98+
testCases := []struct {
99+
name string
100+
args []string
101+
}{
102+
{
103+
name: "Bad process ID",
104+
args: []string{
105+
"--process-id", "a",
106+
},
107+
},
108+
{
109+
name: "Empty process ID",
110+
args: []string{
111+
"--process-id", "",
112+
},
113+
},
114+
{
115+
name: "Bad timeout",
116+
args: []string{
117+
"--timeout", "abc",
118+
},
119+
},
120+
{
121+
name: "Low timeout",
122+
args: []string{
123+
"--timeout", "5ms",
124+
},
125+
},
126+
{
127+
name: "Empty timeout",
128+
args: []string{
129+
"--timeout", "",
130+
},
131+
},
132+
}
133+
134+
for _, tc := range testCases {
135+
t.Run(tc.name, func(t *testing.T) {
136+
flagSet := pflag.NewFlagSet("test", pflag.ContinueOnError)
137+
gc := NewGCDumpFlagSet()
138+
flagSet.AddFlagSet(gc.Parse())
139+
140+
require.Error(t, flagSet.Parse(tc.args))
141+
})
142+
}
143+
}

‎internal/flags/providers.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
package flags
22

3-
import "strings"
3+
import (
4+
"fmt"
5+
"strings"
6+
)
47

58
type Providers []string
69

@@ -13,6 +16,9 @@ func (p *Providers) String() string {
1316
}
1417

1518
func (p *Providers) Set(str string) error {
19+
if strings.TrimSpace(str) == "" {
20+
return fmt.Errorf("empty providers given")
21+
}
1622
providers := strings.Split(str, ProvidersDelimiter)
1723
*p = providers
1824
return nil

‎internal/flags/timeout.go‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
package flags
2+
3+
import (
4+
"fmt"
5+
"strconv"
6+
"strings"
7+
"time"
8+
)
9+
10+
type Timeout time.Duration
11+
12+
func (t *Timeout) String() string {
13+
return fmt.Sprintf(
14+
"%.0f",
15+
time.Duration(*t).
16+
Seconds(),
17+
)
18+
}
19+
20+
func (t *Timeout) Set(str string) error {
21+
if strings.TrimSpace(str) == "" {
22+
return fmt.Errorf("empty timeout passed")
23+
}
24+
d, err := time.ParseDuration(str)
25+
if err == nil {
26+
*t = Timeout(d)
27+
return t.checkDuration()
28+
}
29+
i, err := strconv.Atoi(str)
30+
if err == nil {
31+
*t = Timeout(time.Duration(i) * time.Second)
32+
return t.checkDuration()
33+
}
34+
35+
return fmt.Errorf(
36+
"can't parse duration from %s, provide it as number to define seconds or with units",
37+
str,
38+
)
39+
}
40+
41+
func (t *Timeout) checkDuration() error {
42+
if time.Duration(*t) < (1 * time.Second) {
43+
return fmt.Errorf("provided duration is to low, minimum 1 second")
44+
}
45+
return nil
46+
}
47+
48+
func (t *Timeout) Type() string {
49+
return "timeout"
50+
}
51+
52+
func (t *Timeout) Description() string {
53+
return "Give up on collecting the GC dump if it takes longer than this many seconds.\n" +
54+
"Valid time units are \"ns\", \"us\" (or \"µs\"), \"ms\", \"s\", \"m\", \"h\".\n" +
55+
"Will be rounded to seconds. If no unit provided defaults to seconds.\n" +
56+
"(default 30 sec)"
57+
}

0 commit comments

Comments
 (0)