-
Notifications
You must be signed in to change notification settings - Fork 1
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
Tgrade custom multitest #180
Conversation
b1b8694
to
0cd8ad6
Compare
@@ -1,13 +1,17 @@ | |||
mod gov; | |||
mod hooks; | |||
mod msg; | |||
#[cfg(feature = "multitest")] |
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.
I wonder if this
#[cfg(all(test, feature = "multitest"))]
wouldn't work better - those new multitest dependencies could be moved to [dev-dependencies]
if so.
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.
Hmm... we could improve this flag (I would prefer to do in another PR so there is a much smaller diff).
If package A imports package B in test mode, are the dev-dependencies of package B included?
I understood it would use the dev-dependencies of the package being tested (A) and the normal dependencies of the other packages. Also #[cfg(test)]
flag on the dependencies are exposed to the caller?
Again, I would prefer to merge and get #182 in (which uses these features from another package), then happy to review/approve a PR that does a less intrusive flagging of these deps
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.
I am pretty sure no - cfg(test)
is set if and only if your crate is under test. You want those modules to be exported for testing in other modules (those are helpers for other modules, not this particular one).
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.
I really appreciate this change. Just wanted to take #88, but I was missing this. I see some todos but I guess to be implemented as follow up.
@@ -1,13 +1,17 @@ | |||
mod gov; | |||
mod hooks; | |||
mod msg; | |||
#[cfg(feature = "multitest")] |
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.
I am pretty sure no - cfg(test)
is set if and only if your crate is under test. You want those modules to be exported for testing in other modules (those are helpers for other modules, not this particular one).
Closes #144
Requires CosmWasm/cw-plus#456 (and pointing Cargo.toml to a release)
Follow up (in #182):