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

Added request headers to http tests #496

Merged
merged 5 commits into from Nov 23, 2019
Merged

Conversation

@dacamp
Copy link
Contributor

dacamp commented Nov 21, 2019

No description provided.

@sean-
sean- approved these changes Nov 21, 2019
Copy link

sean- left a comment

Awesome, this looks really good.

@dacamp

This comment has been minimized.

Copy link
Contributor Author

dacamp commented Nov 21, 2019

Build is failing because Travis is pinned to go1.11 up to @aelsabbahy to weigh in on either updating the go version or maintain compatibility:

diff --git a/system/http.go b/system/http.go
index fc18225..7de0646 100644
--- a/system/http.go
+++ b/system/http.go
@@ -74,7 +74,7 @@ func (u *DefHTTP) setup() error {
        if err != nil {
                return u.err
        }
-       req.Header = u.RequestHeader.Clone()
+       req.Header = u.RequestHeader
        if u.Username != "" || u.Password != "" {
                req.SetBasicAuth(u.Username, u.Password)
        }
@aelsabbahy

This comment has been minimized.

Copy link
Owner

aelsabbahy commented Nov 21, 2019

I'm fine with updating go if it doesn't break anything else.

Feel free to submit a different PR for that or just tack it on this one.

@aelsabbahy

This comment has been minimized.

Copy link
Owner

aelsabbahy commented Nov 22, 2019

https://github.com/aelsabbahy/goss/blob/master/.travis.yml#L4

I think this is what you meant to change.

@dacamp

This comment has been minimized.

Copy link
Contributor Author

dacamp commented Nov 22, 2019

Ahh that's what I was looking for! Want me to backout the changes on go.mod?

@aelsabbahy

This comment has been minimized.

Copy link
Owner

aelsabbahy commented Nov 22, 2019

Can you point me to the docs on the go.mod change. Go has changed dep management systems so many times, I actually haven't read up on go.mod yet.

Guess what I'm saying is, I don't understand the implications of the go.mod change you made. :)

@aelsabbahy

This comment has been minimized.

Copy link
Owner

aelsabbahy commented Nov 22, 2019

If you get this PR green today I'll probably cut a Goss release with it tomorrow.

@dacamp

This comment has been minimized.

Copy link
Contributor Author

dacamp commented Nov 22, 2019

This is the best doc I've found to describe the go directive in go.mod. Looks like it defines the expected language version. Because header.Clone was released in go 1.13 I think it makes sense to set the go version.

@dacamp

This comment has been minimized.

Copy link
Contributor Author

dacamp commented Nov 22, 2019

Looks like the build if failing in Travis. I've noticed these fail locally prior to making any changes. If you think they're just flaky tests would you mind retrying the build in Travis so I don't have to push a no-op change?

@aelsabbahy

This comment has been minimized.

Copy link
Owner

aelsabbahy commented Nov 22, 2019

Count: 94, Failed: 0, Skipped: 5
+[[ centos7 == \a\r\c\h ]]
+egrep -q 'Count: 93, Failed: 0, Skipped: 5'

The counts are mismatching since you added a test.

@aelsabbahy

This comment has been minimized.

Copy link
Owner

aelsabbahy commented Nov 22, 2019

Also, docs need to be updated. I'm doing quick eyeball review on my phone currently.

dacamp added 2 commits Nov 22, 2019
@aelsabbahy

This comment has been minimized.

Copy link
Owner

aelsabbahy commented Nov 22, 2019

Thanks for the document on go.mod, explained it very well.

@aelsabbahy aelsabbahy merged commit aa0fc08 into aelsabbahy:master Nov 23, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.