Add testing guide

Hi friends! I’m back, i’ts been two crazy months since I moved to BCN, met some people from aragon and also the white hat group! But I’ve been focused mostly on frontend, need to get back to testing so I’m about to start checking what you’ve been up to and continue with the testing guide.

I still have some concerns about this, there are a few helpers I really dont understand so some help will be much appreciated and also when and how should I start writing markdown.

Glad to be back on this and see all the progress!

2 Likes

Hey there @ianbrtt!

I see that since you were working on this a little bit has changed for us in terms of testing. We've released openzeppelin-test-helpers and migrated the test suite to use that package.

People had been using our helpers informally for a long time, so we decided to make them a proper package. The helpers are basically the same than the ones we had before, though we've been improving them at a faster pace now.

Another big difference is that the helpers have their own documentation now. Any doubts you have about the helpers themselves should be explained in that reference, and if they aren't explained please open an issue or a pull request.

What I think we're missing is a self-contained guide/tutorial on "how OpenZeppelin tests its smart contracts", that can help people test their own contracts or contribute to OpenZeppelin. If you'd like to continue working on this guide, we think that this should be the focus.

We put together some points that describe how a contract's tests are usually structured:

  • Top level its that test the ways the constructor can revert.

  • A main top level context to test all functionality.

    • A beforeEach block that deploys the contract.
    • Tests for the simple view functions.
    • A describe block for each "big feature" of the contract.
      • A test for each way an argument can be wrong (seeing the requires in the code).
      • Tests for the happy paths. If there is a common setup step for several tests, group them in a describe block and add the setup in beforeEach. Make sure to test edge cases.
  • Potentially a few other contexts to test some functionality with edge cases in the constructor parameters. (For example, a PaymentSplitter with a single payee.)

  • All of the above has to be done by reading the source or the spec if available.

Ideally what we want is a guide describing this in a bit more detail, and pointing out examples in the repository. In some cases it could be a good idea to point out helpers that might be useful.

2 Likes

A test guide covering the OpenZeppelin test methodology will be awesome.

I had previously gleemed what I could from the OpenZeppelin repository to teach me how to test my smart contracts.

I made my first PR to OpenZeppelin recently and definitely needed guidance on the testing.

As an aside, the OpenZeppelin test helpers are great to use.

3 Likes

Thanks fran, I’ll start writing the guide in markdown with this in mind and then PR. “how OpenZeppelin tests its smart contracts” sounds great. Thanks @abcoathup too for the interest, will let you know when I have the first draft of the guide ready and we can think about ways to improve it.

3 Likes

[ WIP ] First Draft of the Testing Guide.

How OpenZeppelin Tests its Smart Contracts.

Install

git clone https://github.com/OpenZeppelin/openzeppelin-solidity.git
cd openzeppelin-solidity
npm install 

We now have all the dependencies from the package installed and we are ready to go.

Usage

To run all the tests:

npm test 

If you want to test a particular contract, describe, context or it, you can add a .only, after them and npm test will only run the test for that file.

contract.only('Roles', function ([_, authorized, otherAuthorized, other]) {

The next should display in the console:

  Contract: Roles
    ✓ reverts when querying roles for the zero account (124ms)
    initially
      ✓ doesn't pre-assign roles (528ms)
      adding roles
        ✓ adds roles to a single account (370ms)
        ✓ reverts when adding roles to an already assigned account (291ms)
        ✓ reverts when adding roles to the zero account (133ms)
    with added roles
      removing roles
        ✓ removes a single role (356ms)
        ✓ reverts when removing unassigned roles (160ms)
        ✓ reverts when removing roles from the zero account (150ms)
  
  8 passing (5s)

Reference

Let’s see how Roles.test.js works.

Import all required modules from openzeppelin-test-helpers


const { expectRevert, constants } = require('openzeppelin-test-helpers');
const { ZERO_ADDRESS } = constants;

The contract being tested will be RolesMock.sol.

const RolesMock = artifacts.require('RolesMock');

The mock contracts are a collection of abstract contracts that can be used as the foundation for your own custom implementations. These mocks demonstrate how OpenZeppelin’s secure base contracts can be used with multiple inheritance. They are primarily used for tests, but also serve as good examples for usage and combinations.

To “wrap” all the tests in this file, OpenZeppelin uses the contract() function instead of describe(), as explained in the truffle docs.

contract('Roles', function ([_, authorized, otherAuthorized, other]) {
  beforeEach(async function () {
    this.roles = await RolesMock.new();
  });

BeforeEach contract() function is run, the contracts are redeployed to ganache-cli so the tests within it run with a clean contract state. Each test uses a brand new instance of a contract.

Top level it test the ways the constructor can revert.

  it('reverts when querying roles for the zero account', async function () {
    await expectRevert(this.roles.has(ZERO_ADDRESS), 'Roles: account is the zero address');
  });

A main top level context test all the functionality.

  context('initially', function () {
    it('doesn\'t pre-assign roles', async function () {
      (await this.roles.has(authorized)).should.equal(false);
      (await this.roles.has(otherAuthorized)).should.equal(false);
      (await this.roles.has(other)).should.equal(false);
    });

    describe('adding roles', function () {
      it('adds roles to a single account', async function () {
        await this.roles.add(authorized);
        (await this.roles.has(authorized)).should.equal(true);
        (await this.roles.has(other)).should.equal(false);
      });

      it('reverts when adding roles to an already assigned account', async function () {
        await this.roles.add(authorized);
        await expectRevert(this.roles.add(authorized), 'Roles: account already has role');
      });

      it('reverts when adding roles to the zero account', async function () {
        await expectRevert(this.roles.add(ZERO_ADDRESS), 'Roles: account is the zero address');
      });
    });
  });

describe()’s are commonly known as test suites, which contain test cases. They are merely used for grouping, and you can have groups within groups.


  context('with added roles', function () {
    beforeEach(async function () {
      await this.roles.add(authorized);
      await this.roles.add(otherAuthorized);
    });

    describe('removing roles', function () {
      it('removes a single role', async function () {
        await this.roles.remove(authorized);
        (await this.roles.has(authorized)).should.equal(false);
        (await this.roles.has(otherAuthorized)).should.equal(true);
      });

      it('reverts when removing unassigned roles', async function () {
        await expectRevert(this.roles.remove(other), 'Roles: account does not have role');
      });

      it('reverts when removing roles from the zero account', async function () {
        await expectRevert(this.roles.remove(ZERO_ADDRESS), 'Roles: account is the zero address');
      });
    });
  });
});

  • Is the Roles.test.js a good example to use? I try to explain in a way that applies for all tests.
  • I will add more about BDD, external documentation about mocha and async/await, describe every it, how it uses web3.js and BN, link to helpers.
3 Likes

Hey all! I just changed the topic name (it now begins with “Add”) to better reflect what the thread is about, otherwise it seems like the actual testing guide instead of a discussion.

1 Like

Thanks @martriay, I plan to finish the guide this week.

2 Likes

Cool, that’s awesome! :slight_smile:

1 Like

Great start @ianbrtt

Look forward to the section on BDD and mocha.

1 Like

I'm not sure it is. It's an unusual test because it's a library and not a contract, thus it doesn't have a constructor, for example. This is important because it's one of the things that are reflected in the test structure that I shared above. It's also important because we want this guide to be helpful to contract developers in general, and this kind of library is unlikely to be the thing they are working on.

The structure of the guide is good I think. I would change the focus a bit and make it more of a general guide which happens to show a few code samples, instead of a walkthrough of a test file in particular. Along these lines, you mentioend "describe every it" as one of the next steps, though this is definitely not a requirement for this guide. So don't worry about that level of detail, instead focus on the way the tests are structured as a whole.

3 Likes

@ianbrtt Ownable or a Simple ERC20 token could be good examples to use. They are both concepts that developers are likely to either know a little about or come across early on. Otherwise we need to devise a simpler example.

Also could be worth mentioning OpenZeppelin design guidelines as good practice to follow.

2 Likes

Thanks a lot @frangio and @abcoathup, just what I needed.

3 Likes

Here we go, still WIP but I think is getting closer.

Tried to make it more general, plan to go deeper on every description but I’d like to know if I’m going in this direction:

Screenshot%20from%202019-05-31%2011-34-36


How OpenZeppelin Tests its Smart Contracts.

Install

git clone https://github.com/OpenZeppelin/openzeppelin-solidity.git
cd openzeppelin-solidity
npm install

Usage

To run all the tests:

npm test

To test a particular contract, describe, context or it , add a .only after them and npm test will only run the test for that file.

contract.only('Ownable', function ([_, owner, ...otherAccounts]) {...}

The next should display in the console:

Contract: Ownable
  as an ownable
    ✓ should have an owner (79ms)
    ✓ changes owner after transfer (475ms)
    ✓ should prevent non-owners from transferring (166ms)
    ✓ should guard ownership against stuck state (162ms)
    ✓ loses owner after renouncement (338ms)
    ✓ should prevent non-owners from renouncement (157ms)

6 passing (3s)

Reference

Helpers

Import all required modules from openzeppelin-test-helpers


const { expectRevert, constants } = require('openzeppelin-test-helpers');
const { ZERO_ADDRESS } = constants;

Migrations

OpenZeppelin uses Truffle Framework for testing, but instead of using the truffle migrations, it imports the contract artifacts (a truffle-contract object) in each test file:

const Ownable = artifacts.require('OwnableMock');

That then lets you deploy new instances of the contract using new:

this.ownable = await Ownable.new({ from: owner });

Mocks

The contract being tested in this example is OwnableMock.sol.

The mock contracts are a collection of abstract contracts that can be used as the foundation for your own custom implementations. These mocks demonstrate how OpenZeppelin’s secure base contracts can be used with multiple inheritance. They are primarily used for tests, but also serve as good examples for usage and combinations.

Async/Await

Async/Await is a way to write asynchronous code.

  • Async ensures that the function always returns a promise.
  • Await expression causes then Async function execution to pause until a Promise is resolved (fulfilled or rejected).
  • When resumed, the value of the Await expression is that of the fulfilled Promise.

More about Async/Await.

BDD (Behavior Driven Development)

OpenZeppelin uses Mocha’s BDD Interface to test it’s Smart Contracts.

contract ( )

The contract() function “wraps” all the tests, as explained in the truffle docs.

contract('Ownable', function ([_, owner, ...otherAccounts]) {
  beforeEach(async function () {
    this.ownable = await Ownable.new({ from: owner });
  });

  shouldBehaveLikeOwnable(owner, otherAccounts);
});

beforeEach ( )

Before each contract() function is run, the contracts are redeployed to ganache-cli so the tests within it run with a clean contract state.

behavior

const { shouldBehaveLikeOwnable } = require('./Ownable.behavior');

describe()

Commonly known as test suites, which contain it() test cases . They are merely used for grouping, and you can have groups within groups.

describe('as an ownable', function () {
  it('should have an owner', async function () {
    (await this.ownable.owner()).should.equal(owner);
  });
});

context()

context() is just an alias for describe() and behaves the same way; it just provides a way to keep tests easier to read and organized.

Hi all! I think it’s almost there, what do you think? @nventuro @frangio @abcoathup @martriay.

  • I can add more about web3.js, it()'s and examples.
  • I was thinking if I should PR in the docs, right?.

How OpenZeppelin Tests its Smart Contracts.

Install

git clone https://github.com/OpenZeppelin/openzeppelin-solidity.git
cd openzeppelin-solidity
npm install

Usage

To run all the tests:

npm test

To test a particular contract, describe, context or it , add a .only after them and npm test will only run the test for that file.

contract.only('Ownable', function ([_, owner, ...otherAccounts]) {...}

The next should display in the console:

Contract: Ownable
  as an ownable
    ✓ should have an owner (79ms)
    ✓ changes owner after transfer (475ms)
    ✓ should prevent non-owners from transferring (166ms)
    ✓ should guard ownership against stuck state (162ms)
    ✓ loses owner after renouncement (338ms)
    ✓ should prevent non-owners from renouncement (157ms)

6 passing (3s)

Reference

OpenZeppelin Tests its Smart Contracts using Truffle 5, Web3 1.0, Mocha, Chai and BN.

Helpers

Import all required modules from openzeppelin-test-helpers.

const { expectRevert, constants } = require('openzeppelin-test-helpers');
const { ZERO_ADDRESS } = constants;

Migrations

Import the contract artifacts (a contract abstraction), in each test file.

const Ownable = artifacts.require('OwnableMock');

This allows to deploy a new instance of the contract using new:

this.ownable = await Ownable.new({ from: owner });

The truffle migrate command is not used.

Mocks

OwnableMock.sol is a contract that imports and inherits Ownable.sol.

The mock contracts are a collection of abstract contracts that can be used as the foundation for your own custom implementations. These mocks demonstrate how OpenZeppelin’s secure base contracts can be used with multiple inheritance. They are primarily used for tests, but also serve as good examples for usage and combinations.

Behavior

Group of tests, modular and reusable.

const { shouldBehaveLikeOwnable } = require('./Ownable.behavior');

BDD (Behavior Driven Development)

OpenZeppelin uses Mocha’s BDD Interface to test it’s Smart Contracts.

Async / Await

Async / Await is a way to write asynchronous code.

  • Async ensures that the function returns a promise.
  • Await makes JavaScript wait until that promise is resolved (fulfilled or rejected) and returns its result.
  • When returned, the value of the await expression is that of the fulfilled Promise.

beforeEach() contract()

Before each contract() function is run, the contracts are redeployed to ganache-cli so the tests within it run with a clean contract state, like explained in the Truffle docs.

The contract() function contains all the tests.

contract('Ownable', function ([_, owner, ...otherAccounts]) {
  beforeEach(async function () {
    this.ownable = await Ownable.new({ from: owner });
  });

  shouldBehaveLikeOwnable(owner, otherAccounts);
});

describe()

Commonly known as test suites, which contain test cases it() 's. They are merely used for grouping “big features” of the contract, and you can have groups within groups. context() is just an alias for describe().

describe('as an ownable', function () {
  it('should have an owner', async function () {
    (await this.ownable.owner()).should.equal(owner);
  });
});

it()

it()'s are usually known as test cases and they are used to test for each way an argument can be wrong.

  • Top level it() tests the ways the constructor() can revert, like the example above.

  • Other it()'s to test all the functionalities of the contract and make sure to cover the happy paths, simple view functions and edge cases.

it('changes owner after transfer', async function () {
  (await this.ownable.isOwner({ from: other })).should.be.equal(false);
  const { logs } = await this.ownable.transferOwnership(other, { from: owner });
  expectEvent.inLogs(logs, 'OwnershipTransferred');

  (await this.ownable.owner()).should.equal(other);
  (await this.ownable.isOwner({ from: other })).should.be.equal(true);
});

Great work putting this together. :smiley:

I went back and read the original issue:

Add testing guide #1077
https://github.com/OpenZeppelin/openzeppelin-solidity/issues/1077
We should have a readme in the test directory, explaining how OZ tests are structured, linking to relevant truffle docs and maybe including a couple of frequent errors.

Assuming this is still the case, then assume the guide should be a PR to https://github.com/OpenZeppelin/openzeppelin-solidity/blob/master/test/TESTING.md

This guide could then be referenced at a higher level in the docs for how OpenZeppelin users (as opposed to contributors) should test their smart contracts.


Please find below my feedback (though it is just my opinion from my perspective :smile:)

The guide is heavily specific on Ownable and may need to be a bit more generic.
We could add something like: To assist with understanding how OpenZeppelin tests are written, Ownable.sol is used as an example. We could also have links to Ownable and the Ownable tests.

Suggest having a sub heading: Run specific tests, to separate this.
I didn't know that you could do .only. Nice. I have previously run a specific test file but this gives greater granularity.

Assume you mean:
The following should display in the console:

Should this be split into explicit (direct dependencies) and implicit (indirect dependencies) e.g. Truffle (web3) and exclude versions (to future proof this document, though the openzeppelin-test-helpers documentation includes versions)?

Should we have a section that covers the overall structure of tests (also acts as a table of contents for the breakdown of each component afterwards):
Test structure generally consists of:

  • Helpers
  • Import contract artifacts
  • contract - contains all the tests for a contract
    • beforeeach - deploy contracts to run each test in a clean state
    • describe - test suites
      • it - test cases

Should we link to the repo to see the README.md rather than the src?

Should we have something like: OpenZeppelin test helpers have been separated into their own repository so that they can be used by other projects for their testing.

Should we add something to this section like: OpenZeppelin generally uses Mock contracts inheriting from the contract or library under test.

Should we add something to explain behaviors like:
Behavior files export a function that can be reused for testing different contracts, while tests have the tests themselves, possibly using one of the behaviors.

Should we cover that: OpenZeppelin uses expect rather than should. If we could explain why we chose expect that would help with understanding.


Should we have a reference to the Guidelines section on testing? https://github.com/OpenZeppelin/openzeppelin-solidity/blob/master/GUIDELINES.md#testing

Should we should include that tests follow a specific style and are linted. Linting is run using npm lint:js and fixed using npm lint:js:fix

Should we also see if there are any other recommendations? e.g. avoid global variables, use account names, don't use account[0]

1 Like

That’s some great feedback @abcoathup thanks a lot. I agree and will add the things you mentioned, will let you know when I have it. I must say I don’t know how well I’m following the guidelines, trying to make it look good but for sure I need to improve that too.

1 Like