Skip to content

Conversation

@ross-rosario
Copy link
Contributor

...besides that, I've marked cases where expected and actual revert mismatch with // @FIXME. I'll remove those comments once this PR has been reviewed.

@ross-rosario
Copy link
Contributor Author

I'm looking into these failed tests...

@coveralls
Copy link

coveralls commented Jul 18, 2019

Coverage Status

Coverage increased (+0.06%) to 96.24% when pulling a683165 on chore/reason-strings-cont into 3ab0a11 on dev-3.1.0.

@ross-rosario
Copy link
Contributor Author

Ready for review.

@ross-rosario ross-rosario changed the title Unit testing: add explicit revert reason checks for the tests from b_* to h_* Unit testing: add explicit revert reason checks for unit tests Jul 19, 2019
@ross-rosario ross-rosario changed the title Unit testing: add explicit revert reason checks for unit tests Unit testing: add explicit revert reason checks Jul 19, 2019
@ross-rosario
Copy link
Contributor Author

Now it's really ready for review. Please check assertions marked with // @FIXME and let me know how to better tackle them.

Copy link
Contributor

@maxsam4 maxsam4 left a comment

Choose a reason for hiding this comment

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

Reviewed till m

@satyamakgec
Copy link
Contributor

@maxsam4 to review and fixes. It is better to review done by one person so another will take a look into other PR.

Copy link
Contributor

@maxsam4 maxsam4 left a comment

Choose a reason for hiding this comment

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

I have fixed most test cases that needed fixing and added "Fixed" review comment. I have left some more review comments that should answer your question. I have removed the "fixme" from code for the respective fixes I made.

@satyamakgec satyamakgec merged commit bd09190 into dev-3.1.0 Jul 22, 2019
@ross-rosario ross-rosario deleted the chore/reason-strings-cont branch July 23, 2019 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants