Skip to content

Commit

Permalink
pkg/apis tests cleanup on duplicate test name 💅
Browse files Browse the repository at this point in the history 
The error/fatal message should report the name of the test. It is
already available when the test fail as it is part of the name of the
test.

This reduces some duplication and make some tests a bit easier to read
and write 😉.

Signed-off-by: Vincent Demeester <vdemeest@redhat.com>
  • Loading branch information
@vdemeester
vdemeester committed Nov 4, 2020
1 parent 7b5b2fa commit acd2d73
Show file tree
Hide file tree
Showing 10 changed files with 48 additions and 63 deletions.
2 changes: 1 addition & 1 deletion pkg/apis/pipeline/v1beta1/param_types_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func TestParamSpec_SetDefaults(t *testing.T) {
ctx := context.Background()
tc.before.SetDefaults(ctx)
if d := cmp.Diff(tc.before, tc.defaultsApplied); d != "" {
t.Errorf("ParamSpec.SetDefaults/%s %s", tc.name, diff.PrintWantGot(d))
t.Error(diff.PrintWantGot(d))
}
})
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/pipeline/v1beta1/pipeline_defaults_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func TestPipelineSpec_SetDefaults(t *testing.T) {
ctx := context.Background()
tc.ps.SetDefaults(ctx)
if d := cmp.Diff(tc.want, tc.ps); d != "" {
t.Errorf("Mismatch of pipelineSpec after setting defaults: %s: %s", tc.desc, diff.PrintWantGot(d))
t.Errorf("Mismatch of pipelineSpec after setting defaults: %s", diff.PrintWantGot(d))
}
})
}
Expand Down
38 changes: 19 additions & 19 deletions pkg/apis/pipeline/v1beta1/pipeline_validation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func TestPipeline_Validate_Success(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
err := tt.p.Validate(context.Background())
if err != nil {
t.Errorf("Pipeline.Validate() returned error for valid Pipeline: %s: %v", tt.name, err)
t.Errorf("Pipeline.Validate() returned error for valid Pipeline: %v", err)
}
})
}
Expand Down Expand Up @@ -155,7 +155,7 @@ func TestPipeline_Validate_Failure(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
err := tt.p.Validate(context.Background())
if err == nil {
t.Errorf("Pipeline.Validate() did not return error for invalid pipeline: %s", tt.name)
t.Error("Pipeline.Validate() did not return error for invalid pipeline")
}
if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" {
t.Errorf("Pipeline.Validate() errors diff %s", diff.PrintWantGot(d))
Expand Down Expand Up @@ -385,7 +385,7 @@ func TestPipelineSpec_Validate_Failure(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
err := tt.ps.Validate(context.Background())
if err == nil {
t.Errorf("PipelineSpec.Validate() did not return error for invalid pipelineSpec: %s", tt.name)
t.Errorf("PipelineSpec.Validate() did not return error for invalid pipelineSpec")
}
if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" {
t.Errorf("PipelineSpec.Validate() errors diff %s", diff.PrintWantGot(d))
Expand Down Expand Up @@ -430,7 +430,7 @@ func TestValidatePipelineTasks_Success(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
err := validatePipelineTasks(context.Background(), tt.tasks, []PipelineTask{})
if err != nil {
t.Errorf("Pipeline.validatePipelineTasks() returned error for valid pipeline tasks: %s: %v", tt.name, err)
t.Errorf("Pipeline.validatePipelineTasks() returned error for valid pipeline tasks: %v", err)
}
})
}
Expand Down Expand Up @@ -520,7 +520,7 @@ func TestValidatePipelineTasks_Failure(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
err := validatePipelineTasks(context.Background(), tt.tasks, []PipelineTask{})
if err == nil {
t.Error("Pipeline.validatePipelineTasks() did not return error for invalid pipeline tasks:", tt.name)
t.Error("Pipeline.validatePipelineTasks() did not return error for invalid pipeline tasks")
}
if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" {
t.Errorf("PipelineSpec.Validate() errors diff %s", diff.PrintWantGot(d))
Expand Down Expand Up @@ -554,7 +554,7 @@ func TestValidateFrom_Success(t *testing.T) {
t.Run(desc, func(t *testing.T) {
err := validateFrom(tasks)
if err != nil {
t.Errorf("Pipeline.validateFrom() returned error for: %s: %v", desc, err)
t.Errorf("Pipeline.validateFrom() returned error for: %v", err)
}
})
}
Expand Down Expand Up @@ -668,7 +668,7 @@ func TestValidateFrom_Failure(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
err := validateFrom(tt.tasks)
if err == nil {
t.Error("Pipeline.validateFrom() did not return error for invalid pipeline task resources: ", tt.name)
t.Error("Pipeline.validateFrom() did not return error for invalid pipeline task resources")
}
if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" {
t.Errorf("PipelineSpec.Validate() errors diff %s", diff.PrintWantGot(d))
Expand Down Expand Up @@ -764,7 +764,7 @@ func TestValidateDeclaredResources_Success(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
err := validateDeclaredResources(tt.resources, tt.tasks, []PipelineTask{})
if err != nil {
t.Errorf("Pipeline.validateDeclaredResources() returned error for valid resource declarations: %s: %v", tt.name, err)
t.Errorf("Pipeline.validateDeclaredResources() returned error for valid resource declarations: %v", err)
}
})
}
Expand Down Expand Up @@ -860,7 +860,7 @@ func TestValidateDeclaredResources_Failure(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
err := validateDeclaredResources(tt.resources, tt.tasks, []PipelineTask{})
if err == nil {
t.Errorf("Pipeline.validateDeclaredResources() did not return error for invalid resource declarations: %s", tt.name)
t.Errorf("Pipeline.validateDeclaredResources() did not return error for invalid resource declarations")
}
if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" {
t.Errorf("PipelineSpec.Validate() errors diff %s", diff.PrintWantGot(d))
Expand Down Expand Up @@ -1062,7 +1062,7 @@ func TestValidatePipelineParameterVariables_Success(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
err := validatePipelineParameterVariables(tt.tasks, tt.params)
if err != nil {
t.Errorf("Pipeline.validatePipelineParameterVariables() returned error for valid pipeline parameters: %s: %v", tt.name, err)
t.Errorf("Pipeline.validatePipelineParameterVariables() returned error for valid pipeline parameters: %v", err)
}
})
}
Expand Down Expand Up @@ -1340,7 +1340,7 @@ func TestValidatePipelineParameterVariables_Failure(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
err := validatePipelineParameterVariables(tt.tasks, tt.params)
if err == nil {
t.Errorf("Pipeline.validatePipelineParameterVariables() did not return error for invalid pipeline parameters: %s", tt.name)
t.Errorf("Pipeline.validatePipelineParameterVariables() did not return error for invalid pipeline parameters")
}
if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" {
t.Errorf("PipelineSpec.Validate() errors diff %s", diff.PrintWantGot(d))
Expand All @@ -1362,7 +1362,7 @@ func TestValidatePipelineWorkspaces_Success(t *testing.T) {
t.Run(desc, func(t *testing.T) {
err := validatePipelineWorkspaces(workspaces, tasks, []PipelineTask{})
if err != nil {
t.Errorf("Pipeline.validatePipelineWorkspaces() returned error for valid pipeline workspaces: %s: %v", desc, err)
t.Errorf("Pipeline.validatePipelineWorkspaces() returned error for valid pipeline workspaces: %v", err)
}
})
}
Expand Down Expand Up @@ -1420,7 +1420,7 @@ func TestValidatePipelineWorkspaces_Failure(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
err := validatePipelineWorkspaces(tt.workspaces, tt.tasks, []PipelineTask{})
if err == nil {
t.Errorf("Pipeline.validatePipelineWorkspaces() did not return error for invalid pipeline workspaces: %s", tt.name)
t.Errorf("Pipeline.validatePipelineWorkspaces() did not return error for invalid pipeline workspaces")
}
if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" {
t.Errorf("PipelineSpec.Validate() errors diff %s", diff.PrintWantGot(d))
Expand Down Expand Up @@ -1498,7 +1498,7 @@ func TestValidatePipelineWithFinalTasks_Success(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
err := tt.p.Validate(context.Background())
if err != nil {
t.Errorf("Pipeline.Validate() returned error for valid pipeline with finally: %s: %v", tt.name, err)
t.Errorf("Pipeline.Validate() returned error for valid pipeline with finally: %v", err)
}
})
}
Expand Down Expand Up @@ -1731,7 +1731,7 @@ func TestValidatePipelineWithFinalTasks_Failure(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
err := tt.p.Validate(context.Background())
if err == nil {
t.Errorf("Pipeline.Validate() did not return error for invalid pipeline with finally: %s", tt.name)
t.Errorf("Pipeline.Validate() did not return error for invalid pipeline with finally")
}
if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" {
t.Errorf("PipelineSpec.Validate() errors diff %s", diff.PrintWantGot(d))
Expand Down Expand Up @@ -1767,7 +1767,7 @@ func TestValidateTasksAndFinallySection_Success(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
err := validateTasksAndFinallySection(tt.ps)
if err != nil {
t.Errorf("Pipeline.ValidateTasksAndFinallySection() returned error for valid pipeline with finally: %s: %v", tt.name, err)
t.Errorf("Pipeline.ValidateTasksAndFinallySection() returned error for valid pipeline with finally: %v", err)
}
})
}
Expand Down Expand Up @@ -1871,7 +1871,7 @@ func TestValidateFinalTasks_Failure(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
err := validateFinalTasks(tt.finalTasks)
if err == nil {
t.Errorf("Pipeline.ValidateFinalTasks() did not return error for invalid pipeline: %s", tt.name)
t.Errorf("Pipeline.ValidateFinalTasks() did not return error for invalid pipeline")
}
if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" {
t.Errorf("PipelineSpec.Validate() errors diff %s", diff.PrintWantGot(d))
Expand Down Expand Up @@ -1933,7 +1933,7 @@ func TestContextValid(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if err := validatePipelineContextVariables(tt.tasks); err != nil {
t.Errorf("Pipeline.validatePipelineContextVariables() returned error for valid pipeline context variables: %s: %v", tt.name, err)
t.Errorf("Pipeline.validatePipelineContextVariables() returned error for valid pipeline context variables: %v", err)
}
})
}
Expand Down Expand Up @@ -1986,7 +1986,7 @@ func TestContextInvalid(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
err := validatePipelineContextVariables(tt.tasks)
if err == nil {
t.Errorf("Pipeline.validatePipelineContextVariables() did not return error for invalid pipeline parameters: %s, %s", tt.name, tt.tasks[0].Params)
t.Errorf("Pipeline.validatePipelineContextVariables() did not return error for invalid pipeline parameters: %s", tt.tasks[0].Params)
}
if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" {
t.Errorf("PipelineSpec.Validate() errors diff %s", diff.PrintWantGot(d))
Expand Down
16 changes: 9 additions & 7 deletions pkg/apis/pipeline/v1beta1/pipelinerun_types_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,10 +304,12 @@ func TestPipelineRunGetServiceAccountName(t *testing.T) {
},
} {
for taskName, expected := range tt.saNames {
sa := tt.pr.GetServiceAccountName(taskName)
if expected != sa {
t.Errorf("%s: wrong service account: got: %v, want: %v", tt.name, sa, expected)
}
t.Run(tt.name, func(t *testing.T) {
sa := tt.pr.GetServiceAccountName(taskName)
if expected != sa {
t.Errorf("wrong service account: got: %v, want: %v", sa, expected)
}
})
}
}
}
Expand Down Expand Up @@ -385,7 +387,7 @@ func TestPipelineRunGetPodSpecSABackcompatibility(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
sa, _ := tt.pr.GetTaskRunSpecs(taskName)
if expected != sa {
t.Errorf("%s: wrong service account: got: %v, want: %v", tt.name, sa, expected)
t.Errorf("wrong service account: got: %v, want: %v", sa, expected)
}
})
}
Expand Down Expand Up @@ -428,10 +430,10 @@ func TestPipelineRunGetPodSpec(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
sa, taskPodTemplate := tt.pr.GetTaskRunSpecs(taskName)
if values[0] != taskPodTemplate.SchedulerName {
t.Errorf("%s: wrong task podtemplate scheduler name: got: %v, want: %v", tt.name, taskPodTemplate.SchedulerName, values[0])
t.Errorf("wrong task podtemplate scheduler name: got: %v, want: %v", taskPodTemplate.SchedulerName, values[0])
}
if values[1] != sa {
t.Errorf("%s: wrong service account: got: %v, want: %v", tt.name, sa, values[1])
t.Errorf("wrong service account: got: %v, want: %v", sa, values[1])
}
})
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func TestPipelineRun_Invalidate(t *testing.T) {
t.Run(ps.name, func(t *testing.T) {
err := ps.pr.Validate(context.Background())
if d := cmp.Diff(err.Error(), ps.want.Error()); d != "" {
t.Errorf("PipelineRun.Validate/%s %s", ps.name, diff.PrintWantGot(d))
t.Error(diff.PrintWantGot(d))
}
})
}
Expand Down Expand Up @@ -203,7 +203,7 @@ func TestPipelineRun_Validate(t *testing.T) {
for _, ts := range tests {
t.Run(ts.name, func(t *testing.T) {
if err := ts.pr.Validate(context.Background()); err != nil {
t.Errorf("Unexpected PipelineRun.Validate() error = %v", err)
t.Error(err)
}
})
}
Expand Down Expand Up @@ -278,7 +278,7 @@ func TestPipelineRunSpec_Invalidate(t *testing.T) {
t.Run(ps.name, func(t *testing.T) {
err := ps.spec.Validate(context.Background())
if d := cmp.Diff(ps.wantErr.Error(), err.Error()); d != "" {
t.Errorf("PipelineRunSpec.Validate/%s (-want, +got) = %v", ps.name, d)
t.Error(diff.PrintWantGot(d))
}
})
}
Expand All @@ -304,7 +304,7 @@ func TestPipelineRunSpec_Validate(t *testing.T) {
for _, ps := range tests {
t.Run(ps.name, func(t *testing.T) {
if err := ps.spec.Validate(context.Background()); err != nil {
t.Errorf("PipelineRunSpec.Validate/%s (-want, +got) = %v", ps.name, err)
t.Error(err)
}
})
}
Expand Down
17 changes: 0 additions & 17 deletions pkg/apis/pipeline/v1beta1/pod_test.go
View file

This file was deleted.

8 changes: 4 additions & 4 deletions pkg/apis/pipeline/v1beta1/resultref_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func TestNewResultReference(t *testing.T) {
} else {
got := v1beta1.NewResultRefs(expressions)
if d := cmp.Diff(tt.want, got); d != "" {
t.Errorf("TestNewResultReference/%s %s", tt.name, diff.PrintWantGot(d))
t.Error(diff.PrintWantGot(d))
}
}
})
Expand Down Expand Up @@ -205,7 +205,7 @@ func TestHasResultReference(t *testing.T) {
return true
})
if d := cmp.Diff(tt.wantRef, got); d != "" {
t.Errorf("TestHasResultReference/%s %s", tt.name, diff.PrintWantGot(d))
t.Error(diff.PrintWantGot(d))
}
})
}
Expand Down Expand Up @@ -384,7 +384,7 @@ func TestNewResultReferenceWhenExpressions(t *testing.T) {
} else {
got := v1beta1.NewResultRefs(expressions)
if d := cmp.Diff(tt.want, got); d != "" {
t.Errorf("TestNewResultReference/%s %s", tt.name, diff.PrintWantGot(d))
t.Error(diff.PrintWantGot(d))
}
}
})
Expand Down Expand Up @@ -459,7 +459,7 @@ func TestHasResultReferenceWhenExpression(t *testing.T) {
}
got := v1beta1.NewResultRefs(expressions)
if d := cmp.Diff(tt.wantRef, got); d != "" {
t.Errorf("TestHasResultReference/%s %s", tt.name, diff.PrintWantGot(d))
t.Errorf(diff.PrintWantGot(d))
}
})
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/apis/pipeline/v1beta1/taskrun_validation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func TestTaskRun_Invalidate(t *testing.T) {
t.Run(ts.name, func(t *testing.T) {
err := ts.task.Validate(context.Background())
if d := cmp.Diff(err.Error(), ts.want.Error()); d != "" {
t.Errorf("TaskRun.Validate/%s %s", ts.name, diff.PrintWantGot(d))
t.Error(diff.PrintWantGot(d))
}
})
}
Expand Down Expand Up @@ -120,7 +120,7 @@ func TestTaskRun_Workspaces_Invalid(t *testing.T) {
t.Errorf("Expected error for invalid TaskRun but got none")
}
if d := cmp.Diff(ts.wantErr.Error(), err.Error()); d != "" {
t.Errorf("TaskRunSpec.Validate/%s %s", ts.name, diff.PrintWantGot(d))
t.Error(diff.PrintWantGot(d))
}
})
}
Expand Down Expand Up @@ -224,7 +224,7 @@ func TestTaskRunSpec_Invalidate(t *testing.T) {
t.Run(ts.name, func(t *testing.T) {
err := ts.spec.Validate(context.Background())
if d := cmp.Diff(ts.wantErr.Error(), err.Error()); d != "" {
t.Errorf("TaskRunSpec.Validate/%s %s", ts.name, diff.PrintWantGot(d))
t.Error(diff.PrintWantGot(d))
}
})
}
Expand Down Expand Up @@ -287,7 +287,7 @@ func TestTaskRunSpec_Validate(t *testing.T) {
for _, ts := range tests {
t.Run(ts.name, func(t *testing.T) {
if err := ts.spec.Validate(context.Background()); err != nil {
t.Errorf("TaskRunSpec.Validate()/%s error = %v", ts.name, err)
t.Error(err)
}
})
}
Expand Down Expand Up @@ -520,7 +520,7 @@ func TestResources_Invalidate(t *testing.T) {
t.Run(ts.name, func(t *testing.T) {
err := ts.resources.Validate(context.Background())
if d := cmp.Diff(err.Error(), ts.wantErr.Error()); d != "" {
t.Errorf("TaskRunInputs.Validate/%s %s", ts.name, diff.PrintWantGot(d))
t.Error(diff.PrintWantGot(d))
}
})
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/apis/pipeline/v1beta1/when_types_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func TestAllowsExecution(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
got := tc.whenExpressions.AllowsExecution()
if d := cmp.Diff(tc.expected, got); d != "" {
t.Errorf("Error evaluating AllowsExecution() for When Expressions in test case %s: %s", tc.name, diff.PrintWantGot(d))
t.Errorf("Error evaluating AllowsExecution() for When Expressions in test case %s", diff.PrintWantGot(d))
}
})
}
Expand Down Expand Up @@ -142,7 +142,7 @@ func TestHaveVariables(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
got := tc.whenExpressions.HaveVariables()
if d := cmp.Diff(tc.expected, got); d != "" {
t.Errorf("Error evaluating HaveVariables() for When Expressions in test case %s: %s", tc.name, diff.PrintWantGot(d))
t.Errorf("Error evaluating HaveVariables() for When Expressions in test case %s", diff.PrintWantGot(d))
}
})
}
Expand Down Expand Up @@ -263,7 +263,7 @@ func TestReplaceWhenExpressionsVariables(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
got := tc.whenExpressions.ReplaceWhenExpressionsVariables(tc.replacements)
if d := cmp.Diff(tc.expected, got); d != "" {
t.Errorf("Error evaluating When Expressions in test case %s: %s", tc.name, diff.PrintWantGot(d))
t.Errorf("Error evaluating When Expressions in test case %s", diff.PrintWantGot(d))
}
})
}
Expand Down

0 comments on commit acd2d73

Please sign in to comment.