Add requests library to the Projects Tester

[wilzbach]

CC @ikod

[dlang-bot]

Thanks for your pull request, @wilzbach!

[ikod]

Performing "unittest" build using /var/lib/jenkins/dlang_projects@3/distribution/bin/dmd for x86_64.
idna 0.0.1: building configuration "std"...
vibe-d:utils 0.8.2: building configuration "library"...
../../.dub/packages/vibe-d-0.8.2/vibe-d/utils/vibe/internal/memory_legacy.d(187,37): Error: constructor vibe.utils.hashmap.HashMap!(void*, ulong, DefaultHashMapTraits!(void*)).HashMap.this(IAllocator allocator) is not callable using argument types (RCIAllocator)
../../.dub/packages/vibe-d-0.8.2/vibe-d/utils/vibe/internal/memory_legacy.d(187,37): cannot pass argument (*function () => allocatorObject(instance))() of type RCIAllocator to parameter IAllocator allocator
../../.dub/packages/vibe-d-0.8.2/vibe-d/utils/vibe/internal/utilallocator.d(14,85): Error: cannot implicitly convert expression (*function () => allocatorObject(instance))() of type RCIAllocator to std.experimental.allocator.IAllocator
/var/lib/jenkins/dlang_projects@3/distribution/bin/dmd failed with exit code 1.

Is it problem with dlang-requests or with vibed-utils? I can't reproduce this problem on my ubuntu.

[wilzbach]

This is a problem with dmd-nightly. You need vibe.d 0.8.3-alpha.1 for a fixed vibe.d.
FYI: only the latest stable git tag is tested on the project tester, so this dub bump would need a new tag. Would that be possible? Thanks!

[ikod]

Tag v0.7.1 added. Is it what you expected?

[wilzbach]

Yes. Thanks! Force-pushed to restart the CI.

[wilzbach]

Yes. Thanks!

Oh sorry. I replied to eagerly.

https://github.com/ikod/dlang-requests/blob/v0.7.1/dub.json

DUB won't download alpha releases, but only the latest stable release. This means .travis.yml must use vibe-d.0.8.3-alpha (by default it will use 0.8.2, hence the failures).
There are multiple ways to do this. I think dub.selections.json is the easiest, but the Project Tester will use execute whatever you define in .travis.yml - so whatever works for you (and dmd-nightly) should be fine.

BTW in case you are interested why this is necessary (i.e. why vibe-d is broken with nightly) -> dlang/phobos#5921

[ikod]

I'd prefer .travis.yml, but sorry, I don't know how to point out vibe.d version there (and can't find docs or examples). Can you give me some instructions?

[wilzbach]

One hack - if you don't want to use the dub.selections.json would be something like this:

dub upgrade
sed 's/"vibe-d": "[^"]*"/"vibe-d": "0.8.3-alpha.1"/' -i dub.selections.json
sed 's/"vibe-d": "[^"]*"/"vibe-core": "1.4.0-alpha.1"/' -i dub.selections.json

Note that your users might run into the same problems with the next dmd release :O
Anyhow, looking at your test suite I see that there's a lot of network activity which looks quite susceptible to failures. Do you have any experiences with how prone to failures it is?
Should we maybe just run dub build here?
Note that the commands to be run can be defined directly here too - the travis.yml approach is just a convenience feature.

[ikod]

Ok, I made some changes in requests, as it didn't compile with 0.8.3-alpha, bumped vibe-d version in requests dependencies from 0.7 to ~> 0.8, and also changed .tavis.yml to run custom script

#!/bin/bash

set -ev

echo testing std
dub test --config std

echo testing vibed
cat << EOF > dub.selections.json
{
    "fileVersion": 1,
    "versions": {
        "vibe-d": "0.8.3-alpha.1",
        "vibe-core": "1.4.0-alpha.1"
    }
}
EOF
dub test --config vibed
rm dub.selections.json

Here is result: https://travis-ci.org/ikod/dlang-requests/jobs/338944879

I'll remove all custom travis settings later, after next vibed and dmd release.

Thanks,
Igor

PS. I'd like not to add dub.selection.json to my package (I suspect this can hurt some users).
PPS. if something is wrong here - fell free to ping me or even make PR for requests with required changes.

[wilzbach]

Ok, I made some changes in requests, as it didn't compile with 0.8.3-alpha, bumped vibe-d version in requests dependencies from 0.7 to ~> 0.8, and also changed .tavis.yml to run custom script

Wow. Thanks!

Checking out Revision 6f58f3645898a67697748b4a4807f262cad4139b (v0.7.1)

https://ci.dlang.io/blue/organizations/jenkins/dlang-org%2Fci/detail/PR-166/3/pipeline/204

PS. I'd like not to add dub.selection.json to my package (I suspect this can hurt some users).

How? Could you explain this?
See also: dlang/dub#829

fell free to ping me or even make PR for requests with required changes.

Thanks.

PPS. if something is wrong here -

Well, the only thing I am still wondering about is whether we can make the testsuite "soft-fail" on network errors. The Project Tester receives quite some traffic and from my experience, servers aren't always up.
Just have a look at the result of this run - one vibe.d test failed ... because google.com wasn't reachable - I just removed the test from the testsuite here (#169), because for a PR to Phobos or DMD failing should mean failing, not potentially failing or google has a bad day today.

The environment variable DETERMINISTIC_HINT is set to 1 by the Project Tester, s.t. it's easier for authors to avoid non-deterministic tests.
My ideas:

1. Just run dub build (should find most regressions)
2. catch network exceptions or related errors and "soft fail" on them for DETERMINISTIC_HINT
3. Start a local HTTPBIN-like testing server (probably a lot more work)

What are your thoughts on this?
(1) looks like the easiest option for now and already provides value (prevents regressions).

What do you think?

[ikod]

Checking out Revision 6f58f3645898a67697748b4a4807f262cad4139b (v0.7.1)

https://ci.dlang.io/blue/organizations/jenkins/dlang-org%2Fci/detail/PR-166/3/pipeline/204

Sorry, I forgot to create tag. Run it please one more time.

PS. I'd like not to add dub.selection.json to my package (I suspect this can hurt some users).

How? Could you explain this?

I can be totally wrong, but what happens if someone have application depending on vibe-d ver x.x.x and requests depends (using dub.selections.json) on vibe-d y.y.y? Which version of vibe will be linked with app finaly?

Well, the only thing I am still wondering about is whether we can make the testsuite "soft-fail" on network errors. The Project Tester receives quite some traffic and from my experience, servers aren't always up.

Forgot to answer this question, sorry. So, in std http tests I use internal http server, for ftp tests I still use some external ftp servers and this sometimes lead to problems (not too frequent but anyway).

For vibe test I use external servers both for http and for ftp.

Right solution is, of course, to depend only on internal http and ftp servers, but this require some time, and eventually I will do this.

Just run dub build (should find most regressions)

Yes, of course. I anyway test everything on travis site. Should I change something so that you can test only dub build?

[wilzbach]

Yes, of course. I anyway test everything on travis site. Should I change something so that you can test only dub build?

Yes, that would be best. Well, we have two possibilities.

1. We overwrite .travis.yml here (example: https://github.com/dlang/ci/blob/master/vars/runPipeline.groovy#L270)
2. Or you check DETERMINISTIC_HINT in in your Travis script.

What do you prefer?

BTW I restarted it again:

Checking out Revision 5475661aa008bd8fee8fd491e0e9f07c910915c0 (v0.7.2)

2018-02-08T16:53:48.469:streams.d:connect:1021 Failed to connect to 127.0.0.1:8081(127.0.0.1:8081): Unable to connect socket: Connection refused
2018-02-08T16:53:48.469:streams.d:connect:1021 Failed to connect to 127.0.0.1:8081(127.0.0.1:8081): Unable to connect socket: Connection refused
2018-02-08T16:53:48.469:streams.d:connect:1021 Failed to connect to 127.0.0.1:8081(127.0.0.1:8081): Unable to connect socket: Connection refused
requests.streams.ConnectError@source/requests/streams.d(1026): Can't connect to 127.0.0.1:8081
----------------
source/requests/streams.d:25 requests.streams.SocketStream requests.streams.SocketStream.connect(immutable(char)[], ushort, core.time.Duration) [0x742356]
source/requests/http.d:817 void requests.http.HTTPRequest.setupConnection() [0x67ef97]
source/requests/http.d:1343 requests.http.HTTPResponse requests.http.HTTPRequest.exec!("GET").exec(immutable(char)[], std.typecons.Tuple!(immutable(char)[], "key", immutable(char)[], "value").Tuple[]) [0x686fef]
source/requests/http.d:1501 requests.http.HTTPResponse requests.http.HTTPRequest.get!().get() [0x694101]
source/requests/request.d:291 requests.base.Response requests.request.Request.get!().get(immutable(char)[]) [0x6ab209]
source/requests/package.d:45 void requests.__unittest_L17_C9() [0x695783]
??:? void requests.__modtest() [0x69bc04]

and that's why dub build is probably the best for now ;-)

[ikod]

What do you prefer?

overwrite .travis.yml :)

[wilzbach]

overwrite .travis.yml :)

Done. Let's see ;-)

[wilzbach]

Okay I'm going to merge this now as "requests" is imho a very important library in the D ecosystem and this is just using dub build -c std which means the build time is tiny (9s on Jenkins) and there are now problems with unittests that time out.
It's not ideal and I would prefer if we could run the entire testsuite, but let's move step by step and this is already a lot better than nothing.

@ikod if you ever get around depending only on internal servers, please let us know and simply send a PR to this file here. Thanks!