impl: test sub cmd with support for --manifest-path (#280)#283
Hidden character warning
Conversation
|
User @chiefbiiko, please sign the CLA here. |
Robbepop
left a comment
Robbepop
left a comment
There was a problem hiding this comment.
What precisely is the win of having cargo contract test over just using cargo test as it can be done right now?
|
@Robbepop At the moment If one invokes So we could either remove the sub-command or just hand over to What's your opinion? |
I currently do not see a major reason but it might help against confusion. So yeah let's do it the proposed way. |
cmichi
left a comment
cmichi
left a comment
There was a problem hiding this comment.
Thanks for the PR, it looks good!
I've got just some minor things. We use the writing off-chain everywhere, hence the suggestion of keeping it consistent.
| [package] | ||
| name = "cargo-contract" | ||
| version = "0.12.1" | ||
| version = "0.13.0" |
There was a problem hiding this comment.
| version = "0.13.0" | |
| version = "0.12.1" |
Good that you thought of this, but we plan on having another change in the next release as well, so let's wait for that :-).
| ## [0.13.0] - 2021-05-27 | ||
|
|
There was a problem hiding this comment.
| ## [0.13.0] - 2021-05-27 |
|
|
||
| ##### `cargo contract test` | ||
|
|
||
| Runs test suites defined for a smart contract - offchain. |
There was a problem hiding this comment.
| Runs test suites defined for a smart contract - offchain. | |
| Runs test suites defined for a smart contract off-chain. |
|
|
||
| - cargo run --all-features -- contract build --manifest-path new_project/Cargo.toml | ||
| - cargo run --all-features -- contract check --manifest-path new_project/Cargo.toml | ||
| - cargo run --all-features -- contract test --manifest-path new_project/Cargo.toml |
There was a problem hiding this comment.
Great that you thought of adding it here!
| [[package]] | ||
| name = "cargo-contract" | ||
| version = "0.12.1" | ||
| version = "0.13.0" |
There was a problem hiding this comment.
| version = "0.13.0" | |
| version = "0.12.1" |
| #[derive(Debug, StructOpt)] | ||
| #[structopt(name = "test")] | ||
| pub struct TestCommand { | ||
| /// Path to the Cargo.toml of the contract to test |
There was a problem hiding this comment.
| /// Path to the Cargo.toml of the contract to test | |
| /// Path to the `Cargo.toml` of the contract to test |
| use std::{convert::TryFrom, path::PathBuf}; | ||
| use structopt::StructOpt; | ||
|
|
||
| /// Executes smart-contract tests offchain by delegating to `cargo test`. |
There was a problem hiding this comment.
| /// Executes smart-contract tests offchain by delegating to `cargo test`. | |
| /// Executes smart-contract tests off-chain by delegating to `cargo test`. |
| } | ||
| } | ||
|
|
||
| /// Executes cargo +nightly test. |
There was a problem hiding this comment.
| /// Executes cargo +nightly test. | |
| /// Executes `cargo +nightly test`. |
| with_new_contract_project(|manifest_path| { | ||
| let res = super::execute(&manifest_path, Verbosity::Default).expect("test failed"); | ||
|
|
||
| assert!(res.stdout.len() > 0); |
There was a problem hiding this comment.
Could you add another assertions which checks for something like test result: ok. 2 passed; 0 failed; 0 ignored?
This way we can then be sure that there was no other unrelated failure the cause of stdout.len() > 0.
| ## [0.13.0] - 2021-05-27 | ||
|
|
||
| ### Added | ||
| - Convenient offchain testing through `cargo contract test` |
There was a problem hiding this comment.
| - Convenient offchain testing through `cargo contract test` | |
| - Convenient off-chain testing through `cargo contract test` |
|
thanks for the review |
3 participants
drafts
cargo contract test [FLAGS] [--manifest-path]in this first impl stdout is uncolored
let me know if you care much for the colors