radius-project / radius

Radius is a cloud-native, portable application platform that makes app development easier for teams building cloud-native apps.
https://radapp.io
Apache License 2.0
1.46k stars 93 forks source link

Examine Data Model->Versioned API Model conversion tests for bugs #5025

Open rynowak opened 1 year ago

rynowak commented 1 year ago

Summary

As part of some recent work, @mishrapratikshya and I noticed that the test for the environment conversion have incorrect assertions. We identified this issue because the conversion code was obviously incorrect from inspection, but the tests were passing.

Example

In this example r is the test input (the data model) and versioned is the test output (the versioned model). The assertions test a mix of r and versioned.

require.Equal(t, "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/radius-test-rg/providers/Applications.Core/environments/env0", r.ID)
                require.Equal(t, "env0", r.Name)
                require.Equal(t, "Applications.Core/environments", r.Type)
                require.Equal(t, "kubernetes", string(r.Properties.Compute.Kind))
                require.Equal(t, "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/testGroup/providers/Microsoft.ContainerService/managedClusters/radiusTestCluster", r.Properties.Compute.KubernetesCompute.ResourceID)
                require.Equal(t, 1, len(r.Properties.Recipes))
                require.Equal(t, linkrp.MongoDatabasesResourceType, r.Properties.Recipes["cosmos-recipe"].LinkType)
                require.Equal(t, "br:sampleregistry.azureacr.io/radius/recipes/cosmosdb", r.Properties.Recipes["cosmos-recipe"].TemplatePath)
                require.Equal(t, map[string]any{"throughput": float64(400)}, r.Properties.Recipes["cosmos-recipe"].Parameters)
                require.Equal(t, "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/testGroup", r.Properties.Providers.Azure.Scope)
                require.Equal(t, "kubernetesMetadata", *versioned.Properties.Extensions[0].GetExtension().Kind)
                require.Equal(t, 1, len(versioned.Properties.Extensions))

From: https://github.com/project-radius/radius/blob/main/pkg/corerp/api/v20220315privatepreview/environment_conversion_test.go#L259-L270

Next Steps

We are fixing the bugs in the environment conversion tests as part of https://github.com/project-radius/radius/pull/5013/files

However, This problem pre-dates the functionality we were working on - the test already had this bug. Since many of the conversion tests were copy-pasted and then modified, we suspect that more of our tests may have similar problems. We should inspect all of our converter tests (~20 files) and verify that they are testing the correct things.

AB#5867

rynowak commented 1 year ago

/cc @youngbupark