Repository: kubernetes
Issue: test simplification needed: statefulset
Labels: area/test,kind/cleanup,sig/apps,help wanted,sig/testing,needs-triage

Table-driven tests are awesome, but they can be very verbose and obscure what's actually being tested in each case.  This issue is for stateful set in particular but the same pattern could be applied in many places.

Example test today - pkg/apis/apps/validation/validation_test.go.  We find large blocks like:

```
    successCases := []testCase{
        {
            name: "update replica count",
            old: apps.StatefulSet{
                ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
                Spec: apps.StatefulSetSpec{
                    PodManagementPolicy: apps.OrderedReadyPodManagement,
                    Selector:            &metav1.LabelSelector{MatchLabels: validLabels},
                    Template:            validPodTemplate.Template,
                    UpdateStrategy:      apps.StatefulSetUpdateStrategy{Type: apps.RollingUpdateStatefulSetStrategyType},
                },
            },
            update: apps.StatefulSet{
                ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
                Spec: apps.StatefulSetSpec{
                    PodManagementPolicy: apps.OrderedReadyPodManagement,
                    Replicas:            3,
                    Selector:            &metav1.LabelSelector{MatchLabels: validLabels},
                    Template:            validPodTemplate.Template,
                    UpdateStrategy:      apps.StatefulSetUpdateStrategy{Type: apps.RollingUpdateStatefulSetStrategyType},
                },
            },
        },
        {
            name: "update containers 1",
            old: apps.StatefulSet{
                ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
                Spec: apps.StatefulSetSpec{
                    PodManagementPolicy: apps.OrderedReadyPodManagement,
                    Selector:            &metav1.LabelSelector{MatchLabels: validLabels},
                    Template:            validPodTemplate.Template,
                    UpdateStrategy:      apps.StatefulSetUpdateStrategy{Type: apps.RollingUpdateStatefulSetStrategyType},
                },
            },
            update: apps.StatefulSet{
                ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
                Spec: apps.StatefulSetSpec{
                    PodManagementPolicy: apps.OrderedReadyPodManagement,
                    Selector:            &metav1.LabelSelector{MatchLabels: validLabels},
                    Template:            addContainersValidTemplate.Template,
                    UpdateStrategy:      apps.StatefulSetUpdateStrategy{Type: apps.RollingUpdateStatefulSetStrategyType},
                },
            },
        },
        {
            name: "update containers 2",
            old: apps.StatefulSet{
                ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
                Spec: apps.StatefulSetSpec{
                    PodManagementPolicy: apps.OrderedReadyPodManagement,
                    Selector:            &metav1.LabelSelector{MatchLabels: validLabels},
                    Template:            addContainersValidTemplate.Template,
                    UpdateStrategy:      apps.StatefulSetUpdateStrategy{Type: apps.RollingUpdateStatefulSetStrategyType},
                },
            },
            update: apps.StatefulSet{
                ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
                Spec: apps.StatefulSetSpec{
                    PodManagementPolicy: apps.OrderedReadyPodManagement,
                    Selector:            &metav1.LabelSelector{MatchLabels: validLabels},
                    Template:            validPodTemplate.Template,
                    UpdateStrategy:      apps.StatefulSetUpdateStrategy{Type: apps.RollingUpdateStatefulSetStrategyType},
                },
            },
        },
```

This goes on for hundreds of lines.  To understanc this test, you have to really read and visually compare multi-line blocks, which humans are BAD at.  I made a similar transformation for Service tests, so the result now looks like:

```
    testCases := []struct {
        name   string
        input  runtime.Object
        expect runtime.Object
    }{{   
        name:  "single v4",
        input: svctest.MakeService("foo", svctest.SetClusterIPs("10.0.0.1")),
        expect: svctest.MakeService("foo", svctest.SetClusterIPs("10.0.0.1"),
            svctest.SetIPFamilyPolicy(api.IPFamilyPolicySingleStack),
            svctest.SetIPFamilies(api.IPv4Protocol)),
    }, {  
        name:  "single v6",
        input: svctest.MakeService("foo", svctest.SetClusterIPs("2000::1")),
        expect: svctest.MakeService("foo", svctest.SetClusterIPs("2000::1"),
            svctest.SetIPFamilyPolicy(api.IPFamilyPolicySingleStack),
            svctest.SetIPFamilies(api.IPv6Protocol)),
    }, {  
        name:  "missing clusterIPs v4",
        input: svctest.MakeService("foo", svctest.SetClusterIP("10.0.0.1")),
        expect: svctest.MakeService("foo", svctest.SetClusterIPs("10.0.0.1"),
            svctest.SetIPFamilyPolicy(api.IPFamilyPolicySingleStack),
            svctest.SetIPFamilies(api.IPv4Protocol)),
    }, {  
        name:  "missing clusterIPs v6",
        input: svctest.MakeService("foo", svctest.SetClusterIP("2000::1")),
        expect: svctest.MakeService("foo", svctest.SetClusterIPs("2000::1"),
            svctest.SetIPFamilyPolicy(api.IPFamilyPolicySingleStack),
            svctest.SetIPFamilies(api.IPv6Protocol)),
```

The `MakeService` function returns a fully formed, valid object.  The optional variadic arguments are explicitly noteworthy because they are CHANGES from the default.  When I look at a table like this it's obvious what is being modulated for each case.  It's also far denser with less boilerplate that occasionally needs to be updated.

I'll bet a similar transformation could be made for statefulset validation and a dozen other places