-
Notifications
You must be signed in to change notification settings - Fork 55
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
improve(tests): silence unsafeAllowDelegatecall
warning in tests
#820
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: bennett <[email protected]>
Signed-off-by: bennett <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OOC will this potentially silence other warnings?
It would if there were other warnings thrown in those fixtures. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixtures by design are only used in tests so this is nice. Is there way to target specific warnings?
Signed-off-by: bennett <[email protected]>
I don't see a way to do this. I was referencing this post: https://forum.openzeppelin.com/t/logging-verbose-during-local-tests-with-upgradeable-contracts/4633, which seemed to have similar issues as ours. I changed this to just instead silence warnings for our contract tests only. The upside is that we should have a clean output for our tests, but the downside is that any warnings emitted from Edit: I'm reverting this change and going to back to what I originally had since if we don't suppress the warnings in the fixtures, then we will get spammed in the other repositories, e.g. the relayer. |
Signed-off-by: bennett <[email protected]>
Signed-off-by: bennett <[email protected]>
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
Signed-off-by: bennett <[email protected]>
Report too large to display inline |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious about Nick's comment. This is what I was probing at. If we can't and this looks good I will approve so that I'm not a blocker
Applies the function introduced by OZ in this PR: OpenZeppelin/openzeppelin-upgrades#228. This is only used on two test fixtures, so we still should see warnings when we deploy.