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

Travis CI #16

Open
gaul opened this issue Mar 14, 2017 · 4 comments
Open

Travis CI #16

gaul opened this issue Mar 14, 2017 · 4 comments

Comments

@gaul
Copy link
Contributor

gaul commented Mar 14, 2017

Please enable Travis CI to prevent regressions like the current one in 2af00d7:

--- FAIL: TestDeleteCookies (0.00s)
        Error Trace:    handlers_test.go:404
        Error:          "[k3=v3]" does not contain "k1="

FAIL
exit status 1
FAIL    github.com/ahmetalpbalkan/go-httpbin    2.607s
@ahmetb
Copy link
Owner

ahmetb commented Mar 14, 2017

It passed the tests as you can see in https://travis-ci.org/ahmetb/go-httpbin/builds/210435220

@gaul
Copy link
Contributor Author

gaul commented Mar 14, 2017

This test succeeds with go 1.7.5 and fails with 1.8. The test is actually wrong; it sets three cookies, deletes two, then checks to see if all three remain.

@ahmetb
Copy link
Owner

ahmetb commented Mar 14, 2017

We don't just check if all 3 exists. We check that 2 are unset (to empty value), 1 is modified but all 3 are present. Since it worked this way with go1.7 just fine, I've reason to believe that either the CookieJar implementation or the http.Client's way of dealing with cookies has changed.

@dbenque
Copy link

dbenque commented Nov 29, 2017

For a cookie defined like this:
http.Cookie{ Name: "k1", Value: "", Path: "/", Expires: time.Unix(0, 0), MaxAge: 0, }

Go 1.7 produce the following string:
k1=; Path=/

While Go 1.9 produce the following string:
k1=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT

This is due to the following fix golang/go@d86a6ef

So now in 1.9 you have a real deletion of the cookie thanks to the expiration time that is correctly set.

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

No branches or pull requests

3 participants