Skip to content
This repository has been archived by the owner on Jan 24, 2022. It is now read-only.

Migrates SDK CLI tests to test-environment #1288

Merged
merged 53 commits into from
Nov 27, 2019
Merged

Migrates SDK CLI tests to test-environment #1288

merged 53 commits into from
Nov 27, 2019

Conversation

ylv-io
Copy link
Contributor

@ylv-io ylv-io commented Nov 21, 2019 •
edited
Loading

This makes all tests in cli run using test-environment with Mocha and Chai instead of truffle test. On my machine, a clean run using this setup takes 3min 38s, while master takes 5min 02s. There is a massive difference on the time until the first test starts executing.

This PR is similar to a PR by @nventuro Migrate Lib tests.

Environment Truffle Test Test Environment % Gain
Local 4:35 3:38 ~21
CircleCI 5:33 4:59 ~10

Migration

  • Mocha, Chain and @openzeppelin/test-environment dependencies added.
  • Truffle contract statements replaced by describe.
  • Tests fixed.

I had to fix some tests due to differences how truffle test and test-env operate but they are rather small and trivial.
Screen Shot 2019-11-25 at 11 27 23 PM

image

PS Commit history is a mess. Sorry for that but it is possible to review only file changes.

nventuro and others added 27 commits November 20, 2019 18:56
@nventuro
Migrate lib contract tests to test-environment
e1f2654
@ylv-io
Migrate cli tests to test-env
af58643
@ylv-io
Fix accounts.test.js
a2e61c5
@ylv-io
Fix accounts.test.js
a3343f6
@abcoathup @frangio
Update gsn-dapp.adoc for Contracts 2.4 (#1287)
0e3a80a 
`GSNRecipient` doesn't implement `_preRelayedCall` and `_postRelayedCall` so our `Counter` contract needs to otherwise it is abstract and can't be `oz create`d.

Add empty implementations of `_preRelayedCall` and `_postRelayedCall`
@nventuro
Migrate src tests
a92cc79
@nventuro
Remove remnants of old setup and unused coverage
86e5373
@nventuro
No longer skip the first account
ea599c7
@nventuro
Fix ZWeb3 with HTTP provider test
cae6c6d
@nventuro
Fix ZWeb3 accounts test
b6106e0
@nventuro
Fix ZWeb3 balance test
471c413
@nventuro
Fix ZWeb3 network id tests
a45e1e7
@nventuro
Fix sendTransaction test
2fae6f4
@nventuro
Remove --exit flag, fix Loggy tests
085a1f9
@ylv-io
Remove package-lock.json
98d9cc1
@ylv-io
Fix tests, Bumpt cache version
ac06ddc
@ylv-io
Fix tests
d92fd8d
@abcoathup @spalladino
Documentation: Update linking.adoc (#1292)
510aed6 
Remove version `npm install @openzeppelin/upgrades@2.4.0` as `@openzeppelin/upgrades@2.4.0` doesn't exist and to use the latest version.
@ylv-io
Add timeout for Mocha test
429fcf5
@ylv-io
Set Mocha timeout to 5000
ac21405
@ylv-io
Fix ContractManager test by removing Foo.json after the test
bbdad15
@ylv-io
Fix ContractManager test by removing Foo.json after the test
6db985b
@nventuro
Upgrade test-enviroment to release version
12ae5db
@nventuro
Apply mocha timeout to single test
89c15a8
@nventuro
Require up-to-date yarn version
15c0bf7
@nventuro
Make Circle abort if the yarn lockfile is modified on install
6b67324
@nventuro
Update CI docker image to one with an updated yarn
4d645a2
@nventuro nventuro mentioned this pull request Nov 25, 2019
Merged
@ylv-io ylv-io marked this pull request as ready for review November 26, 2019 12:14
spalladino
spalladino reviewed Nov 26, 2019
View reviewed changes
Copy link
Contributor

@spalladino spalladino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work Igor! Left a minor suggestion, after addressed, it's good to merge for me.

packages/cli/package.json Outdated Show resolved Hide resolved
packages/cli/package.json Outdated Show resolved Hide resolved
nventuro
nventuro reviewed Nov 26, 2019
View reviewed changes
Copy link
Contributor

@nventuro nventuro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! I left a couple minor comments and questions. There's a couple duplicated changes with the current master, but I'm not sure it'll be easy to improve that, given the mess I've made of the repo's history 😅

packages/cli/package.json Outdated Show resolved Hide resolved
packages/cli/test/models/ContractManager.test.js Show resolved Hide resolved
packages/cli/test/models/ContractManager.test.js Show resolved Hide resolved
packages/cli/test/models/Dependency.test.js Outdated Show resolved Hide resolved
packages/cli/test/scripts/balance.test.js Outdated Show resolved Hide resolved
packages/cli/test/scripts/transfer.test.js Outdated Show resolved Hide resolved
@ylv-io ylv-io requested a review from nventuro November 27, 2019 07:44
nventuro
nventuro approved these changes Nov 27, 2019
View reviewed changes
Copy link
Contributor

@nventuro nventuro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a commit moving some leftover account destructurings inside the describe blocks. Great work @ylv-io!

@nventuro nventuro added the status:ready-to-merge Order mergify to merge label Nov 27, 2019
@mergify mergify bot merged commit e5d011e into master Nov 27, 2019
@mergify mergify bot deleted the feature/test-env-cli branch November 27, 2019 16:17
@nventuro nventuro mentioned this pull request Nov 28, 2019
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@spalladino spalladino spalladino left review comments

@nventuro nventuro nventuro approved these changes

Assignees
No one assigned
Labels
status:ready-to-merge Order mergify to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ylv-io @spalladino @nventuro @abcoathup