Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: add revert error strings to assertion checks #852

Merged
merged 26 commits into from Jul 13, 2019

Conversation

Projects
None yet
4 participants
@haythem96
Copy link
Contributor

commented May 15, 2019

Fixes #816

@CLAassistant

This comment has been minimized.

Copy link

commented May 15, 2019

CLA assistant check
All committers have signed the CLA.

@coveralls

This comment has been minimized.

Copy link

commented May 15, 2019

Coverage Status

Coverage increased (+1.7%) to 98.748% when pulling a420ca8 on haythem96:revert-error-strings into 73a3702 on aragon:master.

@haythem96 haythem96 changed the title [WIP] tests: add revert error strings to assertion checks tests: add revert error strings to assertion checks May 15, 2019

@sohkai sohkai self-requested a review May 16, 2019

@sohkai
Copy link
Member

left a comment

@haythem96 Thanks for adding this! There's a small syntax issue to fix and then the tests should be able to run properly :).

Show resolved Hide resolved apps/agent/test/agent_shared.js Outdated
@sohkai

This comment has been minimized.

Copy link
Member

commented May 21, 2019

@haythem96 It looks like there are still a few tests not passing due to incorrect revert strings, do you mind fixing those?

@haythem96 haythem96 changed the title tests: add revert error strings to assertion checks [WIP] tests: add revert error strings to assertion checks May 21, 2019

@sohkai

This comment has been minimized.

Copy link
Member

commented May 29, 2019

@haythem96 Just wanted to ping, could you also merge with master? 🤗

haythem96 added some commits May 29, 2019

@haythem96 haythem96 changed the title [WIP] tests: add revert error strings to assertion checks tests: add revert error strings to assertion checks Jun 2, 2019

@haythem96

This comment has been minimized.

Copy link
Contributor Author

commented Jun 2, 2019

@haythem96 It looks like there are still a few tests not passing due to incorrect revert strings, do you mind fixing those?

Fixed.

@haythem96

This comment has been minimized.

Copy link
Contributor Author

commented Jul 5, 2019

can we merge this one ?

@sohkai

This comment has been minimized.

Copy link
Member

commented Jul 5, 2019

@haythem96 I will be looking at this soon!

sohkai added some commits Jul 8, 2019

sohkai added some commits Jul 8, 2019

@sohkai

This comment has been minimized.

Copy link
Member

commented Jul 8, 2019

@haythem96 I've pushed a few commits that add a bit of polish to the existing tests, especially those where we weren't checking for a revert case (now easy to detect thanks to these revert messages) :).

We'll get this merged within the next few days!

@haythem96

This comment has been minimized.

Copy link
Contributor Author

commented Jul 8, 2019

@haythem96 I've pushed a few commits that add a bit of polish to the existing tests, especially those where we weren't checking for a revert case (now easy to detect thanks to these revert messages) :).

We'll get this merged within the next few days!

awesome.

@sohkai

sohkai approved these changes Jul 9, 2019

@sohkai sohkai merged commit de4491f into aragon:master Jul 13, 2019

4 checks passed

Travis CI - Pull Request Build Passed
Details
WIP Ready for review
Details
coverage/coveralls Coverage increased (+1.7%) to 98.748%
Details
license/cla Contributor License Agreement is signed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.