From 016876413a2a88a8b574ab578a5f0328fd5b274e Mon Sep 17 00:00:00 2001 From: cgewecke Date: Mon, 16 Dec 2019 21:50:52 -0800 Subject: [PATCH 1/6] Add tests for core-subscriptions edge cases --- packages/web3-core-subscriptions/src/index.js | 2 +- .../src/subscription.js | 6 +- test/eth.subscribe.ganache.js | 56 +++++++++++++++++++ 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/packages/web3-core-subscriptions/src/index.js b/packages/web3-core-subscriptions/src/index.js index e12811af8e5..113f6757fd9 100644 --- a/packages/web3-core-subscriptions/src/index.js +++ b/packages/web3-core-subscriptions/src/index.js @@ -59,7 +59,7 @@ Subscriptions.prototype.buildCall = function() { } var subscription = new Subscription({ - subscription: _this.subscriptions[arguments[0]], + subscription: _this.subscriptions[arguments[0]] || {}, // Subscript might not exist requestManager: _this.requestManager, type: _this.type }); diff --git a/packages/web3-core-subscriptions/src/subscription.js b/packages/web3-core-subscriptions/src/subscription.js index 65310126f4a..59b4e25e746 100644 --- a/packages/web3-core-subscriptions/src/subscription.js +++ b/packages/web3-core-subscriptions/src/subscription.js @@ -77,7 +77,11 @@ Subscription.prototype._validateArgs = function (args) { subscription.params = 0; if (args.length !== subscription.params) { - throw errors.InvalidNumberOfParams(args.length, subscription.params + 1, args[0]); + throw errors.InvalidNumberOfParams( + args.length, + subscription.params, + subscription.subscriptionName + ); } }; diff --git a/test/eth.subscribe.ganache.js b/test/eth.subscribe.ganache.js index cde97b6c2db..354bff9429e 100644 --- a/test/eth.subscribe.ganache.js +++ b/test/eth.subscribe.ganache.js @@ -35,6 +35,62 @@ describe('subscription connect/reconnect', function() { }); }); + it('subscribes with a callback', function(){ + let subscription; + + return new Promise(async function (resolve) { + subscription = web3.eth + .subscribe('newBlockHeaders', function(err, result){ + assert(result.parentHash); + subscription.unsubscribe(); // Stop listening.. + resolve(); + }); + }) + }); + + it('resubscribes', function(){ + let subscription; + + return new Promise(async function (resolve) { + subscription = web3.eth.subscribe('newBlockHeaders'); + subscription.unsubscribe(); + subscription.resubscribe() + + subscription.on('data', function(result){ + console.log(result.parentHash) + assert(result.parentHash); + subscription.unsubscribe(); // Stop listening.. + resolve(); + }) + }) + }); + + it('allows a subscription which does not exist', function(){ + web3.eth.subscribe('subscription-does-not-exists'); + }); + + it('errors when zero params subscrip. is called with the wrong arguments', function(){ + try { + web3.eth.subscribe('newBlockHeaders', 5); + assert.fail(); + } catch (err) { + assert(err.message.includes('Invalid number of parameters for "newHeads"')) + assert(err.message.includes('Got 1 expected 0')); + } + }); + + // Could not get the .on('error') version of this to work - maybe a race condition setting it up. + it('errors when the provider is not set', function(){ + web3 = new Web3(); + + return new Promise(async function (resolve) { + web3.eth.subscribe('newBlockHeaders', function(err, result){ + assert(err.message.includes('No provider set')); + resolve() + }) + }); + }); + it('errors when the `eth_subscribe` request got send, the reponse isnt returned from the node, and the connection does get closed in the mean time', async function() { await pify(server.close)(); From 6cf61ed02d0692be6d01c3e657a5f15a8ac8fb05 Mon Sep 17 00:00:00 2001 From: cgewecke Date: Mon, 16 Dec 2019 22:35:27 -0800 Subject: [PATCH 2/6] Remove console.log in test --- test/eth.subscribe.ganache.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/eth.subscribe.ganache.js b/test/eth.subscribe.ganache.js index 354bff9429e..c5923dbf970 100644 --- a/test/eth.subscribe.ganache.js +++ b/test/eth.subscribe.ganache.js @@ -57,7 +57,6 @@ describe('subscription connect/reconnect', function() { subscription.resubscribe() subscription.on('data', function(result){ - console.log(result.parentHash) assert(result.parentHash); subscription.unsubscribe(); // Stop listening.. resolve(); From 4c6bab9939369e283cbd7688f1d00ea40c553a8a Mon Sep 17 00:00:00 2001 From: cgewecke Date: Tue, 17 Dec 2019 01:52:52 -0800 Subject: [PATCH 3/6] Use done --- test/eth.subscribe.ganache.js | 69 +++++++++++++++-------------------- 1 file changed, 29 insertions(+), 40 deletions(-) diff --git a/test/eth.subscribe.ganache.js b/test/eth.subscribe.ganache.js index c5923dbf970..763e75ec6c4 100644 --- a/test/eth.subscribe.ganache.js +++ b/test/eth.subscribe.ganache.js @@ -7,6 +7,7 @@ describe('subscription connect/reconnect', function() { let server; let web3; let accounts; + let subscription; const port = 8545; beforeEach(async function() { @@ -24,44 +25,34 @@ describe('subscription connect/reconnect', function() { } }); - it('subscribes (baseline)', function() { - return new Promise(async function (resolve) { - web3.eth - .subscribe('newBlockHeaders') - .on('data', function(result) { - assert(result.parentHash); - resolve(); - }); - }); + it('subscribes (baseline)', function(done) { + web3.eth + .subscribe('newBlockHeaders') + .on('data', function(result) { + assert(result.parentHash); + done(); + }); }); - it('subscribes with a callback', function(){ - let subscription; - - return new Promise(async function (resolve) { - subscription = web3.eth - .subscribe('newBlockHeaders', function(err, result){ - assert(result.parentHash); - subscription.unsubscribe(); // Stop listening.. - resolve(); + it('subscribes with a callback', function(done){ + subscription = web3.eth + .subscribe('newBlockHeaders', function(err, result){ + assert(result.parentHash); + subscription.unsubscribe(); // Stop listening.. + done(); }); - }) }); - it('resubscribes', function(){ - let subscription; + it('resubscribes', function(done){ + subscription = web3.eth.subscribe('newBlockHeaders') + subscription.unsubscribe(); + subscription.resubscribe() - return new Promise(async function (resolve) { - subscription = web3.eth.subscribe('newBlockHeaders'); - subscription.unsubscribe(); - subscription.resubscribe() - - subscription.on('data', function(result){ - assert(result.parentHash); - subscription.unsubscribe(); // Stop listening.. - resolve(); - }) - }) + subscription.on('data', function(result){ + assert(result.parentHash); + subscription.unsubscribe(); // Stop listening.. + done(); + }) }); it('allows a subscription which does not exist', function(){ @@ -70,7 +61,7 @@ describe('subscription connect/reconnect', function() { it('errors when zero params subscrip. is called with the wrong arguments', function(){ try { - web3.eth.subscribe('newBlockHeaders', 5); + web3.eth.subscribe('newBlockHeaders', 5) assert.fail(); } catch (err) { assert(err.message.includes('Invalid number of parameters for "newHeads"')) @@ -79,15 +70,13 @@ describe('subscription connect/reconnect', function() { }); // Could not get the .on('error') version of this to work - maybe a race condition setting it up. - it('errors when the provider is not set', function(){ + it('errors when the provider is not set', function(done){ web3 = new Web3(); - return new Promise(async function (resolve) { - web3.eth.subscribe('newBlockHeaders', function(err, result){ - assert(err.message.includes('No provider set')); - resolve() - }) - }); + web3.eth.subscribe('newBlockHeaders', function(err, result){ + assert(err.message.includes('No provider set')); + done(); + }) }); it('errors when the `eth_subscribe` request got send, the reponse isnt returned from the node, and the connection does get closed in the mean time', async function() { From 21d854c362475a694d79f32dc38dbfa4272e41f9 Mon Sep 17 00:00:00 2001 From: cgewecke Date: Tue, 17 Dec 2019 02:05:21 -0800 Subject: [PATCH 4/6] Rewrite resubscribe test per review comment --- test/eth.subscribe.ganache.js | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/test/eth.subscribe.ganache.js b/test/eth.subscribe.ganache.js index 763e75ec6c4..d111d74689b 100644 --- a/test/eth.subscribe.ganache.js +++ b/test/eth.subscribe.ganache.js @@ -44,15 +44,28 @@ describe('subscription connect/reconnect', function() { }); it('resubscribes', function(done){ - subscription = web3.eth.subscribe('newBlockHeaders') - subscription.unsubscribe(); - subscription.resubscribe() + let stage = 0; - subscription.on('data', function(result){ - assert(result.parentHash); - subscription.unsubscribe(); // Stop listening.. - done(); - }) + subscription = web3.eth + .subscribe('newBlockHeaders') + .on('data', function(result) { + assert(result.parentHash); + subscription.unsubscribe(); + stage = 1; + }); + + // Resubscribe from outside + interval = setInterval(async function(){ + if (stage === 1){ + clearInterval(interval); + subscription.resubscribe(); + subscription.on('data', function(result){ + assert(result.parentHash); + subscription.unsubscribe(); // Stop listening.. + done(); + }) + } + }, 500) }); it('allows a subscription which does not exist', function(){ From 1cb8b8034a615a04b22855049762f04f3a6ac931 Mon Sep 17 00:00:00 2001 From: cgewecke Date: Tue, 17 Dec 2019 02:19:53 -0800 Subject: [PATCH 5/6] Increase resubscribes timeout --- test/eth.subscribe.ganache.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/eth.subscribe.ganache.js b/test/eth.subscribe.ganache.js index d111d74689b..fbfa6cba40c 100644 --- a/test/eth.subscribe.ganache.js +++ b/test/eth.subscribe.ganache.js @@ -3,7 +3,7 @@ const ganache = require('ganache-cli'); const pify = require('pify'); const Web3 = require('./helpers/test.utils').getWeb3(); -describe('subscription connect/reconnect', function() { +describe.only('subscription connect/reconnect', function() { let server; let web3; let accounts; @@ -44,6 +44,8 @@ describe('subscription connect/reconnect', function() { }); it('resubscribes', function(done){ + this.timeout(5000); + let stage = 0; subscription = web3.eth From 7790a26ac5a8d72d5a54e520e21e7bd97484c8b9 Mon Sep 17 00:00:00 2001 From: cgewecke Date: Tue, 17 Dec 2019 02:29:30 -0800 Subject: [PATCH 6/6] Remove .only --- test/eth.subscribe.ganache.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/eth.subscribe.ganache.js b/test/eth.subscribe.ganache.js index fbfa6cba40c..1604f63c347 100644 --- a/test/eth.subscribe.ganache.js +++ b/test/eth.subscribe.ganache.js @@ -3,7 +3,7 @@ const ganache = require('ganache-cli'); const pify = require('pify'); const Web3 = require('./helpers/test.utils').getWeb3(); -describe.only('subscription connect/reconnect', function() { +describe('subscription connect/reconnect', function() { let server; let web3; let accounts;