Make hostname:port configurable for test server. #207
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Namek
force-pushed
the
tests-configurable_hostname_port
branch
from
5a5e6fb
to
362b768
Compare
Sija
reviewed
Feb 20, 2020
src/test_runner.cr
Outdated
| @@ -110,6 +111,7 @@ module Mint | |||
| </body> | |||
| </html> | |||
| HTML | |||
src/test_runner.cr
Outdated
| @@ -8,7 +8,8 @@ module Mint | |||
| }) | |||
| end | |||
|
|
|||
| PAGE = <<-HTML | |||
| def get_page | |||
There was a problem hiding this comment.
Suggested change
| def get_page | |
| def page_source : String |
src/test_runner.cr
Outdated
| @@ -282,7 +285,7 @@ module Mint | |||
| get "/" do | |||
| @failed = [] of Message | |||
| @succeeded = 0 | |||
| PAGE | |||
| get_page | |||
There was a problem hiding this comment.
Suggested change
| get_page | |
| page_source |
Namek
force-pushed
the
tests-configurable_hostname_port
branch
3 times, most recently
from
b22ef40
to
8823052
Compare
Sija
reviewed
Feb 20, 2020
src/test_runner.cr
Outdated
| @@ -8,7 +8,8 @@ module Mint | |||
| }) | |||
| end | |||
|
|
|||
| PAGE = <<-HTML | |||
| def page_source : String | |||
| <<-HTML | |||
There was a problem hiding this comment.
It makes sense to memoize it since it's not a constant anymoar.
Suggested change
| <<-HTML | |
| @page_source ||= <<-HTML |
|
Please don't merge it until my signal. I want to make it work on Windows first. |
Namek
force-pushed
the
tests-configurable_hostname_port
branch
3 times, most recently
from
b813ca8
to
6f63a0f
Compare
|
@Sija @gdotdesign It works, I have also updated the PR description. Please re-review and merge if it's OK. |
Sija
reviewed
Feb 21, 2020
Namek
force-pushed
the
tests-configurable_hostname_port
branch
from
6f63a0f
to
e4c7b6f
Compare
Sija
reviewed
Feb 22, 2020
Namek
force-pushed
the
tests-configurable_hostname_port
branch
from
e4c7b6f
to
12b71f8
Compare
gdotdesign
approved these changes
Feb 23, 2020
|
Thanks 👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Solves #205
What?
There are 4 new options for
mint test:Running test using Docker:
mint test --manual --host 0.0.0.0This launches test server on host
0.0.0.0which is available outside of the Docker container but also sets--browser-hostto127.0.0.1by default which is a connection address for websocket outside the container. Same withport/browser_portpair.Full instruction will be at mint-lang/mint-docker#1