-
Notifications
You must be signed in to change notification settings - Fork 156
Add Functional tests for ProxySettingsPolicy #4482
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
base: feat/proxy-settings-policy
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| package framework | ||
|
|
||
| type Option func(*Options) | ||
|
|
||
| type Options struct { | ||
| logEnabled bool | ||
| withContext bool | ||
| } | ||
|
|
||
| func WithLoggingDisabled() Option { | ||
| return func(opts *Options) { | ||
| opts.logEnabled = false | ||
| } | ||
| } | ||
|
|
||
| func WithContextDisabled() Option { | ||
| return func(opts *Options) { | ||
| opts.withContext = false | ||
| } | ||
| } | ||
|
|
||
| func TestOptions(opts ...Option) *Options { | ||
| options := &Options{logEnabled: true, withContext: true} | ||
| for _, opt := range opts { | ||
| opt(options) | ||
| } | ||
|
|
||
| return options | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,7 +24,7 @@ func Get( | |
| headers, queryParams map[string]string, | ||
| opts ...Option, | ||
| ) (int, string, error) { | ||
| options := LogOptions(opts...) | ||
| options := TestOptions(opts...) | ||
|
|
||
| resp, err := makeRequest(http.MethodGet, url, address, nil, timeout, headers, queryParams, opts...) | ||
| if err != nil { | ||
|
|
@@ -46,12 +46,27 @@ func Get( | |
| return resp.StatusCode, "", err | ||
| } | ||
| if options.logEnabled { | ||
| GinkgoWriter.Printf("Successfully received response and parsed body: %s\n", body.String()) | ||
| printResponseBody(body) | ||
| } | ||
|
|
||
| return resp.StatusCode, body.String(), nil | ||
| } | ||
|
|
||
| func printResponseBody(body *bytes.Buffer) { | ||
| maxLogBodyBytes := 2048 | ||
| bs := body.Bytes() | ||
| if len(bs) <= maxLogBodyBytes { | ||
| GinkgoWriter.Printf("Successfully received response and parsed body (%d bytes): %s\n", len(bs), string(bs)) | ||
| } else { | ||
| GinkgoWriter.Printf( | ||
| "Successfully received response and parsed body (%d bytes, showing first %d): %s...\n", | ||
| len(bs), | ||
| maxLogBodyBytes, | ||
| string(bs[:maxLogBodyBytes]), | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| // Post sends a POST request to the specified url with the body as the payload. | ||
| // It resolves to the specified address instead of using DNS. | ||
| func Post( | ||
|
|
@@ -93,10 +108,7 @@ func makeRequest( | |
| return dialer.DialContext(ctx, network, fmt.Sprintf("%s:%s", address, port)) | ||
| } | ||
|
|
||
| ctx, cancel := context.WithTimeout(context.Background(), timeout) | ||
| defer cancel() | ||
|
|
||
| options := LogOptions(opts...) | ||
| options := TestOptions(opts...) | ||
| if options.logEnabled { | ||
| requestDetails := fmt.Sprintf( | ||
| "Method: %s, URL: %s, Address: %s, Headers: %v, QueryParams: %v\n", | ||
|
|
@@ -106,10 +118,24 @@ func makeRequest( | |
| headers, | ||
| queryParams, | ||
| ) | ||
| GinkgoWriter.Printf("Sending request: %s", requestDetails) | ||
| GinkgoWriter.Printf("\nSending request: %s", requestDetails) | ||
| } | ||
|
|
||
| req, err := http.NewRequestWithContext(ctx, method, url, body) | ||
| var ( | ||
| req *http.Request | ||
| err error | ||
| ) | ||
| if options.withContext { | ||
| ctx, cancel := context.WithTimeout(context.Background(), timeout) | ||
| defer cancel() | ||
|
|
||
| req, err = http.NewRequestWithContext(ctx, method, url, body) | ||
| } else { | ||
| // Create request without a short-lived context; rely on http.Client.Timeout | ||
| // so that the response body can be fully read by the caller without | ||
| // being canceled when this function returns. | ||
| req, err = http.NewRequest(method, url, body) //nolint:noctx | ||
|
Comment on lines
+134
to
+137
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not just provide a context with a smaller timeout? That way we don't need this conditional.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that is what i did at the beginning: i added parameter and passed timeout from test, but the thing is that it had no sense somehow: once request was sent - context was cancelled regardless size of timeout.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why smaller timeout?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh I misread. Could just do a larger timeout.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, i tried larger :D so,
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, context is cancelled because of the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. exactly. But i still want to try to use timeout, but in this case i guess i'll need to cancel it in the parent func. |
||
| } | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to cut body in logs: