IBM / keyprotect-go-client

Go SDK for interacting with the IBM Cloud KeyProtect service.
Apache License 2.0
6 stars 29 forks source link

Failing unit-test #43

Closed vatine closed 3 years ago

vatine commented 3 years ago

Hi, as part of an occasional "look at how the Go eco-system is doing" project I occasionally poke at, I found a failing unit test. This looks like it shoudl be relatively easy to fix, and I would be happy to prepare a PR that changes the string looked for in the unit test, but I simply don't know the code base well enough to say if "fix the expected string" is the right fix, or if this is actually highlighting a problem. FWIW, it was also present in 0.5.0, but I can't speak for versions before that.

$ go test github.com/IBM/keyprotect-go-client       
go: downloading github.com/stretchr/testify v1.4.0
go: downloading gopkg.in/h2non/gock.v1 v1.0.15
go: downloading github.com/pmezard/go-difflib v1.0.0
go: downloading github.com/davecgh/go-spew v1.1.1
go: downloading gopkg.in/yaml.v2 v2.2.2
go: downloading github.com/h2non/parth v0.0.0-20190131123155-b4df798d6542
--- FAIL: TestKeys (0.02s)
    --- FAIL: TestKeys/New_API_with_Logger (0.00s)
        kp_test.go:179: 
                Error Trace:    kp_test.go:179
                                            kp_test.go:991
                Error:          Error message not equal:
                                expected: "parse :/api/v2/: missing protocol scheme"
                                actual  : "parse \":/api/v2/\": missing protocol scheme"
                Test:           TestKeys/New_API_with_Logger
FAIL
FAIL    github.com/IBM/keyprotect-go-client 0.033s
FAIL
jgiannuzzi commented 3 years ago

I had a closer look at this, and it happens with Go >= 1.14.

It stems from a formatting change in net.url.(*Error).Error(). I think a proper fix would be to not compare the error, but instead the op and the unwrapped error, i.e.

assert.IsType(t, &url.Error{}, err)
assert.Equal(t, err.(*url.Error).Op, "parse")
assert.EqualError(t, err.(*url.Error).Unwrap(), "missing protocol scheme")
mrodden commented 3 years ago

Thanks, it appears they switched the url.Error.Error() output and the unit test was fragile

Should be fixed with PR #47