Functional tx test type 2 - Closes #830 #839

diego-G · 2017-10-10T14:13:46Z

Closes #830

4miners · 2017-10-11T13:04:27Z

test/common/apiHelpers.js

 	getAccounts: getAccounts,
 	getAccountsPromise: getAccountsPromise,
 	getPublicKey: getPublicKey,
 	getBalancePromise: getBalancePromise,
 	getBalance: getBalance,
 	getPublicKeyPromise: getPublicKeyPromise
-};
+};


4miners · 2017-10-11T13:06:36Z

test/functional/http/get/delegates.js

+				getJsonForKeyPromise = node.Promise.promisify(cache.getJsonForKey);
+				node.expect(err).to.not.exist;
+				node.expect(__cache).to.be.an('object');
+				return done(err, __cache);


why return __cache here?

4miners · 2017-10-11T13:07:19Z

test/functional/http/get/delegates.js

+			cache.flushDb(function (err, status) {
+				node.expect(err).to.not.exist;
+				node.expect(status).to.equal('OK');
+				done(err, status);


why return status?

4miners · 2017-10-11T13:11:38Z

test/functional/http/get/delegates.js

-	it('using orderBy == "unknown:asc" should fail', function (done) {
-		var orderBy = 'unknown:asc';
-		var params = 'orderBy=' + orderBy;
+		it('using orderBy == "unknown:asc" should fail', function () {


why ==? globally

4miners · 2017-10-11T13:14:02Z

test/functional/http/get/delegates.js

-			for (var i = 0; i < res.body.delegates.length; i++) {
-				if (res.body.delegates[i + 1] != null) {
-					node.expect(res.body.delegates[i].rank).to.be.at.above(res.body.delegates[i + 1].rank);
+		it('using orderBy == "rank:desc" should be ok', function () {


btw, maybe change structure to something like:

describe('using orderBy'... it('rank:desc should be ok'...

4miners · 2017-10-11T13:21:49Z

test/functional/http/get/delegates.js


-		http.get('/api/delegates/forging/ge


4miners · 2017-10-11T13:25:02Z

test/functional/http/get/transactions.js

@@ -32,8 +32,8 @@ describe('GET /api/transactions', function () {
 	before(function () {

 		var promises = [];
-		promises.push(creditAccountPromise(account.address, 100*100000000));
-		promises.push(creditAccountPromise(account2.address, 20*100000000));
+		promises.push(creditAccountPromise(account.address, 100 * 100000000));


Use node.normalizer (name is not the best one) or maybe something from constants for calculations

4miners · 2017-10-11T13:26:19Z

test/functional/http/get/transactions.js

@@ -321,7 +318,7 @@ describe('GET /api/transactions', function () {
 		});

 		it('using no params should be ok', function () {
-			return getTransactionsPromise([]).then(function (res) {
+			return getTransactionsPromise(null).then(function (res) {


4miners · 2017-10-11T13:26:34Z

test/functional/http/get/transactions.js

 					node.expect(res).to.have.property('success').to.be.ok;
 					node.expect(res).to.have.property('transactions').that.is.an('array');

 					var dividedIndices = res.transactions.reduce(function (memo, peer, index) {
 						memo[peer[sortField] === null ? 'nullIndices' : 'notNullIndices'].push(index);
 						return memo;
-					}, {notNullIndices: [], nullIndices: []});
+					}, { notNullIndices: [], nullIndices: [] });


why spaces here?

Eslint does not complain, my editor suggested me.

4miners · 2017-10-11T13:29:59Z

test/node.js

@@ -266,13 +266,13 @@ node.expectedFeeForTrsWithData = function (amount) {

 // Returns a random username of 16 characters
 node.randomUsername = function () {
-	var randomLetter = randomString.generate({
+	var randomLetter = node.randomString.generate({


why first char need to be alphabetic lowercase?

It's just a choice. How would you make it?

New commits

SargeKhan · 2017-10-11T15:54:15Z

test/functional/http/get/delegates.js

-				node.expect(err).to.not.exist;
-				node.expect(res).to.eql(response);
-				done(err, res);
+			return getDelegatesPromise(null).then(function (res) {


no need to pass null

MaciejBaj · 2017-10-13T09:48:00Z

test/common/apiHelpers.js

@@ -5,6 +5,15 @@ var lisk = require('lisk-js');
 var node = require('../node');


Just a comment - please don't consider it as a requested changes but a thing to keep in mind for the future - the file is already big and it is pretty hard to find the functions.
Maybe it is better to organize it in a json structure like:

transactions: { get: function () .... getUnconfirmed: function () .... } delegates: { register: function () {.... }

Sure. It is time to think out a better way. I am prioritising the inclusion of the tests first to see the final picture.

SargeKhan · 2017-10-13T14:13:18Z

test/functional/http/get/delegates.js


-		http.get('/api/delegates/forging/getForgedByAccount?' + buildParams(), function (err, res) {
-			node.expect(res.


The test description says no end, and the test has end property.

Added commit

diego-G added 2 commits October 10, 2017 14:42

More functions to apiHelpers

74fced2

Review GET delegates tests

ebdf40b

diego-G added the in progress label Oct 10, 2017

diego-G assigned diego-G and Isabello Oct 10, 2017

diego-G requested review from LucasIsasmendi, SargeKhan, MaciejBaj, Isabello and 4miners October 10, 2017 14:13

diego-G added pending review and removed in progress labels Oct 10, 2017

Add functional test tx type 2

92559a7

diego-G force-pushed the 830-functional_test_type_2 branch from 3307bdf to 92559a7 Compare October 11, 2017 11:21

4miners previously requested changes Oct 11, 2017

View reviewed changes

diego-G added 2 commits October 11, 2017 16:45

Format & description changes

015e2ed

Use node.normalizer

ae798db

diego-G force-pushed the 830-functional_test_type_2 branch from d9990ae to ae798db Compare October 11, 2017 14:55

SargeKhan previously approved these changes Oct 11, 2017

View reviewed changes

diego-G added in progress and removed pending review labels Oct 12, 2017

diego-G dismissed SargeKhan’s stale review via 4e8c147 October 12, 2017 10:34

diego-G force-pushed the 830-functional_test_type_2 branch 2 times, most recently from 949164f to 1b4500c Compare October 12, 2017 10:43

Uniform api helpers params

1671f5d

diego-G force-pushed the 830-functional_test_type_2 branch from 1b4500c to 1671f5d Compare October 12, 2017 10:59

diego-G added pending review and removed in progress labels Oct 12, 2017

diego-G requested a review from SargeKhan October 12, 2017 13:00

diego-G requested a review from MaciejBaj October 12, 2017 14:37

4miners previously approved these changes Oct 13, 2017

View reviewed changes

MaciejBaj previously approved these changes Oct 13, 2017

View reviewed changes

Isabello previously approved these changes Oct 13, 2017

View reviewed changes

Isabello added ready and removed pending review labels Oct 13, 2017

SargeKhan previously requested changes Oct 13, 2017

View reviewed changes

Fix delegate test description

61d953c

diego-G dismissed stale reviews from Isabello, MaciejBaj, and 4miners via 61d953c October 13, 2017 14:20

diego-G requested review from SargeKhan, MaciejBaj and Isabello and removed request for LucasIsasmendi October 13, 2017 14:22

SargeKhan previously approved these changes Oct 13, 2017

View reviewed changes

Isabello previously approved these changes Oct 13, 2017

View reviewed changes

MaciejBaj previously approved these changes Oct 13, 2017

View reviewed changes

Merge branch '1.0.0' into 830-functional_test_type_2

2e90679

Isabello dismissed stale reviews from MaciejBaj, SargeKhan, and themself via 2e90679 October 13, 2017 16:05

Correct ESLint failure

60cb44e

LucasIsasmendi approved these changes Oct 13, 2017

View reviewed changes

MaciejBaj approved these changes Oct 13, 2017

View reviewed changes

Isabello approved these changes Oct 14, 2017

View reviewed changes

Isabello requested a review from LucasIsasmendi October 14, 2017 08:32

Isabello merged commit 5c8709d into 1.0.0 Oct 14, 2017

diego-G deleted the 830-functional_test_type_2 branch October 16, 2017 07:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Functional tx test type 2 - Closes #830 #839

Functional tx test type 2 - Closes #830 #839

diego-G commented Oct 10, 2017

4miners Oct 11, 2017

4miners Oct 11, 2017

4miners Oct 11, 2017

4miners Oct 11, 2017

4miners Oct 11, 2017

4miners Oct 11, 2017

4miners Oct 11, 2017

4miners Oct 11, 2017

4miners Oct 11, 2017

diego-G Oct 11, 2017

4miners Oct 11, 2017

diego-G Oct 12, 2017 •

edited

Loading

SargeKhan Oct 11, 2017

MaciejBaj Oct 13, 2017

diego-G Oct 13, 2017

SargeKhan Oct 13, 2017

		@@ -5,6 +5,15 @@ var lisk = require('lisk-js');
		var node = require('../node');


		http.get('/api/delegates/forging/getForgedByAccount?' + buildParams(), function (err, res) {
		node.expect(res.

Functional tx test type 2 - Closes #830 #839

Functional tx test type 2 - Closes #830 #839

Conversation

diego-G commented Oct 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diego-G Oct 12, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diego-G Oct 12, 2017 •

edited

Loading