Skip to content

Commit

Permalink
Fix linting
Browse files Browse the repository at this point in the history
  • Loading branch information
@atburke
atburke committed Feb 9, 2022
1 parent e2df595 commit 8281dfc
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 11 deletions.
11 changes: 7 additions & 4 deletions api/client/webclient/webclient_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"net"
"net/http"
"net/http/httptest"
"os"
"testing"

"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -292,10 +291,14 @@ func TestExtract(t *testing.T) {
}

func TestNewWebClientRespectHTTPProxy(t *testing.T) {
os.Setenv("HTTPS_PROXY", "localhost:9999")
defer os.Unsetenv("HTTPS_PROXY")
t.Setenv("HTTPS_PROXY", "localhost:9999")
client := newWebClient(false /* insecure */, nil /* pool */)
_, err := client.Get("https://example.com")
resp, err := client.Get("https://example.com")
defer func() {

This comment has been minimized.

# to view

Copy link
@zmb3

zmb3 Feb 9, 2022

Collaborator

Hmm, this may be a case of the linter not understanding what we're doing.

The general pattern that the linter wants to check for is:

resp, err := client.Get(...)
if err != nil {
   // do something
}
defer resp.Body.Close()

You typically defer the close only if you didn't get an error.

In this case, you're getting an error, so it shouldn't be necessary.

Maybe the linter doesn't understand that require.Error(t, err) is equivalent to an error check.
if resp != nil {
resp.Body.Close()
}
}()
// Client should try to proxy through nonexistent server at localhost.
require.Error(t, err)
require.Contains(t, err.Error(), "proxyconnect")
Expand Down
21 changes: 14 additions & 7 deletions lib/client/https_client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,28 +17,35 @@ limitations under the License.
package client

import (
"os"
"testing"

"github.com/stretchr/testify/require"
)

func TestNewInsecureWebClientHTTPProxy(t *testing.T) {
os.Setenv("HTTPS_PROXY", "localhost:9999")
defer os.Unsetenv("HTTPS_PROXY")
t.Setenv("HTTPS_PROXY", "localhost:9999")
client := NewInsecureWebClient()
_, err := client.Get("https://example.com")
resp, err := client.Get("https://example.com")
defer func() {
if resp != nil {
resp.Body.Close()
}
}()
// Client should try to proxy through nonexistent server at localhost.
require.Error(t, err)
require.Contains(t, err.Error(), "proxyconnect")
require.Contains(t, err.Error(), "connection refused")
}

func TestNewClientWithPoolHTTPProxy(t *testing.T) {
os.Setenv("HTTPS_PROXY", "localhost:9999")
defer os.Unsetenv("HTTPS_PROXY")
t.Setenv("HTTPS_PROXY", "localhost:9999")
client := newClientWithPool(nil)
_, err := client.Get("https://example.com")
resp, err := client.Get("https://example.com")
defer func() {
if resp != nil {
resp.Body.Close()
}
}()
// Client should try to proxy through nonexistent server at localhost.
require.Error(t, err)
require.Contains(t, err.Error(), "proxyconnect")
Expand Down

0 comments on commit 8281dfc

Please # to comment.