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

Cover Assembly #176

Open
cgewecke opened this issue Dec 28, 2017 · 3 comments
Open

Cover Assembly #176

cgewecke opened this issue Dec 28, 2017 · 3 comments

Comments

@cgewecke
Copy link
Member

No description provided.

@nventuro
Copy link

Would this mean that if statements in assembly would have coverage measured on both branches? We've recently encountered this limitation in OpenZeppelin/openzeppelin-contracts#1644

@cgewecke
Copy link
Member Author

cgewecke commented Aug 27, 2019

@nventuro Yes, we would cover those branches.

But I think OZ 1744 should "work" with current coverage because assembly statements are skipped over anyway. I cloned Augusto's fork, swapped in solidity-coverage 0.6.4 , ran his tests with the latest testrpc-sc (equiv. to ganache-cli 6.5.1) and everything for create2 seemed ok....

Screen Shot 2019-08-26 at 4 48 05 PM

Would you be open to a PR into 1744 to fix coverage there by updating the deps? We are running Zeppelin in CI for every PR - it should be pretty stable for you. (Also sincere apologies for the inconveniences caused by the gap in maintenance here last year.)

@nventuro Out of curiosity, I notice you've switched to using codecov which I think we've had difficulty showing branch coverage with because they do their own JS parsing to mark what they call 'partial' hits. (We're covering .sol's and they don't recognize those). Your coverage is perfect - it's hard to tell if that's still an issue there.

Were you having trouble with coveralls or do you prefer codecov for other reasons? (We use it in this repo fwiw).

@nventuro
Copy link

nventuro commented Sep 5, 2019

But I think OZ 1744 should "work" with current coverage because assembly statements are skipped over anyway.

I suspected this to be the case, which is why I wondered what it'd take for solidity-coverage to be able to handle it.

(Also sincere apologies for the inconveniences caused by the gap in maintenance here last year.)

No worries! I was saddened by your departure (and not being able to meet at conferences!), but we managed to get by with what you had left us :) And the plans going forward look great!

@nventuro Out of curiosity, I notice you've switched to using codecov which I think we've had difficulty showing branch coverage with because they do their own JS parsing to mark what they call 'partial' hits. (We're covering .sol's and they don't recognize those). Your coverage is perfect - it's hard to tell if that's still an issue there.

Were you having trouble with coveralls or do you prefer codecov for other reasons? (We use it in this repo fwiw).

It wasn't me who made the change, but it seems to be related to us switching to CircleCI. That said, I don't have strong preferences either way. It is true though that coveralls frequently (about once per one or two months) suddenly stopped reporting coverage on PRs and needed some reconfiguring

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants