Add testing guide

(see #1077 for context)

@ianbrtt proposed to start working on this, but wanted some help understanding our current setup, style, etc. This is the place to ask questions about all of these topics and discuss possible approaches to each issue, the outcome of which should be a guide that both describes the rationale behind our tests, and helps new users write add good tests to both OpenZeppelin contributions and their own projects.

3 Likes

Great! So, I will start asking questions from a beginner point of view and then covering all necessary topics to have a good understanding of how this great project works.

I’ll be using:

  • Ubuntu 16.04 LTS
  • Npm 6.4.1
  • Node v10.14.1
  • Truffle v4.1.14 (core: 4.1.14)
  • Solidity v0.4.24 (solc-js)
  • Ganache CLI v6.2.3 (ganache-core: 2.3.1)
  • OpenZeppelin 2.0 RC 1 ( should I stick to this version? )

1Âş Download OpenZeppelin

Two ways:

I tried Git clone and it downloaded the whole repository…

Then tried Npm but it downloaded less files.

In the repository, I entered package.json and in the scripts section It says that tests will depend on the file scripts/test.sh .

  • In the npm installation there is no scripts folder. Shouldn’t have it? Or is it somewhere else because of npm?.

Also, The package.json its different in both examples. Repo (left) Npm (right)

Screenshot%20from%202018-12-21%2010-36-19

  • Is something wrong here or I’m making a mistake in the installation process?

Depends on what it is you want to do: users of the library install using npm install, developers clone the github repo, where they get all of the development-related files (including the test suite).

The npm installation should only have the files required to use the library, i.e. the contract source files and the built artifacts (build/contracts/*.json). IMO the test directory should not be there (since for a user to run the tests they'd need to also install our dependencies - I'd rather they just clone the repo in that case), but it may be also published so that people can use the test helpers: perhaps @frangio can shed some light on this?

I think npm modifies the package.json a bit when publishing, but there shouldn't be any major differences (e.g. different dependencies).

Good point, it depends on what the user wants. I will continue the next question assuming that we have cloned the github repo.

So wee cloned the repo and now we have a local copy, it’s a truffle project + other files. We will focus on:

  • contracts
  • migrations
  • truffle-config.js

I opened the console, went to the contracts folder, ran truffle complie and it worked, creating a build folder. Then tried truffle migrate to deploy it on a local blockchain using ganache-cli, but it didn’t work.

This is because we are not using truffle migrations, the migration folder is empty, which is confusing, should we remove it, or it would affect the project?. I can open an issue to address this.

  • How do we deploy a contract to the local blockchain if there is no migration files?
  • Do contracts need to be first deployed in order do be tested?

Truffle throws an error if the migrations directory doesn't exist when running truffle test, so we can't really delete it.

We import the contract artifacts (a truffle-contract object) in each test file: const ERC20Detailed = artifacts.require('ERC20Detailed'). That then lets you deploy new instances of the contract using new:

this.token = await ERC20Detailed.new("MyToken", "MTK", 18, { from: creator_account });

Each test file typically has beforeEach blocks where the contracts are deployed, so that every test (every it block) gets a brand new instance, preventing tests affecting each other.

Back to work :slight_smile:

While reading the tests files, saw they require other files from the helpers folder.

  • Are this helpers created from the zeppelin team or collected from somewhere else?
  • The main purpose of these files is modularity and re usability, am I right?

Would be nice to know If there is any source to learn more about where these helpers come from.

Historically they've come from different sources, some of them were added by contributors when opening a PR, because they noticed it was missing. I've been working on the openzeppelin-test-helpers package lately, which exports all of these, so that they can be reused by other people. Documentation etc. is still missing, but most of the code is already there.

1 Like

Now I started using version 2.1.1 that I downloaded here, then I went to the console, cd path / to / openzeppelin-solidity-2.1.1 , then npm install and it all went fine.

Then I npm test and found an error because the scripts/test.sh file has a different path to ganache-cli and truffle than mine. Someone else can have the same problem while doing this.

In my case, I installed NPM via NVM, so the path for me is /home/ian/.nvm/versions/node/v10.14.1/bin/ganache-cli (and the same for truffle)

Once I fixed this, tried npm test again and all the tests started raining on my screen, beautiful moment haha

  • What about this scripts folder? Does it come originally from npm? I don’t know bash but it’s kind of clear what every script does. I guess we shouldn’t play much with them.

So then I picked the first test that appeared in the console and started looking at it to see how it works

Screenshot%20from%202019-01-08%2011-25-11

This is the test of openzeppelin-solidity-2.1.1/test/cryptography/ECDSA.test.js, which starts like this:

This “header” requires a few files from the helpers folder and also the solidity contract ECDSAMock from the contracts/mocks folder. Also uses web3.sha3 to hash two messages. (I’ll be making a more detailed explanation of all this when describing a newer test like pausable).

Then, we use the contract() function instead of describe(), as explained in the truffle docs.

Before each 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.

  • Any comment or addition to this first part would be super useful.
  • How do you pick a specific test and deploy that one only?
  • I don’t see a new ganache-cli console window when I deploy the tests, should I? I know its running in the background though.

Hmm, interesting. What if you instead run npx ganache-cli?

They are just some simple scripts we wrote whenever some process needed some extra customization. The only interesting ones are test and build, iirc.

That is because ganache is being run in the background by the script, like you noticed. You can run ganache yourself, and the suite will use that instance.

You can add .only to any contract, describe, context or it Mocha block, and npm test will only test that file. Alternatively, you can force truffle to only test a single file with npx truffle test <file>, but that may require you launching ganache-cli on your own beforehand.

1 Like

In order to understand what the Pausable.test.js does, we should first understand the contract being tested, which is actually PausableMock.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.

  • Is it the right description to use? took it from here.

PausableMock.sol

pragma solidity ^0.5.0;

import "../lifecycle/Pausable.sol";
import "./PauserRoleMock.sol";

// mock class using Pausable
contract PausableMock is Pausable, PauserRoleMock {
    bool public drasticMeasureTaken;
    uint256 public count;

    constructor () public {
        drasticMeasureTaken = false;
        count = 0;
    }

    function normalProcess() external whenNotPaused {
        count++;
    }

    function drasticMeasure() external whenPaused {
        drasticMeasureTaken = true;
    }
}
  1. It first imports and then inherits Pausable.sol and PauserRoleMock.sol. More info about them in the API Reference and Role-Based Access Control & Roles.sol.

  2. The constructor sets the initial values of drasticMeasureTaken and count, whose values will change whenever the test calls for the two functions drasticsMeasure() and normalProcess().

  3. These functions can only be called from the outside of the contract (external) and only if their modifiers (whenNotPaused and whenPaused) evaluate true.

So to deploy this test only, I first went to the Pausable.test.js and added a .only in contract():

I went to the console and typed ganache-cli to have a separate window and check that the contract is being deployed correctly.

Then I npm test in another console and it worked, but it first compiled all the contracts again.

  • Is it necesary or a problem to go always through this process? Is there any other way to just compile and use the contract you need? I guess it can be tweaked on scripts/compile.sh.
  • How this.pausable and this.contract work?

Pausable.test.js is a fairly complex test, we have to go back and forth among many files to understand what every part does.

We first set a pauser role in Pausable.test.js:

But we have to jump to another js file for now. In line 5, we require a function from PublicRole.behavior.js.

const { shouldBehaveLikePublicRole } = require('../access/roles/PublicRole.behavior');

When we deploy the test, half of it will be about this shouldBehaveLikePublicRole

Screenshot%20from%202019-01-11%2011-30-51

So, in PublicRole.behavior.js, we go to our shouldBehaveLikePublicRole function and start analyzing it to see how it works.

Now a bit of mocha:

Async/await is a way to write asynchronous code.

  • async ensures that the function returns a promise.
  • await expression causes async function execution to pause until a Promise is resolved (fulfilled or rejected). Makes JavaScript wait until that promise settles and returns its result.
  • When resumed, the value of the await expression is that of the fulfilled Promise .

More about async/await.

Tests here are written following the best practices of TDD (Test Driven Development), but they are more like BDD (Behavior Driven Develpment), using describe() , context() , it() , specify() , before() , after() , beforeEach() , and afterEach().

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. So inside of 'should behave like public role' we will have all the tests cases of this file.

beforeEach() runs before each test case it().

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. Similarly, specify() is an alias for it() .

This guide was very helpful to understand what each one of them do. We also have mocha documentation.

1 Like

I moved to another test to find similarities and common patterns.

Now I started digging into Escrow.tests.js. We npm test the same way as before and it deploys correctly.

Screenshot%20from%202019-01-17%2011-06-58

These tests come from the shouldBehaveLikeEscrow function in Escrow.behavior.js, which we required in line 1 of Escrow.tests.js.

Screenshot%20from%202019-01-17%2011-04-04

Now in line 5, as many other tests we have an array of accounts with ether, that truffle will get from ganache.

  • How does this work? is _ the first account in ganache, primary the second and so on? And do these ... mean something.
  • Also I still don’t get how this.escrow works. Specifically why it doesn’t have capital E and what is it accessing.
1 Like

In the shouldBehaveLikeEscrow function we first set some accounts as parameters and the amount equal to 42 ether.

Parent describe( 'as an escrow'...) contains all the tests of this file. Inside of it, the describe('deposits'...) contains the first half of the tests up until line 57 and describe('withdrawals'...) the rest of them.

In deposits tests we have:

  • L15: Calls the deposit() function from Escrow.sol. Sends amount from the primary account to payee1.
  • L17: Checks the balance of the Escrow contract to see if it equals the amount just sent.
  • L19: Calls depositsOf() in the escrow contract and checks that the amount sent corresponds to payee1.

  • L23: Checks if an empty transaction to payee1 can be done. Calling deposit() from Escrow.

  • L27: Making a deposit() from an account that is not primary like payee2 should revert the transaction. shouldFail.reverting catches the revert from the promise.

shouldFail.js is required at the top of the file and should comes from Chai BDD Assertion Styles, required at test/helpers/setup.js.


  • L31: Catches the log produced once the deposit is made.
  • L32, 33, 34: Compares the log to what it should be with the help of expectEvent.inLogs.

  • L39: Makes the first deposit() of amount to payee1.
  • L40: Second deposit() of amount * 2 to payee1.
  • L42: Checks the balance of Escrow equals the two previously sent transactions = amount * 3.
  • L44: Checks that the amount * 3 sent corresponds to payee1.

  • L48: Makes the first deposit() of amount to payee1.
  • L49: Second deposit() of amount * 2 to payee2.
  • L51: Checks the balance of Escrow equals the two previously sent transactions = amount * 3.
  • L53: depositsOf() payee1 should be amount.
  • L55: depositsOf() payee2 should be amount * 2.

Then in withdrawals tests:

  • L61: Helper balanceDifference.js, reads the balance of payee1 before and after the transactions.
  • L62 & 63: First deposit()'s amount to payee1 and then withdraw()'s.
  • L64: Withdrawal of payee1 should equal amount.

First, the balance of payee1 = 0. After deposit() the balance of payee1 = amount. After withdrawal the balance of payee1 = 0 again. The difference between what first came in to payee1 and then was withdrawn should be amount, meaning that payee1 can perform withdrawals.

  • L66: Balance of Escrow should be 0 after the withdrawal.
  • L67: depositsOf payee1 in Escrow should be 0 after withdrawal.

  • L71: At this point, balance of Escrow = 0. Here is made an empty withdrawal anyway and it should work.

  • L75: Withdrawals from unautorized accounts should not be allowed.

  • L79, 80, 81, 82, 83: Makes a deposit and then withdraws, catching the log to then compare it to what it should be, with the help of expectEvent.inLogs.

Helpers - Part 1/2

From openzeppelin-solidity/test/helpers


Screenshot%20from%202019-01-25%2011-01-49

L1: pify returns a Promise wrapped version of the supplied function or module.
L3: Promisify web3.eth methods and save them into a constant.
L5 to L9: Exports promisified versions of web3.eth methods.


Screenshot%20from%202019-01-25%2011-00-38

L1: Requires ethGetBalance from web3.js.
L3: Takes an account and a promise as parameters.
L4: First, gets the balance from the account passed, and saves it.
L5: Waits the promise.
L6: Gets the new balance of the account and saves it.
L7: Calculates the difference between both balances


Screenshot%20from%202019-01-25%2011-05-27

L1: Calls web3.BigNumber and saves it in a constant.
L4 to L7: Exports related BigNumber constants.


Screenshot%20from%202019-01-25%2011-06-23

L1: Value (n) to convert.
L2: Converts any ether value (n) into wei with the toWei web3 method.


Screenshot%20from%202019-01-25%2011-08-16

  • Whats the difference between chai-bignumber and web3.BigNumber? why do we use both?

I’m gonna need help with these helpers









Helpers - Part 2/2 (WIP)

From openzeppelin-solidity/test/helpers/test


Are web3.js BigNumber and chai-bignumber two different BigNumber libraries working together?

Screenshot%20from%202019-02-04%2011-09-12


Tests balanceDifference.js, by checking it returns balance incerements and decrements.

Screenshot%20from%202019-02-04%2011-09-122

L12: balanceDifference will check the balance of receiver before and after the transaction.
L13: send 1 ether from sender to receiver.
L14: checks balanceDifference of receiver equals 1 ether.

Screenshot%20from%202019-02-04%2011-09-123

Now checks the balanceDifference of sender, so when it sends 1 ether to receiver, sender's balance decrements that exact same ether.


Tests ether.js, checking that 1 and -1 ether equal the same value in wei.


Gonna need help with these helpers too






@nventuro where do you think I should go from here? Maybe start a first draft of the guide in the test/TESTING.md? I can fork and PR so we can work from there.

I’ll need some help with the helpers :smiley: and then I’ll like to check Travis CI that I don’t yet understand. We can also discuss what are some frequent errors so we include them in the guide. Let me know if you consider something else I should cover.

WIP

Newer test helpers will come from openzeppelin-test-helpers repository from now on. I’ll be exploring how this integrates to the existing OpenZeppelin.

  • Whats the reason behind this improvement?

Hey @ianbrtt! We had a bug going on with a discourse plugin that rendered this topic empty. I hope it didn’t cause much troubles :sweat_smile:

Is this still on?

1 Like

Hey! haha I was about to ask because I was trying to use it it.
Yes, Im just a little busy just moved to BCN so I need some time to come back to it.
I’ve been learning a lot of unit testing lately so I’d like to do this one also.
I still have some of the questions from above though.
Thanks for fixing it!

1 Like