Replace chai.should with chai.expect.

[nventuro]

Throughout the codebase we're using chai.should, which patches all objects so that they have the should property (this is done here).

It'd be great to get rid of that, and switch to expect, which is not only simpler due to it just requiring to import a function, but also has arguably clearer semantics (and prevents ugly hacks like (await promise()).should.bla.bla.bla). This conversion should be rather straightforward, with most obj.should.equal being replaced for expect(obj).to.equal, though we'd also have to add const { expect } = require('chai') at the top of the relevant files.

[frangio]

Can you explain what is an ugly hack about (await promise()).should.bla.bla.bla?

[nventuro]

'Hack' isn't really the best name, but I dislike having to wrap the thing in parenthesis simply due to how await works (which is IMO a quirkiness of the language, which wouldn't happen if e.g. await was a function).

That said, I much prefer expect due to it being a verb, and allowing things like expectFailure, expectEvent, expectRevert, etc.

[ckshei]

tagging this - I can work on this next

[ckshei]

@nventuro I updated one of the tests and wanted to see if I was on the right track with what you were thinking. Could you look it over and let me know your thoughts?

#1723

[yopereir]

I can work on this within a few days. Just had one doubt- do we need to import bignumber for whenever we use expect(obj).to.bignumber.equal(obj); ?

[nventuro]

Awesome @RCYP! @ckshei had started working on this (#1723), but closed the issue due to a lack of time. We could merge intermediate work to a development branch, since this may end up being somewhat big.

Regarding bignumber, the plugin is automatically installed by openzeppelin-test-helpers, but we sometimes need to import BN to do things such as new BN(3). All in all, I don't think you'll need to touch any of these imports for your PR.

[frangio]

We could merge intermediate work to a development branch

Yes, let's move forward with that.