-
Notifications
You must be signed in to change notification settings - Fork 119
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
ink::test attributes in new template #190
Conversation
The problem here ist that the implication of The tests for which you added the attribute in this PR don't require a simulated chain environment, as they only unit test isolated contract logic. I acknowledge that it can be misleading to have the similarly named
|
Hm, maybe if the current design is confusing we should consider renaming it or even change semantics. |
@cmichi yeah I guess understand not adding it in the name of efficiency. What I would say though is that it's not totally clear to me what tests need to be ran in the simulated environment, and which do not. If things stay the same, it's probably safe to say that everyone who writes a smart contract is going to have a test blow up in their face and have to figure out why, which could be fine 🤷 Maybe a better solution would be to improve the error message you get when the test does need to be ran in the simulated environment:
|
The simplest solution so far is: Write #[test]. Compile it. If it explodes use #[ink(test)]. Profit. |
@Robbepop I have no problem with this issue being closed if the consensus is that this is just part of the learning curve |
We are aware that this is hard to learn and are very happy about your feedback. Ideally we use syntax that does not need to be taught or which can be taught in an autonomous or automated fashion, e.g. by some intelligent compiler errors or warnings. |
I like these two ideas: (1) Rename
or
(2) Improve the compiler error on uninitialized context to give hints about using the ink! test attribute. |
Co-authored-by: Michael Müller <mich@elmueller.net>
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.
Thanks for bringing this discussion up, @trace-andreason !
We're currently migrating all public facing contract tests to use #[ink::test]
.
Just adding the ink prefix to the test attributes in the new template. Pretty minor, but if you don't notice and you copy the
#[test]
attribute for a more complex test, your test will break in a confusing way.