Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify tests with runSkycfgTests test runner #98

Merged
merged 2 commits into from
Sep 8, 2021

Conversation

seena-stripe
Copy link
Collaborator

Summary

Adds a test runner runSkycfgTests to remove redundant testing logic. Many of the tests had a 1 line skycfg expression and a 1 line desired output, but duplicated all the test boilerplate, so this PR adds a test runner to simplify setting up a new test

This is purely cleanup and shouldn't affect evaluation.

Example

 func TestUnsetProto2Fields(t *testing.T) {
        // Proto v2 distinguishes between unset and set-to-empty.
-       msg, err := eval(`proto.package("skycfg.test_proto").MessageV2(
-                f_string = None,
-        )`, nil)
-       if err != nil {
-               t.Fatal(err)
-       }
-       got := mustProtoMessage(t, msg)
-       want := &pb.MessageV2{
-               FString: nil,
-       }
-       checkProtoEqual(t, want, got)
+       runSkycfgTests(t, []skycfgTest{
+               {
+                       src: `proto.package("skycfg.test_proto").MessageV2(
+                               f_string = None,
+                       )`,
+                       want: &pb.MessageV2{
+                               FString: nil,
+                       },
+               },
+       })
 }

This doesn't save on LoC very much but reduces the boilerplate so the test code is a lot more signalful and test output is consistent

Copy link
Collaborator

@kathleen-stripe kathleen-stripe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the want != nil && got == nil change is blocking. Thanks for doing this refactor!

go/protomodule/protomodule_message_test.go Outdated Show resolved Hide resolved
go/protomodule/protomodule_test.go Outdated Show resolved Hide resolved
go/protomodule/protomodule_test.go Show resolved Hide resolved
@seena-stripe seena-stripe merged commit c020cc1 into trunk Sep 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants