Skip to content

Commit dc29c5a

Browse files
SaschaSchwarze0SaschaSchwarze0
committed
Fix findings
1 parent f0a037e commit dc29c5a

21 files changed

+96
-100
lines changed

‎pkg/shp/cmd/build/run.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ func (r *RunCommand) Complete(params *params.Params, io *genericclioptions.IOStr
6060
case 1:
6161
r.buildName = args[0]
6262
default:
63-
return errors.New("Build name is not informed")
63+
return errors.New("build name is not informed")
6464
}
6565

6666
clientset, err := params.ClientSet()
@@ -248,11 +248,12 @@ func (r *RunCommand) Run(params *params.Params, ioStreams *genericclioptions.IOS
248248
return err
249249
}
250250

251+
// Log prints a message
251252
func (r *RunCommand) Log(msg string) {
252253
// concurrent fmt.Fprintf(r.ioStream.Out...) calls need locking to avoid data races, as we 'write' to the stream
253254
r.logLock.Lock()
254255
defer r.logLock.Unlock()
255-
fmt.Fprintf(r.ioStreams.Out, msg)
256+
fmt.Fprint(r.ioStreams.Out, msg)
256257
}
257258

258259
// runCmd instantiate the "build run" sub-command using common BuildRun flags.

‎pkg/shp/cmd/buildrun/buildrun.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ func Command(p *params.Params, ioStreams *genericclioptions.IOStreams) *cobra.Co
2626
runner.NewRunner(p, ioStreams, logsCmd()).Cmd(),
2727
runner.NewRunner(p, ioStreams, createCmd()).Cmd(),
2828
runner.NewRunner(p, ioStreams, cancelCmd()).Cmd(),
29+
runner.NewRunner(p, ioStreams, deleteCmd()).Cmd(),
2930
)
3031
return command
3132
}

‎pkg/shp/cmd/buildrun/cancel.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func (c *CancelCommand) Run(params *params.Params, ioStreams *genericclioptions.
5656
return err
5757
}
5858

59-
br := &buildv1alpha1.BuildRun{}
59+
var br *buildv1alpha1.BuildRun
6060
if br, err = clientset.ShipwrightV1alpha1().BuildRuns(params.Namespace()).Get(c.cmd.Context(), c.name, metav1.GetOptions{}); err != nil {
6161
return fmt.Errorf("failed to retrieve BuildRun %s: %s", c.name, err.Error())
6262
}

‎pkg/shp/cmd/root_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import (
1212
)
1313

1414
func TestCMD_NewCmdSHP(t *testing.T) {
15-
g := gomega.NewGomegaWithT(t)
15+
g := gomega.NewWithT(t)
1616

1717
genericOpts := &genericclioptions.IOStreams{In: os.Stdin, Out: os.Stdout, ErrOut: os.Stderr}
1818
cmd := NewCmdSHP(genericOpts)

‎pkg/shp/cmd/runner/runner_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ func (m *mockedSubCommand) Run(p *params.Params, ioStreams *genericclioptions.IO
3333
}
3434

3535
func TestCMD_Runner(t *testing.T) {
36-
g := gomega.NewGomegaWithT(t)
36+
g := gomega.NewWithT(t)
3737

3838
p := params.NewParams()
3939

‎pkg/shp/flags/build_test.go

Lines changed: 24 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,16 @@ import (
44
"testing"
55
"time"
66

7-
"github.com/onsi/gomega"
7+
. "github.com/onsi/gomega"
88
buildv1alpha1 "github.com/shipwright-io/build/pkg/apis/build/v1alpha1"
99
"github.com/spf13/cobra"
1010
corev1 "k8s.io/api/core/v1"
1111
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1212
"k8s.io/utils/pointer"
13-
14-
o "github.com/onsi/gomega"
1513
)
1614

1715
func TestBuildSpecFromFlags(t *testing.T) {
18-
g := gomega.NewGomegaWithT(t)
16+
g := NewWithT(t)
1917

2018
credentials := corev1.LocalObjectReference{Name: "name"}
2119
buildStrategyKind := buildv1alpha1.ClusterBuildStrategyKind
@@ -53,71 +51,71 @@ func TestBuildSpecFromFlags(t *testing.T) {
5351

5452
t.Run(".spec.source", func(t *testing.T) {
5553
err := flags.Set(SourceURLFlag, expected.Source.URL)
56-
g.Expect(err).To(o.BeNil())
54+
g.Expect(err).To(BeNil())
5755

5856
err = flags.Set(SourceRevisionFlag, *expected.Source.Revision)
59-
g.Expect(err).To(o.BeNil())
57+
g.Expect(err).To(BeNil())
6058

6159
err = flags.Set(SourceContextDirFlag, *expected.Source.ContextDir)
62-
g.Expect(err).To(o.BeNil())
60+
g.Expect(err).To(BeNil())
6361

6462
err = flags.Set(SourceCredentialsSecretFlag, expected.Source.Credentials.Name)
65-
g.Expect(err).To(o.BeNil())
63+
g.Expect(err).To(BeNil())
6664

6765
err = flags.Set(StrategyAPIVersionFlag, expected.Strategy.APIVersion)
68-
g.Expect(err).To(o.BeNil())
66+
g.Expect(err).To(BeNil())
6967

70-
g.Expect(expected.Source).To(o.Equal(spec.Source), "spec.source")
68+
g.Expect(expected.Source).To(Equal(spec.Source), "spec.source")
7169
})
7270

7371
t.Run(".spec.strategy", func(t *testing.T) {
7472
err := flags.Set(StrategyKindFlag, string(buildv1alpha1.ClusterBuildStrategyKind))
75-
g.Expect(err).To(o.BeNil())
73+
g.Expect(err).To(BeNil())
7674

7775
err = flags.Set(StrategyNameFlag, expected.Strategy.Name)
78-
g.Expect(err).To(o.BeNil())
76+
g.Expect(err).To(BeNil())
7977

80-
g.Expect(expected.Strategy).To(o.Equal(spec.Strategy), "spec.strategy")
78+
g.Expect(expected.Strategy).To(Equal(spec.Strategy), "spec.strategy")
8179
})
8280

8381
t.Run(".spec.dockerfile", func(t *testing.T) {
8482
err := flags.Set(DockerfileFlag, *expected.Dockerfile)
85-
g.Expect(err).To(o.BeNil())
83+
g.Expect(err).To(BeNil())
8684

87-
g.Expect(spec.Dockerfile).NotTo(o.BeNil())
88-
g.Expect(*expected.Dockerfile).To(o.Equal(*spec.Dockerfile), "spec.dockerfile")
85+
g.Expect(spec.Dockerfile).NotTo(BeNil())
86+
g.Expect(*expected.Dockerfile).To(Equal(*spec.Dockerfile), "spec.dockerfile")
8987
})
9088

9189
t.Run(".spec.builder", func(t *testing.T) {
9290
err := flags.Set(BuilderImageFlag, expected.Builder.Image)
93-
g.Expect(err).To(o.BeNil())
91+
g.Expect(err).To(BeNil())
9492

9593
err = flags.Set(BuilderCredentialsSecretFlag, expected.Builder.Credentials.Name)
96-
g.Expect(err).To(o.BeNil())
94+
g.Expect(err).To(BeNil())
9795

98-
g.Expect(*expected.Builder).To(o.Equal(*spec.Builder), "spec.builder")
96+
g.Expect(*expected.Builder).To(Equal(*spec.Builder), "spec.builder")
9997
})
10098

10199
t.Run(".spec.output", func(t *testing.T) {
102100
err := flags.Set(OutputImageFlag, expected.Output.Image)
103-
g.Expect(err).To(o.BeNil())
101+
g.Expect(err).To(BeNil())
104102

105103
err = flags.Set(OutputCredentialsSecretFlag, expected.Output.Credentials.Name)
106-
g.Expect(err).To(o.BeNil())
104+
g.Expect(err).To(BeNil())
107105

108-
g.Expect(expected.Output).To(o.Equal(spec.Output), "spec.output")
106+
g.Expect(expected.Output).To(Equal(spec.Output), "spec.output")
109107
})
110108

111109
t.Run(".spec.timeout", func(t *testing.T) {
112110
err := flags.Set(TimeoutFlag, expected.Timeout.Duration.String())
113-
g.Expect(err).To(o.BeNil())
111+
g.Expect(err).To(BeNil())
114112

115-
g.Expect(*expected.Timeout).To(o.Equal(*spec.Timeout), "spec.timeout")
113+
g.Expect(*expected.Timeout).To(Equal(*spec.Timeout), "spec.timeout")
116114
})
117115
}
118116

119117
func TestSanitizeBuildSpec(t *testing.T) {
120-
g := gomega.NewGomegaWithT(t)
118+
g := NewWithT(t)
121119

122120
completeBuildSpec := buildv1alpha1.BuildSpec{
123121
Source: buildv1alpha1.Source{
@@ -183,7 +181,7 @@ func TestSanitizeBuildSpec(t *testing.T) {
183181
t.Run(tt.name, func(t *testing.T) {
184182
copy := tt.in.DeepCopy()
185183
SanitizeBuildSpec(copy)
186-
g.Expect(tt.out).To(o.Equal(*copy))
184+
g.Expect(tt.out).To(Equal(*copy))
187185
})
188186
}
189187
}

‎pkg/shp/flags/buildrun.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func SanitizeBuildRunSpec(br *buildv1alpha1.BuildRunSpec) {
3939
}
4040
if br.ServiceAccount != nil {
4141
if (br.ServiceAccount.Name == nil || *br.ServiceAccount.Name == "") &&
42-
br.ServiceAccount.Generate == false {
42+
!br.ServiceAccount.Generate {
4343
br.ServiceAccount = nil
4444
}
4545
}

‎pkg/shp/flags/buildrun_test.go

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,15 @@ import (
55
"testing"
66
"time"
77

8-
"github.com/onsi/gomega"
8+
. "github.com/onsi/gomega"
99
buildv1alpha1 "github.com/shipwright-io/build/pkg/apis/build/v1alpha1"
1010
"github.com/spf13/cobra"
1111
corev1 "k8s.io/api/core/v1"
1212
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
13-
14-
o "github.com/onsi/gomega"
1513
)
1614

1715
func TestBuildRunSpecFromFlags(t *testing.T) {
18-
g := gomega.NewGomegaWithT(t)
16+
g := NewWithT(t)
1917

2018
str := "something-random"
2119
expected := &buildv1alpha1.BuildRunSpec{
@@ -41,42 +39,42 @@ func TestBuildRunSpecFromFlags(t *testing.T) {
4139

4240
t.Run(".spec.buildRef", func(t *testing.T) {
4341
err := flags.Set(BuildrefNameFlag, expected.BuildRef.Name)
44-
g.Expect(err).To(o.BeNil())
42+
g.Expect(err).To(BeNil())
4543

46-
g.Expect(*expected.BuildRef).To(o.Equal(*spec.BuildRef), "spec.buildRef")
44+
g.Expect(*expected.BuildRef).To(Equal(*spec.BuildRef), "spec.buildRef")
4745
})
4846

4947
t.Run(".spec.serviceAccount", func(t *testing.T) {
5048
err := flags.Set(ServiceAccountNameFlag, *expected.ServiceAccount.Name)
51-
g.Expect(err).To(o.BeNil())
49+
g.Expect(err).To(BeNil())
5250

5351
generate := fmt.Sprintf("%v", expected.ServiceAccount.Generate)
5452
err = flags.Set(ServiceAccountGenerateFlag, generate)
55-
g.Expect(err).To(o.BeNil())
53+
g.Expect(err).To(BeNil())
5654

57-
g.Expect(*expected.ServiceAccount).To(o.Equal(*spec.ServiceAccount), "spec.serviceAccount")
55+
g.Expect(*expected.ServiceAccount).To(Equal(*spec.ServiceAccount), "spec.serviceAccount")
5856
})
5957

6058
t.Run(".spec.timeout", func(t *testing.T) {
6159
err := flags.Set(TimeoutFlag, expected.Timeout.Duration.String())
62-
g.Expect(err).To(o.BeNil())
60+
g.Expect(err).To(BeNil())
6361

64-
g.Expect(*expected.Timeout).To(o.Equal(*spec.Timeout), "spec.timeout")
62+
g.Expect(*expected.Timeout).To(Equal(*spec.Timeout), "spec.timeout")
6563
})
6664

6765
t.Run(".spec.output", func(t *testing.T) {
6866
err := flags.Set(OutputImageFlag, expected.Output.Image)
69-
g.Expect(err).To(o.BeNil())
67+
g.Expect(err).To(BeNil())
7068

7169
err = flags.Set(OutputCredentialsSecretFlag, expected.Output.Credentials.Name)
72-
g.Expect(err).To(o.BeNil())
70+
g.Expect(err).To(BeNil())
7371

74-
g.Expect(*expected.Output).To(o.Equal(*spec.Output), "spec.output")
72+
g.Expect(*expected.Output).To(Equal(*spec.Output), "spec.output")
7573
})
7674
}
7775

7876
func TestSanitizeBuildRunSpec(t *testing.T) {
79-
g := gomega.NewGomegaWithT(t)
77+
g := NewWithT(t)
8078

8179
name := "name"
8280
completeBuildRunSpec := buildv1alpha1.BuildRunSpec{
@@ -119,7 +117,7 @@ func TestSanitizeBuildRunSpec(t *testing.T) {
119117
t.Run(tt.name, func(t *testing.T) {
120118
copy := tt.in.DeepCopy()
121119
SanitizeBuildRunSpec(copy)
122-
g.Expect(tt.out).To(o.Equal(*copy))
120+
g.Expect(tt.out).To(Equal(*copy))
123121
})
124122
}
125123
}

‎pkg/shp/flags/core_envvar_value.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func (c *CoreEnvVarArrayValue) Set(value string) error {
3030
}
3131
for _, e := range *c.envs {
3232
if k == e.Name {
33-
return fmt.Errorf("environment variable '%s' is already set!", k)
33+
return fmt.Errorf("environment variable '%s' is already set", k)
3434
}
3535
}
3636
*c.envs = append(*c.envs, corev1.EnvVar{Name: k, Value: v})

‎pkg/shp/flags/core_envvar_value_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import (
1010
)
1111

1212
func TestCoreEnvVarSliceValue(t *testing.T) {
13-
g := o.NewGomegaWithT(t)
13+
g := o.NewWithT(t)
1414

1515
spec := &buildv1alpha1.BuildSpec{Env: []corev1.EnvVar{}}
1616
c := NewCoreEnvVarArrayValue(&spec.Env)

‎pkg/shp/flags/key_value.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import (
1212
func splitKeyValue(value string) (string, string, error) {
1313
s := strings.SplitN(value, "=", 2)
1414
if len(s) != 2 || s[0] == "" {
15-
return "", "", fmt.Errorf("informed value '%s' is not in key=value format!", value)
15+
return "", "", fmt.Errorf("informed value '%s' is not in key=value format", value)
1616
}
1717
return s[0], s[1], nil
1818
}

‎pkg/shp/flags/map_value.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ func (m *MapValue) Set(value string) error {
3333
// Type analogous to the pflag "stringArray" type, where each flag entry will be translated to a
3434
// single array (slice) entry, therefore the comma (",") is accepted as part of the value, as any
3535
// other special character.
36-
func (c *MapValue) Type() string {
36+
func (m *MapValue) Type() string {
3737
return "stringArray"
3838
}
3939

‎pkg/shp/flags/map_value_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import (
99
)
1010

1111
func TestNewMapValue(t *testing.T) {
12-
g := o.NewGomegaWithT(t)
12+
g := o.NewWithT(t)
1313

1414
spec := &buildv1alpha1.BuildSpec{Output: buildv1alpha1.Image{
1515
Labels: map[string]string{},

‎pkg/shp/flags/strategy_kind_value_test.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,22 +3,21 @@ package flags
33
import (
44
"testing"
55

6-
"github.com/onsi/gomega"
7-
o "github.com/onsi/gomega"
6+
. "github.com/onsi/gomega"
87
buildv1alpha1 "github.com/shipwright-io/build/pkg/apis/build/v1alpha1"
98
)
109

1110
func TestStrategyKindValue(t *testing.T) {
12-
g := gomega.NewGomegaWithT(t)
11+
g := NewWithT(t)
1312

1413
buildStrategyKind := buildv1alpha1.ClusterBuildStrategyKind
1514
v := NewStrategyKindValue(&buildStrategyKind)
1615

1716
expected := buildv1alpha1.NamespacedBuildStrategyKind
1817

1918
err := v.Set(string(expected))
20-
g.Expect(err).To(o.BeNil())
19+
g.Expect(err).To(BeNil())
2120

22-
g.Expect(string(expected)).To(o.Equal(v.String()))
23-
g.Expect(expected).To(o.Equal(buildStrategyKind))
21+
g.Expect(string(expected)).To(Equal(v.String()))
22+
g.Expect(expected).To(Equal(buildStrategyKind))
2423
}

‎pkg/shp/flags/string_pointer_value_test.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,12 @@ package flags
33
import (
44
"testing"
55

6-
"github.com/onsi/gomega"
7-
o "github.com/onsi/gomega"
6+
. "github.com/onsi/gomega"
87
"github.com/spf13/cobra"
98
)
109

1110
func TestStringPointerValue(t *testing.T) {
12-
g := gomega.NewGomegaWithT(t)
11+
g := NewWithT(t)
1312

1413
flagName := "flag"
1514
value := "value"
@@ -20,9 +19,9 @@ func TestStringPointerValue(t *testing.T) {
2019
flags.Var(NewStringPointerValue(&targetStr), flagName, "")
2120

2221
err := flags.Set(flagName, value)
23-
g.Expect(err).To(o.BeNil())
22+
g.Expect(err).To(BeNil())
2423

2524
v, err := flags.GetString(flagName)
26-
g.Expect(err).To(o.BeNil())
27-
g.Expect(value).To(o.Equal(v))
25+
g.Expect(err).To(BeNil())
26+
g.Expect(value).To(Equal(v))
2827
}

‎pkg/shp/params/params.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ func NewParams() *Params {
100100
return p
101101
}
102102

103+
// NewParamsForTest creates an instance of Params for testing purpose
103104
func NewParamsForTest(clientset kubernetes.Interface,
104105
shpClientset buildclientset.Interface,
105106
configFlags *genericclioptions.ConfigFlags,

‎pkg/shp/params/params_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import (
88
)
99

1010
func TestParamsCreation(t *testing.T) {
11-
g := gomega.NewGomegaWithT(t)
11+
g := gomega.NewWithT(t)
1212

1313
flagset := pflag.NewFlagSet("name", 0)
1414

0 commit comments

Comments
 (0)