Repository: kubernetes/kubernetes
File: staging/src/k8s.io/dynamic-resource-allocation/api/internal/test/roundtrip.go (Go)

Code before:
// Package test provides test utilities for DRA API conversion testing.
package test

import (
	"math/rand"
	"strings"
	"testing"

	"github.com/google/go-cmp/cmp"

	"k8s.io/apimachinery/pkg/api/apitesting/fuzzer"
	apiequality "k8s.io/apimachinery/pkg/api/equality"
	genericfuzzer "k8s.io/apimachinery/pkg/apis/meta/fuzzer"
	"k8s.io/apimachinery/pkg/runtime"
	"k8s.io/apimachinery/pkg/runtime/schema"
	"k8s.io/apimachinery/pkg/runtime/serializer"
	"sigs.k8s.io/randfill"
)

// FuzzIterations is the number of fuzz iterations to run for each type.
const FuzzIterations = 100

// NewFiller creates a new randfill.Filler for fuzzing objects.
// It uses NilChance(0.2) to ensure more fields are populated while still
// testing nil code paths, and NumElements(1, 2) to ensure slices and maps
// have at least one element.
func NewFiller(t *testing.T, scheme *runtime.Scheme, customFuncs fuzzer.FuzzerFuncs) *randfill.Filler {
	seed := rand.Int63()
	t.Logf("Using seed: %d", seed)
	codecs := serializer.NewCodecFactory(scheme)
	f := randfill.New().NilChance(0.2).NumElements(1, 2).RandSource(rand.NewSource(seed))
	f.Funcs(fuzzer.MergeFuzzerFuncs(genericfuzzer.Funcs, customFuncs)(codecs)...)
	return f
}

// ConversionRoundTrip tests that all non-list types in srcGV can be converted
// to dstGV and back without loss of information.
func ConversionRoundTrip(t *testing.T, scheme *runtime.Scheme, filler *randfill.Filler, srcGV, dstGV schema.GroupVersion) {
	t.Helper()

	tested := 0
	for kind := range scheme.KnownTypes(srcGV) {
		if strings.HasSuffix(kind, "List") {
			continue
		}

		srcGVK := srcGV.WithKind(kind)
		dstGVK := dstGV.WithKind(kind)
		if _, err := scheme.New(dstGVK); err != nil {
			// Kind does not exist in the destination version.
			continue

Review comment (bug):
This doesn't seem to make a difference, though. I see the same `go test -cover` stats with and without this last commit.

I wonder whether there's some other explanation. The code in v1beta1 which isn't covered includes e.g. `autoConvert_v1_AllocationResult_To_v1beta1_AllocationResult`, which is for a type embedded in the top-level ResourceClaim type (claim.Status.Allocation).

In a debugger I see that the claim has that field set and it's non-empty. So why do we not get `autoConvert_v1_AllocationResult_To_v1beta1_AllocationResult` covered?

Because (insert drum roll here)... it's not needed.

https://github.com/kubernetes/kubernetes/blob/214ef42236333fc2152eeb0747a1e81fdbcd729d/staging/src/k8s.io/dynamic-resource-allocation/api/v1beta1/zz_generated.conversion.go#L1343-L1348

That converts without calling any of the generated functions for the embedded fields.

We have two options:
- Change the test logic from "iterate over gvr" to "iterate over conversion functions to find their types".
- Generate conversion functions only for top-level types.

At first glance the second option looks more appealing because it eliminates dead code that is never called. But while I *think* that we only convert top-level types, it is impossible to be sure because this package might be used in unexpected ways.

I don't know whether the first option is feasible. Probably not, I don't see an accessor for registered conversions. If you don't want to spend more time on this then we can also let it be and merge this PR more-or-less as-is. Just drop the last commit because it doesn't achieve what it was meant to do.

Code after:
// Package test provides test utilities for DRA API conversion testing.
package test

import (
	"math/rand"
	"strings"
	"testing"

	"github.com/google/go-cmp/cmp"

	"k8s.io/apimachinery/pkg/api/apitesting/fuzzer"
	apiequality "k8s.io/apimachinery/pkg/api/equality"
	genericfuzzer "k8s.io/apimachinery/pkg/apis/meta/fuzzer"
	"k8s.io/apimachinery/pkg/runtime"
	"k8s.io/apimachinery/pkg/runtime/schema"
	"k8s.io/apimachinery/pkg/runtime/serializer"
	"sigs.k8s.io/randfill"
)

// FuzzIterations is the number of fuzz iterations to run for each type.
const FuzzIterations = 100

// NewFiller creates a new randfill.Filler for fuzzing objects.
func NewFiller(t *testing.T, scheme *runtime.Scheme, customFuncs fuzzer.FuzzerFuncs) *randfill.Filler {
	seed := rand.Int63()
	t.Logf("Using seed: %d", seed)
	codecs := serializer.NewCodecFactory(scheme)
	return fuzzer.FuzzerFor(
		fuzzer.MergeFuzzerFuncs(genericfuzzer.Funcs, customFuncs),
		rand.NewSource(seed),
		codecs,
	)
}

// ConversionRoundTrip tests that all non-list types in srcGV can be converted
// to dstGV and back without loss of information.
func ConversionRoundTrip(t *testing.T, scheme *runtime.Scheme, filler *randfill.Filler, srcGV, dstGV schema.GroupVersion) {
	t.Helper()

	tested := 0
	for kind := range scheme.KnownTypes(srcGV) {
		if strings.HasSuffix(kind, "List") {
			continue
		}

		srcGVK := srcGV.WithKind(kind)
		dstGVK := dstGV.WithKind(kind)
		if _, err := scheme.New(dstGVK); err != nil {
			// Kind does not exist in the destination version.
			continue
		}