Functional test tx type 3 - Subtask #678 - Close #835 #882

diego-G · 2017-10-19T08:08:42Z

Subtask #678
Close #835

MaciejBaj · 2017-10-19T09:50:33Z

test/functional/http/post/3.votes.js

+	var accountMinimalFunds = node.randomAccount();
+	var accountDuplicate = node.randomAccount();
+
+	var account33 = node.randomAccount();


Is it some better name for account33?

MaciejBaj · 2017-10-19T09:52:27Z

test/functional/http/post/3.votes.js

+		})
+			.then(function (res) {
+				var promisesCredits33 = [];
+				for (var i = 0; i < 33; i++) {


Try to name the variables instead of using magic numbers.
var MAX_VOTES_IN_ONE_TRANSACTION = 33;

MaciejBaj · 2017-10-19T09:53:41Z

test/functional/http/post/3.votes.js

+	var transaction;
+
+	before(function () {
+		// Crediting accounts and registering delegates


I would appreciate a better description - why all the steps below are needed. The scenario descriptions.

MaciejBaj · 2017-10-19T09:54:01Z

test/functional/http/post/3.votes.js

+
+	describe('transactions processing', function () {
+
+		it('upvoting with manipulated vote should fail', function () {


with "++" preset

MaciejBaj · 2017-10-19T09:55:11Z

test/functional/http/post/3.votes.js

+			});
+		});
+
+		it('downvoting with manipulated vote should fail', function () {


MaciejBaj · 2017-10-19T09:56:20Z

test/functional/http/post/3.votes.js

+			});
+		});
+
+		it('upvoting 33 delegates at once should be ok', function () {


33 (maximum amount)

MaciejBaj · 2017-10-19T09:56:41Z

test/functional/http/post/3.votes.js

+			});
+		});
+
+		it('upvoting 101 delegates separately should be ok', function () {


Why is upvoting 102 delegates done in the different place?

MaciejBaj · 2017-10-19T09:56:55Z

test/functional/http/post/3.votes.js

+			});
+		});
+
+		it('upvoting 34 delegates at once should fail', function () {


34 (1 extra)

MaciejBaj · 2017-10-19T09:57:53Z

test/functional/http/post/3.votes.js

+			});
+		});
+
+		it('self downvoting should be ok', function () {


Missing self downvoting twice should fail?

MaciejBaj · 2017-10-19T10:00:10Z

test/functional/http/post/3.votes.js

+				goodTransactions.push(transaction);
+			});
+		});
+


Why is downvoting done in a different place?

Because It performs the downvoting over the same fresh accounts just voted in the prior phase.

Comments added

- Better variable naming - Add comments - Improve descriptions - Get constants from core - Add new test for unconfirmed states - Review arrays of enforcement transactions

MaciejBaj

Good job, few issues mentioned in comments, in addition to that:

you are testing upvoting with '++', but not downvoting with '--'
ss I mentioned the last time - would be really nice if above the before the section you will write the scenario of those tests as the process is complex

MaciejBaj · 2017-10-20T07:52:55Z

test/functional/http/post/3.votes.js

+
+	describe('transactions processing', function () {
+
+		it('voting with invalid vote format should fail', function () {


voting with "++" operator should fail

MaciejBaj · 2017-10-20T07:54:15Z

test/functional/http/post/3.votes.js

+			});
+		});
+
+		it('voting with invalid vote length should fail', function () {


so the test is about voting with invalid length - don't include the special "--" operator here

MaciejBaj · 2017-10-20T07:54:30Z

test/functional/http/post/3.votes.js

+			});
+		});
+
+		it('using invalid vote operator \'x\' should fail', function () {


Thanks for insisting. Also it improves promises code format

MaciejBaj · 2017-10-20T11:21:34Z

test/functional/http/post/3.votes.js

+	   In this case, we create as many delegates as the network allows to be active. This parameter
+       is directly related to the maximum number of allowed votes per account. Another independent 
+	   account is credited and performs tests uniquely to delegates from this scenario.
+	*/


Fixed typos:
Creating two scenarios with two isolated set of accounts

First scenario --> MaxVotesPerTransaction
We register the exact amount of delegates the networks allows to be voted per transaction.
A new fresh account is credited to perform the voting tests to those delegates.

Second scenario --> MaxVotesPerAccount
In this case, we create as many delegates as the network allows to be active. This parameter
is directly related to the maximum number of allowed votes per account. Another independent
account is credited and performs tests uniquely to delegates from this scenario.

SargeKhan · 2017-10-20T08:11:16Z

test/functional/http/post/3.votes.js

+
+	describe('unconfirmed state', function () {
+
+		it('upvoting with valid params and duplicate submission should be ok but just first transaction to arrive will be confirmed', function () {


test and its description are opposite,

Isabello

Fix these descriptions, please.

Isabello · 2017-10-20T12:47:50Z

test/functional/http/post/3.votes.js

+			});
+		});
+
+		it('using invalid vote operator "x" should fail', function () {


can we not use double quotes in descriptions? feels wrong.

Isabello · 2017-10-20T12:48:56Z

test/functional/http/post/3.votes.js

+
+	describe('unconfirmed state', function () {
+
+		it('upvoting with valid params and duplicate submission should be ok but just last transaction to arrive will be confirmed', function () {


this description needs some help.

Isabello · 2017-10-20T12:49:44Z

test/functional/http/post/3.votes.js

+
+	describe('unconfirmed state after validation', function () {
+
+		it('downvoting with valid params and duplicate submission should be ok but just last transaction to arrive will be confirmed', function () {


Add commit

Add transfer transaction class - Closes #882

Add functional test tx type 3

ee51a3c

diego-G added the pending review label Oct 19, 2017

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

diego-G requested review from LucasIsasmendi, SargeKhan, MaciejBaj, Isabello and 4miners October 19, 2017 08:08

MaciejBaj previously requested changes Oct 19, 2017

View reviewed changes

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

diego-G requested a review from MaciejBaj October 19, 2017 15:37

Improve general comprehension

1e65dcd

- Better variable naming - Add comments - Improve descriptions - Get constants from core - Add new test for unconfirmed states - Review arrays of enforcement transactions

diego-G force-pushed the 835-functional_test_type_3 branch from 03befea to 1e65dcd Compare October 19, 2017 15:53

MaciejBaj previously requested changes Oct 20, 2017

View reviewed changes

diego-G requested a review from MaciejBaj October 20, 2017 11:18

MaciejBaj reviewed Oct 20, 2017

View reviewed changes

diego-G force-pushed the 835-functional_test_type_3 branch from bfaa5eb to 871aea9 Compare October 20, 2017 11:26

MaciejBaj previously approved these changes Oct 20, 2017

View reviewed changes

diego-G dismissed MaciejBaj’s stale review via 67e5541 October 20, 2017 11:27

diego-G force-pushed the 835-functional_test_type_3 branch from 871aea9 to 67e5541 Compare October 20, 2017 11:27

diego-G requested a review from MaciejBaj October 20, 2017 11:28

SargeKhan reviewed Oct 20, 2017

View reviewed changes

MaciejBaj previously approved these changes Oct 20, 2017

View reviewed changes

Improve comments & code format

48844e9

diego-G dismissed MaciejBaj’s stale review via 48844e9 October 20, 2017 12:29

diego-G force-pushed the 835-functional_test_type_3 branch from 67e5541 to 48844e9 Compare October 20, 2017 12:29

diego-G requested review from SargeKhan and MaciejBaj October 20, 2017 12:35

SargeKhan previously approved these changes Oct 20, 2017

View reviewed changes

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

MaciejBaj previously approved these changes Oct 20, 2017

View reviewed changes

Isabello previously requested changes Oct 20, 2017

View reviewed changes

Remove obsolete checks before new block

6c5ad3f

diego-G dismissed stale reviews from MaciejBaj and SargeKhan via 6c5ad3f October 20, 2017 14:09

Improve descriptions for critical tests

019a11d

diego-G requested review from SargeKhan, MaciejBaj and Isabello October 20, 2017 14:32

SargeKhan approved these changes Oct 20, 2017

View reviewed changes

Isabello approved these changes Oct 20, 2017

View reviewed changes

MaciejBaj approved these changes Oct 20, 2017

View reviewed changes

Isabello merged commit c0d2637 into 1.0.0 Oct 20, 2017

Isabello deleted the 835-functional_test_type_3 branch October 20, 2017 14:56

diego-G restored the 835-functional_test_type_3 branch October 20, 2017 15:50

Isabello added ready and removed pending review labels Nov 2, 2017

shuse2 added a commit that referenced this pull request Apr 15, 2019

Merge pull request #1002 from LiskHQ/882-add_transfer_class

84cf90a

Add transfer transaction class - Closes #882

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Functional test tx type 3 - Subtask #678 - Close #835 #882

Functional test tx type 3 - Subtask #678 - Close #835 #882

diego-G commented Oct 19, 2017

MaciejBaj Oct 19, 2017

MaciejBaj Oct 19, 2017

MaciejBaj Oct 19, 2017

MaciejBaj Oct 19, 2017

MaciejBaj Oct 19, 2017

MaciejBaj Oct 19, 2017

MaciejBaj Oct 19, 2017

MaciejBaj Oct 19, 2017

MaciejBaj Oct 19, 2017

MaciejBaj Oct 19, 2017

diego-G Oct 19, 2017

MaciejBaj left a comment

MaciejBaj Oct 20, 2017

MaciejBaj Oct 20, 2017

MaciejBaj Oct 20, 2017

MaciejBaj Oct 20, 2017

SargeKhan Oct 20, 2017

Isabello left a comment

Isabello Oct 20, 2017

diego-G Oct 20, 2017

Isabello Oct 20, 2017

Isabello Oct 20, 2017


		describe('transactions processing', function () {

		it('upvoting with manipulated vote should fail', function () {


		describe('transactions processing', function () {

		it('voting with invalid vote format should fail', function () {


		describe('unconfirmed state', function () {

		it('upvoting with valid params and duplicate submission should be ok but just first transaction to arrive will be confirmed', function () {


		describe('unconfirmed state after validation', function () {

		it('downvoting with valid params and duplicate submission should be ok but just last transaction to arrive will be confirmed', function () {

Functional test tx type 3 - Subtask #678 - Close #835 #882

Functional test tx type 3 - Subtask #678 - Close #835 #882

Conversation

diego-G commented Oct 19, 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

MaciejBaj left a 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

Isabello left a 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