Skip to content

Commit 1367c76

Browse files
[Feature Group] Unit Testing (#68)
Note: This PR is meant to be merged on top of [PR #66](#66). Summary: Added unit test to feature group for Create, ReadOne, and Delete (Feature Group does not support Update). Includes testing of state transitions. Code coverage for Feature Group = 53.9%. Total Code Coverage: 3.1% Files Added: - pkg/resource/feature_group/testdata/feature_group/v1alpha1 - Contains yaml files to be used in the desired and expected fields for testing scenarios in test_suite.yaml. (8 files) - fg_before_create.yaml - fg_create_failed_state.yaml - fg_create_initiated.yaml - fg_created_state.yaml - fg_creating_state.yaml - fg_deleting_state.yaml - fg_invalid_before_create.yaml - fg_invalid_create_attempted.yaml - pkg/resource/feature_group/testdata/feature_group/create - Contains json files to be used in the output_fixture fields for testing scenarios in test_suite.yaml which invoke the Create operation. (1 file) - fg_before_create.json - pkg/resource/feature_group/testdata/feature_group/read_one- Contains json files to be used in the output_fixture fields for testing scenarios in test_suite.yaml which invoke the ReadOne operation. (4 files) - fg_create_failed.json - fg_created.json - fg_creating.json - fg_deleting.json Files Edited: - pkg/common/custom_conditions.go - fixed bug revealed by unit test (needed to remove the space in "Create Failed" to do a compare to the terminal state) - pkg/resource/feature_group/manager_test_suite_test.go - Added YamlEqual that is used in test_suite_runner.go and compares the expected and actual resource as yaml files, presenting a diff output (and a copy of the actual output) which is easy to understand and use to modify the yaml files for testing. Also changed the Equal function to ignore the "LastTransitionTime" since as a timestamp it is updated while testing, and cannot be correctly preset in the expected output yaml file. - pkg/resource/feature_group/testdata/test_suite.yaml - Added state transition testing. - pkg/testutil/test_suite_runner.go - Added assert for the YamlEqual function to test the outputs of unit testing, and added YamlEqual to the TestRunnerDelegate interface. Also added option for unit testing scenario sdkapi mocking to have no provided output_fixture or error, and therefore mock both as being nil.
1 parent e1682fb commit 1367c76

15 files changed

+530
-40
lines changed

pkg/resource/feature_group/manager_test_suite_test.go

Lines changed: 92 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,12 @@ package feature_group
1616
import (
1717
"errors"
1818
"fmt"
19+
"os"
20+
"io/ioutil"
21+
"os/exec"
22+
23+
ackv1alpha1 "github.com/aws-controllers-k8s/runtime/apis/core/v1alpha1"
24+
"github.com/ghodss/yaml"
1925
mocksvcsdkapi "github.com/aws-controllers-k8s/sagemaker-controller/test/mocks/aws-sdk-go/sagemaker"
2026
"github.com/aws-controllers-k8s/sagemaker-controller/pkg/testutil"
2127
acktypes "github.com/aws-controllers-k8s/runtime/pkg/types"
@@ -29,6 +35,12 @@ import (
2935
"go.uber.org/zap/zapcore"
3036
)
3137

38+
var (
39+
DefaultTimestamp = "0001-01-01T00:00:00Z"
40+
ReplaceTimestampRegExp = "s/\"[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z\"/\"" + DefaultTimestamp + "\"/"
41+
TestDataDirectory = "testdata"
42+
)
43+
3244
// provideResourceManagerWithMockSDKAPI accepts MockSageMakerAPI and returns pointer to resourceManager
3345
// the returned resourceManager is configured to use mockapi api.
3446
func provideResourceManagerWithMockSDKAPI(mockSageMakerAPI *mocksvcsdkapi.SageMakerAPI) *resourceManager {
@@ -48,8 +60,8 @@ func provideResourceManagerWithMockSDKAPI(mockSageMakerAPI *mocksvcsdkapi.SageMa
4860
}
4961
}
5062

51-
// TestDeclarativeTestSuite runs the test suite for feature group
52-
func TestDeclarativeTestSuite(t *testing.T) {
63+
// TestFeatureGroupTestSuite runs the test suite for feature group
64+
func TestFeatureGroupTestSuite(t *testing.T) {
5365
defer func() {
5466
if r := recover(); r != nil {
5567
fmt.Println(testutil.RecoverPanicString, r)
@@ -88,7 +100,7 @@ func (d *testRunnerDelegate) EmptyServiceAPIOutput(apiName string) (interface{},
88100
switch apiName {
89101
case "CreateFeatureGroupWithContext":
90102
var output svcsdk.CreateFeatureGroupOutput
91-
return &output, nil
103+
return &output, nil
92104
case "DeleteFeatureGroupWithContext":
93105
var output svcsdk.DeleteFeatureGroupOutput
94106
return &output, nil
@@ -102,7 +114,8 @@ func (d *testRunnerDelegate) EmptyServiceAPIOutput(apiName string) (interface{},
102114
func (d *testRunnerDelegate) Equal(a acktypes.AWSResource, b acktypes.AWSResource) bool {
103115
ac := a.(*resource)
104116
bc := b.(*resource)
105-
opts := []cmp.Option{cmpopts.EquateEmpty()}
117+
// Ignore LastTransitionTime since it gets updated each run.
118+
opts := []cmp.Option{cmpopts.EquateEmpty(), cmpopts.IgnoreFields(ackv1alpha1.Condition{}, "LastTransitionTime")}
106119

107120
if cmp.Equal(ac.ko.Status, bc.ko.Status, opts...) {
108121
return true
@@ -112,3 +125,78 @@ func (d *testRunnerDelegate) Equal(a acktypes.AWSResource, b acktypes.AWSResourc
112125
return false
113126
}
114127
}
128+
129+
// Checks to see if the given yaml file, with name stored as expectation,
130+
// matches the yaml marshal of the AWSResource stored as actual.
131+
func (d *testRunnerDelegate) YamlEqual(expectation string, actual acktypes.AWSResource) bool {
132+
// Get the file name of the expected yaml.
133+
expectedYamlFileName := TestDataDirectory + "/" + expectation
134+
135+
// Build a tmp file for the actual yaml.
136+
actualResource := actual.(*resource)
137+
actualYamlByteArray, _ := yaml.Marshal(actualResource.ko)
138+
actualYamlFileName := buildTmpFile("actualYaml", actualYamlByteArray)
139+
defer os.Remove(actualYamlFileName)
140+
if "" == actualYamlFileName {
141+
fmt.Printf("Could not create temporary actual file.\n")
142+
return false
143+
}
144+
145+
// Replace Timestamps that would show up as different.
146+
_, err := exec.Command("sed", "-r", "-i", ReplaceTimestampRegExp, actualYamlFileName).Output()
147+
if isExecCommandError(err) {
148+
return false
149+
}
150+
151+
output,err := exec.Command("diff", "-c", expectedYamlFileName, actualYamlFileName).Output()
152+
if isExecCommandError(err) {
153+
return false
154+
}
155+
156+
if len(output) > 0 {
157+
actualOutput,err := exec.Command("cat", actualYamlFileName).Output()
158+
if isExecCommandError(err) {
159+
return false
160+
}
161+
fmt.Printf("\nExpected Yaml File Name: " + expectedYamlFileName + "\n")
162+
fmt.Printf("\nActual Output Yaml:\n" + string(actualOutput) + "\n")
163+
fmt.Printf("Diff From Expected:\n" + string(output) + "\n")
164+
return false
165+
}
166+
return true
167+
}
168+
169+
func buildTmpFile(fileNameBase string, contents []byte) string {
170+
newTmpFile, err := ioutil.TempFile(TestDataDirectory, fileNameBase)
171+
if err != nil {
172+
fmt.Println(err)
173+
return ""
174+
}
175+
if _, err := newTmpFile.Write(contents); err != nil {
176+
fmt.Println(err)
177+
return ""
178+
}
179+
if err := newTmpFile.Close(); err != nil {
180+
fmt.Println(err)
181+
return ""
182+
}
183+
return newTmpFile.Name()
184+
}
185+
186+
// isExecCommandError returns true if an error
187+
// that is not an ExitError is found.
188+
func isExecCommandError(err error) bool {
189+
if err == nil {
190+
return false
191+
}
192+
switch err.(type) {
193+
case *exec.ExitError:
194+
// ExitError is expected.
195+
return false
196+
default:
197+
// Couldn't run diff.
198+
fmt.Printf("Exec Command Error: ")
199+
fmt.Println(err)
200+
return true
201+
}
202+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"FeatureGroupArn": "arn:aws:sagemaker:us-west-2:123456789012:feature-group/unit-testing-feature-group"
3+
}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
{
2+
"CreationTime": "2021-07-28T00:32:45.698Z",
3+
"Description": null,
4+
"EventTimeFeatureName": "EventTime",
5+
"FailureReason": null,
6+
"FeatureDefinitions": [
7+
{
8+
"FeatureName": "TransactionID",
9+
"FeatureType": "Integral"
10+
},
11+
{
12+
"FeatureName": "EventTime",
13+
"FeatureType": "Fractional"
14+
}
15+
],
16+
"FeatureGroupArn": "arn:aws:sagemaker:us-west-2:123456789012:feature-group/unit-testing-feature-group",
17+
"FeatureGroupName": "unit-testing-feature-group",
18+
"FeatureGroupStatus": "CreateFailed",
19+
"NextToken": null,
20+
"OfflineStoreConfig": {
21+
"DataCatalogConfig": null,
22+
"DisableGlueTableCreation": false,
23+
"S3StorageConfig": {
24+
"KmsKeyId": null,
25+
"ResolvedOutputS3Uri": "s3://source-data-bucket-592697580195-us-west-2/sagemaker/feature-group-data/123456789012/sagemaker/us-west-2/offline-store/unit-testing-feature-group-1627432365/data",
26+
"S3Uri": "s3://source-data-bucket-592697580195-us-west-2/sagemaker/feature-group-data"
27+
}
28+
},
29+
"OfflineStoreStatus": null,
30+
"OnlineStoreConfig": null,
31+
"RecordIdentifierFeatureName": "TransactionID",
32+
"RoleArn": "arn:aws:iam::123456789012:role/ack-sagemaker-execution-role"
33+
}
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
{
2+
"CreationTime": "2021-07-28T00:45:46.813Z",
3+
"Description": null,
4+
"EventTimeFeatureName": "EventTime",
5+
"FailureReason": null,
6+
"FeatureDefinitions": [
7+
{
8+
"FeatureName": "TransactionID",
9+
"FeatureType": "Integral"
10+
},
11+
{
12+
"FeatureName": "EventTime",
13+
"FeatureType": "Fractional"
14+
}
15+
],
16+
"FeatureGroupArn": "arn:aws:sagemaker:us-west-2:123456789012:feature-group/unit-testing-feature-group",
17+
"FeatureGroupName": "unit-testing-feature-group",
18+
"FeatureGroupStatus": "Created",
19+
"NextToken": null,
20+
"OfflineStoreConfig": {
21+
"DataCatalogConfig": {
22+
"Catalog": "AwsDataCatalog",
23+
"Database": "sagemaker_featurestore",
24+
"TableName": "unit-testing-feature-group-1627433146"
25+
},
26+
"DisableGlueTableCreation": false,
27+
"S3StorageConfig": {
28+
"KmsKeyId": null,
29+
"ResolvedOutputS3Uri": "s3://source-data-bucket-592697580195-us-west-2/sagemaker/feature-group-data/123456789012/sagemaker/us-west-2/offline-store/unit-testing-feature-group-1627433146/data",
30+
"S3Uri": "s3://source-data-bucket-592697580195-us-west-2/sagemaker/feature-group-data"
31+
}
32+
},
33+
"OfflineStoreStatus": null,
34+
"OnlineStoreConfig": null,
35+
"RecordIdentifierFeatureName": "TransactionID",
36+
"RoleArn": "arn:aws:iam::123456789012:role/ack-sagemaker-execution-role"
37+
}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
{
2+
"CreationTime": "2021-07-28T00:32:45.698Z",
3+
"Description": null,
4+
"EventTimeFeatureName": "EventTime",
5+
"FailureReason": null,
6+
"FeatureDefinitions": [
7+
{
8+
"FeatureName": "TransactionID",
9+
"FeatureType": "Integral"
10+
},
11+
{
12+
"FeatureName": "EventTime",
13+
"FeatureType": "Fractional"
14+
}
15+
],
16+
"FeatureGroupArn": "arn:aws:sagemaker:us-west-2:123456789012:feature-group/unit-testing-feature-group",
17+
"FeatureGroupName": "unit-testing-feature-group",
18+
"FeatureGroupStatus": "Creating",
19+
"NextToken": null,
20+
"OfflineStoreConfig": {
21+
"DataCatalogConfig": null,
22+
"DisableGlueTableCreation": false,
23+
"S3StorageConfig": {
24+
"KmsKeyId": null,
25+
"ResolvedOutputS3Uri": "s3://source-data-bucket-592697580195-us-west-2/sagemaker/feature-group-data/123456789012/sagemaker/us-west-2/offline-store/unit-testing-feature-group-1627432365/data",
26+
"S3Uri": "s3://source-data-bucket-592697580195-us-west-2/sagemaker/feature-group-data"
27+
}
28+
},
29+
"OfflineStoreStatus": null,
30+
"OnlineStoreConfig": null,
31+
"RecordIdentifierFeatureName": "TransactionID",
32+
"RoleArn": "arn:aws:iam::123456789012:role/ack-sagemaker-execution-role"
33+
}
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
{
2+
"CreationTime": "2021-07-28T00:45:46.813Z",
3+
"Description": null,
4+
"EventTimeFeatureName": "EventTime",
5+
"FailureReason": null,
6+
"FeatureDefinitions": [
7+
{
8+
"FeatureName": "TransactionID",
9+
"FeatureType": "Integral"
10+
},
11+
{
12+
"FeatureName": "EventTime",
13+
"FeatureType": "Fractional"
14+
}
15+
],
16+
"FeatureGroupArn": "arn:aws:sagemaker:us-west-2:123456789012:feature-group/unit-testing-feature-group",
17+
"FeatureGroupName": "unit-testing-feature-group",
18+
"FeatureGroupStatus": "Deleting",
19+
"NextToken": null,
20+
"OfflineStoreConfig": {
21+
"DataCatalogConfig": {
22+
"Catalog": "AwsDataCatalog",
23+
"Database": "sagemaker_featurestore",
24+
"TableName": "unit-testing-feature-group-1627433146"
25+
},
26+
"DisableGlueTableCreation": false,
27+
"S3StorageConfig": {
28+
"KmsKeyId": null,
29+
"ResolvedOutputS3Uri": "s3://source-data-bucket-592697580195-us-west-2/sagemaker/feature-group-data/123456789012/sagemaker/us-west-2/offline-store/unit-testing-feature-group-1627433146/data",
30+
"S3Uri": "s3://source-data-bucket-592697580195-us-west-2/sagemaker/feature-group-data"
31+
}
32+
},
33+
"OfflineStoreStatus": null,
34+
"OnlineStoreConfig": null,
35+
"RecordIdentifierFeatureName": "TransactionID",
36+
"RoleArn": "arn:aws:iam::123456789012:role/ack-sagemaker-execution-role"
37+
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
apiVersion: sagemaker.services.k8s.aws/v1alpha1
2+
kind: FeatureGroup
3+
metadata:
4+
name: unit-testing-feature-group
5+
spec:
6+
eventTimeFeatureName: EventTime
7+
featureDefinitions:
8+
- featureName: TransactionID
9+
featureType: Integral
10+
- featureName: EventTime
11+
featureType: Fractional
12+
featureGroupName: unit-testing-feature-group
13+
offlineStoreConfig:
14+
s3StorageConfig:
15+
s3URI: s3://source-data-bucket-592697580195-us-west-2/sagemaker/feature-group-data
16+
recordIdentifierFeatureName: TransactionID
17+
roleARN: arn:aws:iam::123456789012:role/ack-sagemaker-execution-role
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
apiVersion: sagemaker.services.k8s.aws/v1alpha1
2+
kind: FeatureGroup
3+
metadata:
4+
creationTimestamp: null
5+
name: unit-testing-feature-group
6+
spec:
7+
eventTimeFeatureName: EventTime
8+
featureDefinitions:
9+
- featureName: TransactionID
10+
featureType: Integral
11+
- featureName: EventTime
12+
featureType: Fractional
13+
featureGroupName: unit-testing-feature-group
14+
offlineStoreConfig:
15+
disableGlueTableCreation: false
16+
s3StorageConfig:
17+
resolvedOutputS3URI: s3://source-data-bucket-592697580195-us-west-2/sagemaker/feature-group-data/123456789012/sagemaker/us-west-2/offline-store/unit-testing-feature-group-1627432365/data
18+
s3URI: s3://source-data-bucket-592697580195-us-west-2/sagemaker/feature-group-data
19+
recordIdentifierFeatureName: TransactionID
20+
roleARN: arn:aws:iam::123456789012:role/ack-sagemaker-execution-role
21+
status:
22+
ackResourceMetadata:
23+
arn: arn:aws:sagemaker:us-west-2:123456789012:feature-group/unit-testing-feature-group
24+
ownerAccountID: ""
25+
conditions:
26+
- lastTransitionTime: "0001-01-01T00:00:00Z"
27+
message: FeatureGroup is in CreateFailed status.
28+
status: "True"
29+
type: ACK.ResourceSynced
30+
- lastTransitionTime: "0001-01-01T00:00:00Z"
31+
message: 'FeatureGroup status reached terminal state: CreateFailed. Check the
32+
FailureReason.'
33+
status: "True"
34+
type: ACK.Terminal
35+
featureGroupStatus: CreateFailed
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
apiVersion: sagemaker.services.k8s.aws/v1alpha1
2+
kind: FeatureGroup
3+
metadata:
4+
creationTimestamp: null
5+
name: unit-testing-feature-group
6+
spec:
7+
eventTimeFeatureName: EventTime
8+
featureDefinitions:
9+
- featureName: TransactionID
10+
featureType: Integral
11+
- featureName: EventTime
12+
featureType: Fractional
13+
featureGroupName: unit-testing-feature-group
14+
offlineStoreConfig:
15+
s3StorageConfig:
16+
s3URI: s3://source-data-bucket-592697580195-us-west-2/sagemaker/feature-group-data
17+
recordIdentifierFeatureName: TransactionID
18+
roleARN: arn:aws:iam::123456789012:role/ack-sagemaker-execution-role
19+
status:
20+
ackResourceMetadata:
21+
arn: arn:aws:sagemaker:us-west-2:123456789012:feature-group/unit-testing-feature-group
22+
ownerAccountID: ""
23+
conditions: []
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
apiVersion: sagemaker.services.k8s.aws/v1alpha1
2+
kind: FeatureGroup
3+
metadata:
4+
creationTimestamp: null
5+
name: unit-testing-feature-group
6+
spec:
7+
eventTimeFeatureName: EventTime
8+
featureDefinitions:
9+
- featureName: TransactionID
10+
featureType: Integral
11+
- featureName: EventTime
12+
featureType: Fractional
13+
featureGroupName: unit-testing-feature-group
14+
offlineStoreConfig:
15+
dataCatalogConfig:
16+
catalog: AwsDataCatalog
17+
database: sagemaker_featurestore
18+
tableName: unit-testing-feature-group-1627433146
19+
disableGlueTableCreation: false
20+
s3StorageConfig:
21+
resolvedOutputS3URI: s3://source-data-bucket-592697580195-us-west-2/sagemaker/feature-group-data/123456789012/sagemaker/us-west-2/offline-store/unit-testing-feature-group-1627433146/data
22+
s3URI: s3://source-data-bucket-592697580195-us-west-2/sagemaker/feature-group-data
23+
recordIdentifierFeatureName: TransactionID
24+
roleARN: arn:aws:iam::123456789012:role/ack-sagemaker-execution-role
25+
status:
26+
ackResourceMetadata:
27+
arn: arn:aws:sagemaker:us-west-2:123456789012:feature-group/unit-testing-feature-group
28+
ownerAccountID: ""
29+
conditions:
30+
- lastTransitionTime: "0001-01-01T00:00:00Z"
31+
message: FeatureGroup is in Created status.
32+
status: "True"
33+
type: ACK.ResourceSynced
34+
featureGroupStatus: Created

0 commit comments

Comments
 (0)