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

Option to disable BEP19 web seeds #911

Merged
merged 4 commits into from Sep 17, 2016
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

Test option to disable web seeds

  • Loading branch information
dcposch committed Sep 15, 2016
commit 79484a5280a37221a6a31c3b1ea000dd52e77b40
@@ -9,8 +9,12 @@ var serveStatic = require('serve-static')
var test = require('tape')
var WebTorrent = require('../../')

// it should be fast to download a small torrent over local HTTP
var WEB_SEED_TIMEOUT_MS = 500

test('Download using webseed (via .torrent file)', function (t) {
t.plan(6)
t.timeoutAfter(WEB_SEED_TIMEOUT_MS)

This comment has been minimized.

Copy link
@feross

feross Sep 17, 2016

Member

Won't this just add flakiness to the test suite? We don't do this in any of the other tests. What if it takes 600ms to complete on Travis CI servers?

This comment has been minimized.

Copy link
@dcposch

dcposch Sep 18, 2016

Author Contributor

It completes very fast (~10ms) on my Macbook.

I added the timeout to match the timeout below. How do you verify that webSeeds: false is working? I thought: make sure that the webSeed download always completes in under X time, then set webSeeds: false and wait that same amount of time and verify there are no requests


var parsedTorrent = extend(fixtures.leaves.parsedTorrent)

@@ -72,3 +76,46 @@ test('Download using webseed (via .torrent file)', function (t) {
})
})
})

test('Disable webseeds', function (t) {
var parsedTorrent = extend(fixtures.leaves.parsedTorrent)

var httpServer = http.createServer(function (req, res) {
t.fail('webseed http server should not get any requests')
})
var client

httpServer.on('error', function (err) { t.fail(err) })

series([
function (cb) {
httpServer.listen(cb)
},

function (cb) {
parsedTorrent.urlList = [
'http://localhost:' + httpServer.address().port + '/' + fixtures.leaves.parsedTorrent.name
]

client = new WebTorrent({ dht: false, tracker: false, webSeeds: false })

client.on('error', function (err) { t.fail(err) })
client.on('warning', function (err) { t.fail(err) })

client.add(parsedTorrent, {store: MemoryChunkStore})

// The test above ensures that we can download the whole torrent over webseeds within a
// short time. Here, we wait the same amount of time and make sure no HTTP requests happen.

This comment has been minimized.

Copy link
@feross

feross Sep 17, 2016

Member

I'm not sure what this comment means. Since the webSeeds option was set to false, no download progress will be made, right? This will just timeout after 500ms and move on to the cleanup code below.

That's fine, I think waiting 500ms is a reasonable way to ensure that no http requests get sent to the server, but the comment should be changed to be clearer.

This comment has been minimized.

Copy link
@feross

feross Sep 17, 2016

Member

Also, another test we can add here is:

var torrent = client.add(parsedTorrent, {store: MemoryChunkStore})

torrent.on('ready', function () {
  t.equal(torrent.urlList.length, 0) // ensure no web seeds were added
})

This comment has been minimized.

Copy link
@dcposch

dcposch Sep 18, 2016

Author Contributor

"The test above" refers to the test that has webSeeds enabled. There are two tests in the file, the first ensures that web seeds work, the second ensures that webSeeds: false works

This comment has been minimized.

Copy link
@dcposch

dcposch Sep 18, 2016

Author Contributor

I don't think that extra test works. It doesn't prevent urlList from being populated (just as setting tracker: false won't make the announce list empty, right?). It just means that none of those URLs will be contacted.

setTimeout(cb, WEB_SEED_TIMEOUT_MS)
}
], function (err) {
t.error(err)
client.destroy(function (err) {
t.error(err, 'client destroyed')
})
httpServer.close(function () {
t.pass('http server closed')
t.end()
})
})
})
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.