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

Add convenience constructor to HTTPServerSettings. #1810

Merged
merged 1 commit into from
Jul 3, 2017

Conversation

s-ludwig
Copy link
Member

@s-ludwig s-ludwig commented Jul 2, 2017

Allows to specify the bind interface and port as a single constructor argument

Allows to specify the bind interface and port as a single constructor argument
Copy link
Member

@wilzbach wilzbach left a comment

Choose a reason for hiding this comment

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

I like this - LGTM!

A future extension might accept commas to allow multiple addresses, though I highly doubt that this is a common use case and would need to be part of the convenience constructor.

@s-ludwig
Copy link
Member Author

s-ludwig commented Jul 3, 2017

Something still appears to be wrong with CodeCov - it doesn't pick up the unit test that is added by this patch at all: https://codecov.io/gh/rejectedsoftware/vibe.d/compare/3048f34149ecbd1fdd0362cebe0432e65105e008...4517e3769887f8912df66a8527479e43188aa589/src/http/vibe/http/server.d#L642

@s-ludwig s-ludwig merged commit f0d3ff1 into master Jul 3, 2017
@wilzbach
Copy link
Member

wilzbach commented Jul 3, 2017

Something still appears to be wrong with CodeCov - it doesn't pick up the unit test that is added by this patch at all

It's most likely a problem on our side - one can download the uploaded report if one clicks on a respective commit and then on the "Build" tab:

https://codecov.io/gh/rejectedsoftware/vibe.d/commit/4517e3769887f8912df66a8527479e43188aa589/build

I guess a first problem is that we don't merge the coverage files as they seem to be in different folder:

> grep "server.lst" 8846e4fb-15c9-4040-9693-f2b13f93840d.txt
# path=tests/args/..-..-http-vibe-http-server.lst
# path=tests/dirwatcher/..-..-http-vibe-http-fileserver.lst
# path=tests/dirwatcher/..-..-http-vibe-http-server.lst
# path=tests/mongodb/..-..-http-vibe-http-server.lst
# path=tests/redis/..-..-http-vibe-http-server.lst
# path=tests/rest/..-..-http-vibe-http-server.lst
# path=tests/restclient/..-..-http-vibe-http-fileserver.lst
# path=tests/restclient/..-..-http-vibe-http-server.lst
# path=tests/restcollections/..-..-http-vibe-http-fileserver.lst
# path=tests/restcollections/..-..-http-vibe-http-server.lst
# path=tests/tcpproxy/..-..-http-vibe-http-server.lst
# path=tests/tls/..-..-http-vibe-http-server.lst
# path=tests/vibe.http.client.1389/..-..-http-vibe-http-server.lst
# path=tests/vibe.http.client.1426/..-..-http-vibe-http-server.lst
# path=tests/vibe.http.server.1388/..-..-http-vibe-http-server.lst
# path=tests/vibe.http.server.1721/..-..-http-vibe-http-server.lst
# path=tests/vibe.http.server.host-header/..-..-http-vibe-http-server.lst
# path=tests/vibe.http.websocket/..-..-http-vibe-http-server.lst
# path=tests/vibe.web.rest.1125/..-..-http-vibe-http-fileserver.lst
# path=tests/vibe.web.rest.1125/..-..-http-vibe-http-server.lst
# path=tests/vibe.web.rest.1140/..-..-http-vibe-http-fileserver.lst
# path=tests/vibe.web.rest.1140/..-..-http-vibe-http-server.lst
# path=tests/vibe.web.rest.1230/..-..-http-vibe-http-fileserver.lst
# path=tests/vibe.web.rest.1230/..-..-http-vibe-http-server.lst
# path=tests/vibe.web.rest.auth/..-..-http-vibe-http-server.lst
# path=tests/vibe.web.web.auth/..-..-http-vibe-http-server.lst
# path=tests/vibe.web.web/..-..-http-vibe-http-server.lst
# path=http-vibe-http-server.lst
# path=http-vibe-http-fileserver.lst

This can be done with dmd_coverDestPath.

However, I don't know why in all of 22 server files, there's none which coverage:

>  grep -A10 -i "this(string bind_string)" 8846e4fb-15c9-4040-9693-f2b13f93840d.txt | less
0000000|        this(string bind_string)
       |        @safe {
0000000|                this();
       |
0000000|                if (bind_string.startsWith('[')) {
0000000|                        auto idx = bind_string.indexOf(']');
0000000|                        enforce(idx > 0, "Missing closing bracket for IPv6 address.");
0000000|                        bindAddresses = [bind_string[1 .. idx]];
0000000|                        bind_string = bind_string[idx+1 .. $];
       |
0000000|                        enforce(bind_string.length == 0 || bind_string.startsWith(':'),

Not sure why the all server.lst are effectively empty - dub test :http -b unittest-cov --build-mode=singleFile --force works (and yields a covered lst file) on my machine.

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.

2 participants