From 2af660f6aa158f02c38c2608466bf2cc2e0e61d5 Mon Sep 17 00:00:00 2001 From: jasondborneman Date: Fri, 12 Jan 2024 12:33:51 -0500 Subject: [PATCH] Review Feedback 1 --- .../weather/services/tester/design/design.go | 2 ++ .../tester/gen/grpc/tester/server/server.go | 2 ++ .../services/tester/gen/tester/client.go | 4 +++ .../services/tester/gen/tester/service.go | 5 ++++ example/weather/services/tester/run_tests.go | 27 ++++++++++++++----- 5 files changed, 33 insertions(+), 7 deletions(-) diff --git a/example/weather/services/tester/design/design.go b/example/weather/services/tester/design/design.go index 6ad5340c..915ec4db 100644 --- a/example/weather/services/tester/design/design.go +++ b/example/weather/services/tester/design/design.go @@ -18,6 +18,7 @@ var _ = Service("tester", func() { }) Description("The Weather System Tester Service is used to manage the integration testing of the weater system") Error("include_exclude_both", ErrorResult, "Cannot specify both include and exclude") + Error("wildcard_compile_error", ErrorResult, "Wildcard glob did not compile") Method("test_all", func() { Description("Runs all tests in the iam system") @@ -26,6 +27,7 @@ var _ = Service("tester", func() { GRPC(func() { Response(CodeOK) Response("include_exclude_both", CodeInvalidArgument) + Response("wildcard_compile_error", CodeInvalidArgument) }) }) diff --git a/example/weather/services/tester/gen/grpc/tester/server/server.go b/example/weather/services/tester/gen/grpc/tester/server/server.go index 34ef404d..f389aae5 100644 --- a/example/weather/services/tester/gen/grpc/tester/server/server.go +++ b/example/weather/services/tester/gen/grpc/tester/server/server.go @@ -58,6 +58,8 @@ func (s *Server) TestAll(ctx context.Context, message *testerpb.TestAllRequest) switch en.GoaErrorName() { case "include_exclude_both": return nil, goagrpc.NewStatusError(codes.InvalidArgument, err, goagrpc.NewErrorResponse(err)) + case "wildcard_compile_error": + return nil, goagrpc.NewStatusError(codes.InvalidArgument, err, goagrpc.NewErrorResponse(err)) } } return nil, goagrpc.EncodeError(err) diff --git a/example/weather/services/tester/gen/tester/client.go b/example/weather/services/tester/gen/tester/client.go index 34a1e3ac..4a36c5f1 100644 --- a/example/weather/services/tester/gen/tester/client.go +++ b/example/weather/services/tester/gen/tester/client.go @@ -35,6 +35,7 @@ func NewClient(testAll, testSmoke, testForecaster, testLocator goa.Endpoint) *Cl // TestAll calls the "test_all" endpoint of the "tester" service. // TestAll may return the following errors: // - "include_exclude_both" (type *goa.ServiceError): Cannot specify both include and exclude +// - "wildcard_compile_error" (type *goa.ServiceError): Wildcard glob did not compile // - error: internal error func (c *Client) TestAll(ctx context.Context, p *TesterPayload) (res *TestResults, err error) { var ires any @@ -48,6 +49,7 @@ func (c *Client) TestAll(ctx context.Context, p *TesterPayload) (res *TestResult // TestSmoke calls the "test_smoke" endpoint of the "tester" service. // TestSmoke may return the following errors: // - "include_exclude_both" (type *goa.ServiceError): Cannot specify both include and exclude +// - "wildcard_compile_error" (type *goa.ServiceError): Wildcard glob did not compile // - error: internal error func (c *Client) TestSmoke(ctx context.Context) (res *TestResults, err error) { var ires any @@ -61,6 +63,7 @@ func (c *Client) TestSmoke(ctx context.Context) (res *TestResults, err error) { // TestForecaster calls the "test_forecaster" endpoint of the "tester" service. // TestForecaster may return the following errors: // - "include_exclude_both" (type *goa.ServiceError): Cannot specify both include and exclude +// - "wildcard_compile_error" (type *goa.ServiceError): Wildcard glob did not compile // - error: internal error func (c *Client) TestForecaster(ctx context.Context) (res *TestResults, err error) { var ires any @@ -74,6 +77,7 @@ func (c *Client) TestForecaster(ctx context.Context) (res *TestResults, err erro // TestLocator calls the "test_locator" endpoint of the "tester" service. // TestLocator may return the following errors: // - "include_exclude_both" (type *goa.ServiceError): Cannot specify both include and exclude +// - "wildcard_compile_error" (type *goa.ServiceError): Wildcard glob did not compile // - error: internal error func (c *Client) TestLocator(ctx context.Context) (res *TestResults, err error) { var ires any diff --git a/example/weather/services/tester/gen/tester/service.go b/example/weather/services/tester/gen/tester/service.go index 66d57301..e6632bf2 100644 --- a/example/weather/services/tester/gen/tester/service.go +++ b/example/weather/services/tester/gen/tester/service.go @@ -90,3 +90,8 @@ type TesterPayload struct { func MakeIncludeExcludeBoth(err error) *goa.ServiceError { return goa.NewServiceError(err, "include_exclude_both", false, false, false) } + +// MakeWildcardCompileError builds a goa.ServiceError from an error. +func MakeWildcardCompileError(err error) *goa.ServiceError { + return goa.NewServiceError(err, "wildcard_compile_error", false, false, false) +} diff --git a/example/weather/services/tester/run_tests.go b/example/weather/services/tester/run_tests.go index 58f2e2c5..64ed1e55 100644 --- a/example/weather/services/tester/run_tests.go +++ b/example/weather/services/tester/run_tests.go @@ -79,11 +79,16 @@ func recoverFromTestPanic(ctx context.Context, testName string, testCollection * // Filters a testMap based on a test name that is a glob string // using standard wildcards https://tldp.org/LDP/GNU-Linux-Tools-Summary/html/x11655.htm -func matchTestFilter(ctx context.Context, test string, testMap map[string]func(context.Context, *TestCollection)) (bool, []func(context.Context, *TestCollection)) { +func matchTestFilter(ctx context.Context, test string, testMap map[string]func(context.Context, *TestCollection)) ([]func(context.Context, *TestCollection), error) { match := false - testMatches := []func(context.Context, *TestCollection){} + var testMatches []func(context.Context, *TestCollection) var g glob.Glob - g = glob.MustCompile(test) + g, err := glob.Compile(test) + if err != nil { + _ = logError(ctx, err) + err = errors.New(fmt.Sprintf("wildcard glob [%s] did not compile: %v", test, err)) + return testMatches, err + } i := 0 for testName, _ := range testMap { match = g.Match(testName) @@ -92,7 +97,7 @@ func matchTestFilter(ctx context.Context, test string, testMap map[string]func(c } i++ } - return match, testMatches + return testMatches, nil } // Runs the tests from the testmap and handles filtering/exclusion of tests @@ -116,8 +121,11 @@ func (svc *Service) runTests(ctx context.Context, p *gentester.TesterPayload, te if testFunc, ok := testMap[test]; ok { testsToRun[test] = testFunc } else { // Test didn't match exactly, so we're gonna try for a wildcard match - findByWildcard, testFuncs := matchTestFilter(ctx, test, testMap) - if findByWildcard { + testFuncs, err := matchTestFilter(ctx, test, testMap) + if err != nil { + return nil, gentester.MakeWildcardCompileError(err) + } + if len(testFuncs) > 0 { for i, testFunc := range testFuncs { testsToRun[fmt.Sprintf("%s_%d", test, i)] = testFunc } @@ -132,7 +140,12 @@ func (svc *Service) runTests(ctx context.Context, p *gentester.TesterPayload, te wildcardMatch := false for _, excludeTest := range p.Exclude { var g glob.Glob - g = glob.MustCompile(excludeTest) + g, err := glob.Compile(excludeTest) + if err != nil { + _ = logError(ctx, err) + err = errors.New(fmt.Sprintf("wildcard glob [%s] did not compile: %v", excludeTest, err)) + return nil, gentester.MakeWildcardCompileError(err) + } wildcardMatch = wildcardMatch || g.Match(testName) } if !wildcardMatch {