k8ssandra-operator icon indicating copy to clipboard operation
k8ssandra-operator copied to clipboard

Handle panics in integration test subtests

Open jsanda opened this issue 3 years ago • 1 comments

Background

This is related to #332.

Integration tests make extensive use of subtests. There are a couple of problems caused by not handling panics. First, we end up with lingering kube api-server and etcd processes. Secondly, subsequent subtests will not run.

Here is an initial example:

func TestFailures(t *testing.T) {
	t.Run("Fail1", ftest("one", fail))
	t.Run("Fail2", ftest("two", fail))
}

type testFunc func(t *testing.T, msg string)

func ftest(msg string, f testFunc) func(t *testing.T) {
	return func(t *testing.T) {
		f(t, msg)
	}
}

func fail(t *testing.T, msg string) {
	panic(msg)
}

If you run this, the Fail2 test won't run. We can fix this by handling panics.

func TestFailures(t *testing.T) {
	t.Run("Fail1", ftest("one", fail))
	t.Run("Fail2", ftest("two", fail))
}

type testFunc func(t *testing.T, msg string)

func ftest(msg string, f testFunc) func(t *testing.T) {
	return func(t *testing.T) {
		defer handlePanic(t)
		f(t, msg)
	}
}

func fail(t *testing.T, msg string) {
	panic(msg)
}

func handlePanic(t *testing.T) {
	if r := recover(); r != nil {
		t.Fatalf("recovered from panic: %v", r)
	}
}

Now both subtests run and are reported as failures. Good to go so far. Now let's at another subtest which in turn adds its own subtest:

func TestFailures(t *testing.T) {
	t.Run("Fail1", ftest("one", fail))
	t.Run("FailNotHandled", ftest("nested 1", func(t *testing.T, msg string) {
		t.Run("nested 2", func(t *testing.T) {
			panic(msg)
		})
	}))
	t.Run("Fail2", ftest("two", fail))
}

type testFunc func(t *testing.T, msg string)

func ftest(msg string, f testFunc) func(t *testing.T) {
	return func(t *testing.T) {
		defer handlePanic(t)
		f(t, msg)
	}
}

func fail(t *testing.T, msg string) {
	panic(msg)
}

func handlePanic(t *testing.T) {
	if r := recover(); r != nil {
		t.Fatalf("recovered from panic: %v", r)
	}
}

If you run this you will see that the panic in nested 2 isn't handled and that Fail2 does not run. We get the desired behavior in the final example:

func TestFailures(t *testing.T) {
	t.Run("Fail1", ftest("one", fail))
	t.Run("FailNotHandled", ftest("nested 1", func(t *testing.T, msg string) {
		t.Run("nested 2", func(t *testing.T) {
			defer handlePanic(t)
			panic(msg)
		})
	}))
	t.Run("Fail2", ftest("two", fail))
}

type testFunc func(t *testing.T, msg string)

func ftest(msg string, f testFunc) func(t *testing.T) {
	return func(t *testing.T) {
		defer handlePanic(t)
		f(t, msg)
	}
}

func fail(t *testing.T, msg string) {
	panic(msg)
}

func handlePanic(t *testing.T) {
	if r := recover(); r != nil {
		t.Fatalf("recovered from panic: %v", r)
	}
}

We recover from all panics and all tests run. This is a simplified example of what we are dealing with in our integration tests

Short Term Solution

We can update all the necessary places where nested subtests are created so that they handle panics. That's what I am proposing we do for this ticket. It's not a robust solution though because we will have introduce recovery logic in any number of places.

Long Term Solution

Our integration test framework needs some refactoring which I will address in other tickets. There should be a common interface for our custom test functions. That will make it easier to centralize the creation of the actual test functions which in turn will allow us to add logic for recovering from panics in a single place. For reference consider the composability of http handlers as an example.

Environment

  • K8ssandra Operator version:

    `v0.4.0

┆Issue is synchronized with this Jira Story by Unito ┆Issue Number: K8OP-134

jsanda avatar Feb 09 '22 05:02 jsanda

I found out that using the following recover func allows to get the whole stack trace mentioning on which line the panic happened:

                defer func() {
			if r := recover(); r != nil {
				assert.Nil(t, r, "Recovered from panic, %v", r)
			}
		}()

Gives the following output:

--- FAIL: TestK8ssandraCluster (140.77s)
    --- FAIL: TestK8ssandraCluster/CreateSingleDcCluster (2.56s)
        k8ssandracluster_controller_test.go:1796: check finalizer added to K8ssandraCluster
        k8ssandracluster_controller_test.go:1733: check that the default superuser secret is created
        k8ssandracluster_controller_test.go:1809: check ReplicatedSecret reconciled
        k8ssandracluster_controller_test.go:1766: check that the k8ssandra.io/initial-system-replication annotation is set
        k8ssandracluster_controller_test.go:156: check that the datacenter was created
        k8ssandracluster_controller_test.go:162: update datacenter status to scaling up
        testenv.go:242: 
                Error Trace:    testenv.go:242
                                                        panic.go:1038
                                                        panic.go:221
                                                        signal_unix.go:735
                                                        k8ssandracluster_controller_test.go:173
                                                        testenv.go:246
                Error:          Expected nil, but got: "invalid memory address or nil pointer dereference"
                Test:           TestK8ssandraCluster/CreateSingleDcCluster
                Messages:       Recovered from panic, runtime error: invalid memory address or nil pointer dereference

Otherwise we just get the error message but don't know where the issue precisely is.

adejanovski avatar Feb 10 '22 11:02 adejanovski