Skip to content

Conversation

@dbu
Copy link
Contributor

@dbu dbu commented Nov 21, 2014

No description provided.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we will ask to create a BAN request, so this should really return a BAN request.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird, why did this test succeed before? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because of the oddity that is fixed in #135 - we are only mocking here so we did not detect that we are doing rubbish...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops! 😲

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove http everywhere? I think we support constructing Varnish both with and without http.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think its mostly noise having the protocol. i left a test where we specifically test scheme handling. but i can re-add if you want.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, if there’s at least one with http, I agree.

ddeboer added a commit that referenced this pull request Nov 21, 2014
…k-request

fix mock request for exceptions test
@ddeboer ddeboer merged commit 870c000 into master Nov 21, 2014
@ddeboer ddeboer deleted the fix-exceptions-test-mock-request branch November 21, 2014 10:49
@ddeboer ddeboer mentioned this pull request Nov 21, 2014
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 this pull request may close these issues.

4 participants