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

protocol must be quoted in error strings #2

Closed
drew-parsons opened this issue Apr 12, 2020 · 3 comments · Fixed by #3
Closed

protocol must be quoted in error strings #2

drew-parsons opened this issue Apr 12, 2020 · 3 comments · Fixed by #3

Comments

@drew-parsons
Copy link

Running tests for go-httpclient triggers errors:

    /build/1st/golang-github-koofr-go-httpclient-0.0~git20190818.e0dc8fd/obj-x86_64-linux-gnu/src/github.com/koofr/go-httpclient/httpclient_test.go:42
      New
      /build/1st/golang-github-koofr-go-httpclient-0.0~git20190818.e0dc8fd/obj-x86_64-linux-gnu/src/github.com/koofr/go-httpclient/httpclient_test.go:66
        should get error if FullURL is invalid [It]
        /build/1st/golang-github-koofr-go-httpclient-0.0~git20190818.e0dc8fd/obj-x86_64-linux-gnu/src/github.com/koofr/go-httpclient/httpclient_test.go:103
    
        Expected
            <string>: parse \"://???\": missing protocol scheme
        to equal
            <string>: parse ://???: missing protocol scheme

Apparently the protocol strings need to be quoted. This patch fixes the error:

--- a/httpclient_test.go
+++ b/httpclient_test.go
@@ -106,7 +106,7 @@
                                FullURL: "://???",
                        })
                        Expect(err).To(HaveOccurred())
-                       Expect(err.Error()).To(Equal(`parse ://???: missing protocol scheme`))
+                       Expect(err.Error()).To(Equal(`parse "://???": missing protocol scheme`))
                })
        })

@@ -905,7 +905,7 @@

                        _, err = client.Request(req)
                        Expect(err).To(HaveOccurred())
-                       Expect(err.Error()).To(Equal("Post http:/: broken body"))
+                       Expect(err.Error()).To(Equal("Post \"http:/\": broken body"))

                        <-waitCh
                })

This is evidently a change in behaviour in Go 1.14. The tests passed fine in Go 1.13.

@bancek
Copy link
Member

bancek commented Apr 12, 2020

I think it would be better to just check that the error Contains("missing protocol scheme") and Contains("broken body").

Can you please open a pull request with a fix that will work for both Go 1.14 and Go 1.13?

@drew-parsons
Copy link
Author

I'm not sufficiently fluent in Go to cover both 1.13 and 1.14 efficiently. Using Contains sounds like a good idea.

@bancek
Copy link
Member

bancek commented Apr 20, 2020

Thank you for your report.

@bancek bancek closed this as completed Apr 20, 2020
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 a pull request may close this issue.

2 participants