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

Improve TestWebserverType() #1154

Merged
merged 2 commits into from Oct 8, 2018
Merged

Improve TestWebserverType() #1154

merged 2 commits into from Oct 8, 2018

Conversation

rfay
Copy link
Member

@rfay rfay commented Oct 4, 2018

The Problem/Issue/Bug:

In #1148 (Adding TestWebserverType() to test to make sure we get the type of webserver we're expecting), I was stuck on the fact that the "Server" header was always nginx, so didn't do everything in the test I wanted. Of course the Server header was always nginx, because that's what the router runs. Anyway, I wanted to test for the Server header!

How this PR Solves The Problem:

This PR uses the direct-to-web-container approach and can check for the Server header coming directly from the web container.

It also does minor refactoring to provide easy access to the local-direct URL and port; those were previously done in inline code in app.Describe().

Manual Testing Instructions:

I think if you like the code and it passes that's good enough.

Automated Testing Overview:

This improves TestWebserverType()

Related Issue Link(s):

Release/Deployment notes:

@rfay rfay added this to the v1.3.0 milestone Oct 4, 2018
@rfay rfay self-assigned this Oct 4, 2018
@rfay rfay requested a review from andrewfrench October 4, 2018 19:17
@rfay rfay changed the title Improve TestWebserverType Improve TestWebserverType() Oct 4, 2018
Copy link
Contributor

@andrewfrench andrewfrench left a comment

Choose a reason for hiding this comment

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

Bypassing the router to inspect the server type header make perfect sense. Additions and test changes look good, 👍 from me.

@rfay rfay merged commit d5e7b37 into ddev:master Oct 8, 2018
@rfay rfay deleted the 20181004_server_header branch October 8, 2018 18:47
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.

None yet

2 participants