Lisk-sdk: Unit tests standards

[MaciejBaj]

To keep the same convention across the Lisk unit tests suite, I propose to stick to the same rules:
1. Format
- a) add empty lines after describe blocks

describe('a'), function () {

   describe('b', function () {
      ...
   });
});

• b) keep variables initialization in seperate lines
  var a, b;

var a;
var b;

• c) prefer beforeEach blocks for objects initialization. The tests would be atomic then and the potential modification will not affect other tests.

var obj;

describe('a'), function () {

   beforeEach(function () {
      obj = {prop: 1};
   });

   it('will modify obj.prop but beforeEach will restore it for the next test', function () {
      obj.prop += 1;
   });
});

1. Grammar
   For making descriptions omit the articles a, an, the.

2. Cover another function calls by tests
   Test also each parameter that the function is being called with separately.
   In tests the calls can be mocked easily.

   // Code:
   function foo () {
   bar('a', 'b');
   }
   // Test:
   describe('foo', function () {
   
   it('should call bar');
   
   it('should call bar with "a"');
   
   it('should call bar with "b"');
   });
   

3. Cover all paths
   All code paths should be covered and described by separate contexts

4. if conditions

   // Code:
   function foo (a) {
   if (a === 'a') { ... }
   ...
   }
   // Test:
   describe('foo', function () {
   
   describe('a = "a"' ... );
   
   describe('a != "a"' ... );
   });
   

5. when function path depends from other asynchronous function results split the contexts into two paths: succeeds and fails.

   // Code:
   function foo () {
   bar(function (err, res) {
     if (err) { return err }
     return res;
   });
   }
   // Test:
   describe('foo', function () {
   
   it('should call bar');
   
   describe('when bar succeeds' ... );
   
   describe('when bar fails' ... );
   });
   

6. Returned results

7. synchronous function
   describe returned result including a type description. Describe returned fields in case it is an object (should return result contining fieldA). Include a type description for different types (should return result as string). Use strict assignment for the constants (should return result = null or result = undefined or result = "A").

   // Code:
   function foo () {
   var obj = {prop: 1};
   return obj;
   }
   // Test:
   describe('foo', function () {
   
   it('should return obj containing prop');
   });
   

8. asynchronous function
   due to the convention of calling a callback with error as a first parameter and a result as a second one test should also follow the manner. Always describe the returned error, using error = ... pattern. Use the same pattern to describe the function result: result = ....

   // Code:
   function foo (cb) {
   return cb(null, {a: 'a'});
   }
   // Test:
   describe('foo', function () {
   
   it('should call callback with error = null');
   
   it('should call callback with result containing a');
   });
   

[willclarktech]

On the first point, our eslint config (which we're planning to add to lisk-core at some point) inherits from Airbnb the rule that blocks should not be padded with blank lines.

Good:

describe('some description', function () {
    describe('some other description', function () {
    // ...
    }
}

Bad:

describe('some description', function () {

    describe('some other description', function () {
    // ...
    }

}

If we want something different we should update https://github.com/LiskHQ/eslint-config-lisk-base accordingly.

[MaciejBaj]

Thanks, Will. It is easier to change it the rule 1 a) for no lines in our codebase to be unified with other LiskHQ projects.

[Isabello]

Is omitting articles really that helpful? I feel like test descriptions could be more human readable/writable if we allowed them.

[karmacoma]

Some of standards are outdated by prettier inclusion. Instead we shall write up some documentation on testing standards which can be added to our contribution guide(s).