fix(cli): preprocess test and transaction files when running and applying #2879
Conversation
schoren
changed the title
| @@ -245,28 +245,8 @@ func (a runTestAction) applyTest(ctx context.Context, df defFile) (defFile, erro | |||
| return df, fmt.Errorf("cannot inject local env vars: %w", err) | |||
| } | |||
|
|
|||
| var test openapi.TestResource | |||
There was a problem hiding this comment.
this preprocessing is moved to preprocessor/test.go
| func (a runTestAction) applyTransaction(ctx context.Context, df defFile) (defFile, error) { | ||
| df, err := a.injectLocalEnvVars(ctx, df) | ||
| if err != nil { | ||
| return df, fmt.Errorf("cannot inject local env vars: %w", err) | ||
| } | ||
|
|
||
| var tran openapi.TransactionResource |
There was a problem hiding this comment.
this preprocessing is moved to preprocessor/transaction.go
| @@ -16,8 +16,8 @@ import ( | |||
| ) | |||
|
|
|||
| var ( | |||
| cliLogger = &zap.Logger{} | |||
There was a problem hiding this comment.
having a zeroed reference to a logger, in combination with replacing the pointer later in this file, allows to inject an unconfigured logger if needed on init, but have it configured by runtime.
Worst case scenario, clients get a zeroed, but valid, logger, instead of a nil pointer
| httpClient = &resourcemanager.HTTPClient{} | ||
|
|
||
| testPreprocessor = preprocessor.Test(cliLogger) | ||
| testClient = resourcemanager.NewClient( |
There was a problem hiding this comment.
this client is needed by the transaction preprocessor, so its here now to avoid a ciruclar initialization loop
| @@ -15,8 +15,7 @@ type Client struct { | |||
| client *HTTPClient | |||
| resourceName string | |||
| resourceNamePlural string | |||
| deleteSuccessMsg string | |||
| tableConfig TableConfig | |||
| options options | |||
There was a problem hiding this comment.
3 options seemed enough to move them into its own struct, so it can grow in the future and still havig the client struct readable
| @@ -56,39 +55,24 @@ func (c HTTPClient) do(req *http.Request) (*http.Response, error) { | |||
| return c.client.Do(req) | |||
| } | |||
|
|
|||
| type options func(c *Client) | |||
There was a problem hiding this comment.
moved to its own file
| } | ||
|
|
||
| func (t test) Preprocess(ctx context.Context, input fileutil.File) (fileutil.File, error) { | ||
| var test openapi.TestResource |
There was a problem hiding this comment.
this is the same code moved from the test_run_action.go file, with a few variables renamed
| } | ||
|
|
||
| func (t transaction) Preprocess(ctx context.Context, input fileutil.File) (fileutil.File, error) { | ||
| var tran openapi.TransactionResource |
There was a problem hiding this comment.
this is the same code moved from the test_run_action.go file, with a few variables renamed
| assert.Equal("application/json", listTest.Spec.Trigger.HTTPRequest.Headers[0].Value) | ||
| }) | ||
|
|
||
| t.Run("EmbeddingProtobufFile", func(t *testing.T) { |
There was a problem hiding this comment.
New test case to ensure proper protobuf file contents embed happens
The new cli resources client introduced a bug:
When a test or transaction is run with
tracetest test run ...command, the files gets preprocessed: tests files embed protobuf files if needed, and transactions map test file references to test IDs.The original behaviour was to preprocess files both on
test runandapply transaction(apply testwasn't implemented beofre).This PR adds e2e tests to cover this behavior and fixes it.
In order to implement this in a generic way, we introdiuce a
preprocessorconcept to the resource manager client.Resources can define an optional
applyPreProcessorFnfunction when registering. This func can alter the file that gets submitted to the server before submitting it.