Skip to content

Conversation

@metlos
Copy link
Contributor

@metlos metlos commented Nov 21, 2024

This a WIP of the proposed changes to unify the test assertions.

@sonarqubecloud
Copy link

Copy link
Contributor

@rsoaresd rsoaresd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@metlos Amazing work! 🚀 Tysm for driving this 🏅

Just minor suggestions:

Comment on lines +81 to +83
actual := &corev1.ConfigMap{}
actual.SetName("actual")
actual.SetLabels(map[string]string{"k": "v"})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's worth doing some aux function for creating the ConfigMap just to avoid duplication (there are two occurrences in these test, and another one in TestNamePredicate. maybe will need it more for the upcoming tests). WDYT?

Suggested change
actual := &corev1.ConfigMap{}
actual.SetName("actual")
actual.SetLabels(map[string]string{"k": "v"})
actual := newConfigMap("actual", map[string]string{"k": "v"})

Comment on lines +13 to +76
func TestExplain(t *testing.T) {
t.Run("with diff", func(t *testing.T) {
// given
actual := &corev1.Secret{}
actual.SetName("actual")

pred := Has(Name("expected"))

// when
expl := Explain(pred, actual)

// then
assert.True(t, strings.HasPrefix(expl, "predicate 'test.named' didn't match the object because of the following differences (- indicates the expected values, + the actual values):"))
assert.Contains(t, expl, "-")
assert.Contains(t, expl, "\"expected\"")
assert.Contains(t, expl, "+")
assert.Contains(t, expl, "\"actual\"")
})

t.Run("without diff", func(t *testing.T) {
// given
actual := &corev1.Secret{}
actual.SetName("actual")

pred := Is[client.Object](&predicateWithoutFixing{})

// when
expl := Explain(pred, actual)

// then
assert.Equal(t, "predicate 'test.predicateWithoutFixing' didn't match the object", expl)
})

t.Run("with a slice", func(t *testing.T) {
actual := []int{1, 2, 3}
pred := MockPredicate[[]int]{}
pred.MatchesFunc = func(v []int) bool {
return false
}
pred.FixToMatchFunc = func(v []int) []int {
return []int{1, 2}
}

expl := Explain(Is[[]int](pred), actual)

assert.True(t, strings.HasPrefix(expl, "predicate 'test.MockPredicate[[]int]' didn't match the object because of the following"))
})

t.Run("with conditions", func(t *testing.T) {
actual := []toolchainv1alpha1.Condition{
{
Type: toolchainv1alpha1.ConditionType("test"),
Status: corev1.ConditionFalse,
Reason: "because",
},
}

pred := ConditionThat(toolchainv1alpha1.ConditionType("test"), HasStatus(corev1.ConditionTrue))

expl := Explain(Has(pred), actual)

assert.True(t, strings.HasPrefix(expl, "predicate 'test.conditionsPredicate' didn't match the object because of the following"))
})
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid duplication of the test cases and to make the tests easier to read, we can use a table-driven approach, if you agree. Something like this:

Suggested change
func TestExplain(t *testing.T) {
t.Run("with diff", func(t *testing.T) {
// given
actual := &corev1.Secret{}
actual.SetName("actual")
pred := Has(Name("expected"))
// when
expl := Explain(pred, actual)
// then
assert.True(t, strings.HasPrefix(expl, "predicate 'test.named' didn't match the object because of the following differences (- indicates the expected values, + the actual values):"))
assert.Contains(t, expl, "-")
assert.Contains(t, expl, "\"expected\"")
assert.Contains(t, expl, "+")
assert.Contains(t, expl, "\"actual\"")
})
t.Run("without diff", func(t *testing.T) {
// given
actual := &corev1.Secret{}
actual.SetName("actual")
pred := Is[client.Object](&predicateWithoutFixing{})
// when
expl := Explain(pred, actual)
// then
assert.Equal(t, "predicate 'test.predicateWithoutFixing' didn't match the object", expl)
})
t.Run("with a slice", func(t *testing.T) {
actual := []int{1, 2, 3}
pred := MockPredicate[[]int]{}
pred.MatchesFunc = func(v []int) bool {
return false
}
pred.FixToMatchFunc = func(v []int) []int {
return []int{1, 2}
}
expl := Explain(Is[[]int](pred), actual)
assert.True(t, strings.HasPrefix(expl, "predicate 'test.MockPredicate[[]int]' didn't match the object because of the following"))
})
t.Run("with conditions", func(t *testing.T) {
actual := []toolchainv1alpha1.Condition{
{
Type: toolchainv1alpha1.ConditionType("test"),
Status: corev1.ConditionFalse,
Reason: "because",
},
}
pred := ConditionThat(toolchainv1alpha1.ConditionType("test"), HasStatus(corev1.ConditionTrue))
expl := Explain(Has(pred), actual)
assert.True(t, strings.HasPrefix(expl, "predicate 'test.conditionsPredicate' didn't match the object because of the following"))
})
}
func TestExplain(t *testing.T) {
tests := []struct {
name string
actual any
predicate Predicate[any]
expectedText string
contains []string
}{
{
name: "with diff",
actual: &corev1.Secret{Name: "actual"},
predicate: Has(Name("expected")),
expectedText: "predicate 'test.named' didn't match the object because of the following differences",
contains: []string{"-", "\"expected\"", "+", "\"actual\""},
},
{
name: "without diff",
actual: &corev1.Secret{Name: "actual"},
predicate: Is[client.Object](&predicateWithoutFixing{}),
expectedText: "predicate 'test.predicateWithoutFixing' didn't match the object",
contains: nil,
},
{
name: "with a slice",
actual: []int{1, 2, 3},
predicate: MockPredicate[[]int]{
MatchesFunc: func(v []int) bool { return false },
FixToMatchFunc: func(v []int) []int {
return []int{1, 2}
},
},
expectedText: "predicate 'test.MockPredicate[[]int]' didn't match the object because of the following",
contains: nil,
},
{
name: "with conditions",
actual: []toolchainv1alpha1.Condition{
{
Type: toolchainv1alpha1.ConditionType("test"),
Status: corev1.ConditionFalse,
Reason: "because",
},
},
predicate: Has(ConditionThat(
toolchainv1alpha1.ConditionType("test"),
HasStatus(corev1.ConditionTrue),
)),
expectedText: "predicate 'test.conditionsPredicate' didn't match the object because of the following",
contains: nil,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// when
expl := Explain(tt.predicate, tt.actual)
// then
assert.True(t, strings.HasPrefix(expl, tt.expectedText))
if tt.contains != nil {
for _, part := range tt.contains {
assert.Contains(t, expl, part)
}
}
})
}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants