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

Projects
None yet
2 participants
@s-ludwig
Member

s-ludwig commented Jul 2, 2017

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

Add convenience constructor to HTTPServerSettings.
Allows to specify the bind interface and port as a single constructor argument
@wilzbach

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

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Jul 3, 2017

Member

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

Member

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

3 of 5 checks passed

codecov/patch 6.666% of diff hit (target 37.092%)
Details
codecov/project 37.05% (-0.042%) compared to 3048f34
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@wilzbach

This comment has been minimized.

Show comment
Hide comment
@wilzbach

wilzbach Jul 3, 2017

Contributor

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.

Contributor

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