New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changed code to improve output for files under test/e2e/apimachinery #106764
Changed code to improve output for files under test/e2e/apimachinery #106764
Conversation
@@ -339,7 +339,9 @@ var _ = SIGDescribe("CustomResourceDefinition resources [Privileged:ClusterAdmin | |||
}}, metav1.CreateOptions{}) | |||
framework.ExpectNoError(err, "creating CR") | |||
v, found, err := unstructured.NestedFieldNoCopy(u2.Object, "a") | |||
framework.ExpectEqual(found, true, "\"a\" is defaulted") | |||
if !found{ | |||
framework.Failf("value of object in %+v with field a is nil or it does not exists or its type is not expected type map[string]interface", u2.Object) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not very sure about this output format, open for suggestions of a better output format.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
framework.Failf("value of object in %+v with field a is nil or it does not exists or its type is not expected type map[string]interface", u2.Object) | |
framework.Failf("field `a` should have been defaulted in %+v", u2.Object) |
A developer reading this should know what "defaulted" means. It also was what the original message said.
5ad17ab
to
b7fb835
Compare
test/e2e/apimachinery/aggregator.go
Outdated
@@ -512,7 +512,9 @@ func TestSampleAPIServer(f *framework.Framework, aggrclient *aggregatorclient.Cl | |||
break | |||
} | |||
} | |||
framework.ExpectEqual(locatedWardle, true, "Unable to find v1alpha1.wardle.example.com in APIServiceList") | |||
if !locatedWardle{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check with gofmt
, this looks wrong.
@@ -909,7 +909,9 @@ var _ = SIGDescribe("ResourceQuota", func() { | |||
|
|||
ginkgo.By("Verifying the deleted ResourceQuota") | |||
_, err = client.CoreV1().ResourceQuotas(ns).Get(context.TODO(), quotaName, metav1.GetOptions{}) | |||
framework.ExpectEqual(apierrors.IsNotFound(err), true) | |||
if !apierrors.IsNotFound(err){ | |||
framework.Failf("Error:", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
framework.Failf("Error:", err) | |
framework.Failf("Expected `not found` error, got: %v", err) |
@@ -1099,7 +1101,9 @@ var _ = SIGDescribe("ResourceQuota [Feature:PodPriority]", func() { | |||
ginkgo.It("should verify ResourceQuota's priority class scope (quota set to pod count: 1) against a pod with same priority class.", func() { | |||
|
|||
_, err := f.ClientSet.SchedulingV1().PriorityClasses().Create(context.TODO(), &schedulingv1.PriorityClass{ObjectMeta: metav1.ObjectMeta{Name: "pclass1"}, Value: int32(1000)}, metav1.CreateOptions{}) | |||
framework.ExpectEqual(err == nil || apierrors.IsAlreadyExists(err), true) | |||
if err == nil && apierrors.IsAlreadyExists(err){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't the same logic. The following expressions are equivalent:
(err == nil || apierrors.IsAlreadyExists(err)) != true
(err == nil || apierrors.IsAlreadyExists(err)) == false
!(err == nil || apierrors.IsAlreadyExists(err))
err != nil && !apierrors.IsAlreadyExists(err)
The last one is what we want (simplest).
@@ -1138,7 +1142,9 @@ var _ = SIGDescribe("ResourceQuota [Feature:PodPriority]", func() { | |||
ginkgo.It("should verify ResourceQuota's priority class scope (quota set to pod count: 1) against 2 pods with same priority class.", func() { | |||
|
|||
_, err := f.ClientSet.SchedulingV1().PriorityClasses().Create(context.TODO(), &schedulingv1.PriorityClass{ObjectMeta: metav1.ObjectMeta{Name: "pclass2"}, Value: int32(1000)}, metav1.CreateOptions{}) | |||
framework.ExpectEqual(err == nil || apierrors.IsAlreadyExists(err), true) | |||
if err == nil && apierrors.IsAlreadyExists(err){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above.
@@ -1183,7 +1189,9 @@ var _ = SIGDescribe("ResourceQuota [Feature:PodPriority]", func() { | |||
ginkgo.It("should verify ResourceQuota's priority class scope (quota set to pod count: 1) against 2 pods with different priority class.", func() { | |||
|
|||
_, err := f.ClientSet.SchedulingV1().PriorityClasses().Create(context.TODO(), &schedulingv1.PriorityClass{ObjectMeta: metav1.ObjectMeta{Name: "pclass3"}, Value: int32(1000)}, metav1.CreateOptions{}) | |||
framework.ExpectEqual(err == nil || apierrors.IsAlreadyExists(err), true) | |||
if err == nil && apierrors.IsAlreadyExists(err){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above.
@@ -1229,10 +1237,14 @@ var _ = SIGDescribe("ResourceQuota [Feature:PodPriority]", func() { | |||
|
|||
ginkgo.It("should verify ResourceQuota's multiple priority class scope (quota set to pod count: 2) against 2 pods with same priority classes.", func() { | |||
_, err := f.ClientSet.SchedulingV1().PriorityClasses().Create(context.TODO(), &schedulingv1.PriorityClass{ObjectMeta: metav1.ObjectMeta{Name: "pclass5"}, Value: int32(1000)}, metav1.CreateOptions{}) | |||
framework.ExpectEqual(err == nil || apierrors.IsAlreadyExists(err), true) | |||
if err == nil && apierrors.IsAlreadyExists(err){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above.
|
||
_, err = f.ClientSet.SchedulingV1().PriorityClasses().Create(context.TODO(), &schedulingv1.PriorityClass{ObjectMeta: metav1.ObjectMeta{Name: "pclass6"}, Value: int32(1000)}, metav1.CreateOptions{}) | ||
framework.ExpectEqual(err == nil || apierrors.IsAlreadyExists(err), true) | ||
if err == nil && apierrors.IsAlreadyExists(err){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above.
@@ -1284,7 +1296,9 @@ var _ = SIGDescribe("ResourceQuota [Feature:PodPriority]", func() { | |||
ginkgo.It("should verify ResourceQuota's priority class scope (quota set to pod count: 1) against a pod with different priority class (ScopeSelectorOpNotIn).", func() { | |||
|
|||
_, err := f.ClientSet.SchedulingV1().PriorityClasses().Create(context.TODO(), &schedulingv1.PriorityClass{ObjectMeta: metav1.ObjectMeta{Name: "pclass7"}, Value: int32(1000)}, metav1.CreateOptions{}) | |||
framework.ExpectEqual(err == nil || apierrors.IsAlreadyExists(err), true) | |||
if err == nil && apierrors.IsAlreadyExists(err){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above.
@@ -1318,7 +1332,9 @@ var _ = SIGDescribe("ResourceQuota [Feature:PodPriority]", func() { | |||
ginkgo.It("should verify ResourceQuota's priority class scope (quota set to pod count: 1) against a pod with different priority class (ScopeSelectorOpExists).", func() { | |||
|
|||
_, err := f.ClientSet.SchedulingV1().PriorityClasses().Create(context.TODO(), &schedulingv1.PriorityClass{ObjectMeta: metav1.ObjectMeta{Name: "pclass8"}, Value: int32(1000)}, metav1.CreateOptions{}) | |||
framework.ExpectEqual(err == nil || apierrors.IsAlreadyExists(err), true) | |||
if err == nil && apierrors.IsAlreadyExists(err){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above.
@@ -1357,7 +1373,9 @@ var _ = SIGDescribe("ResourceQuota [Feature:PodPriority]", func() { | |||
ginkgo.It("should verify ResourceQuota's priority class scope (cpu, memory quota set) against a pod with same priority class.", func() { | |||
|
|||
_, err := f.ClientSet.SchedulingV1().PriorityClasses().Create(context.TODO(), &schedulingv1.PriorityClass{ObjectMeta: metav1.ObjectMeta{Name: "pclass9"}, Value: int32(1000)}, metav1.CreateOptions{}) | |||
framework.ExpectEqual(err == nil || apierrors.IsAlreadyExists(err), true) | |||
if err == nil && apierrors.IsAlreadyExists(err){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above.
b7fb835
to
0cd58b8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/assign @caesarxuchao For approval. |
/triage accepted |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: caesarxuchao, NikhilSharmaWe, pohly The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/kind cleanup |
What type of PR is this?
Enhancement
What this PR does / why we need it:
For better (more informative) output for developers when test fails. Changed files are under test/e2e/apimachinery for this PR.
Which issue(s) this PR fixes:
Fixes #105678
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: