impl: test sub cmd with support for --manifest-path (#280) #283

chiefbiiko · 2021-05-27T06:54:17Z

drafts cargo contract test [FLAGS] [--manifest-path]

in this first impl stdout is uncolored

let me know if you care much for the colors

cla-bot-2021 · 2021-05-27T06:54:27Z

User @chiefbiiko, please sign the CLA here.

Robbepop

What precisely is the win of having cargo contract test over just using cargo test as it can be done right now?

cmichi · 2021-06-17T12:22:39Z

@Robbepop At the moment cargo contract --help shows the sub-command as:

test     Test the smart contract off-chain

If one invokes cargo contract test currently this results in an unimplemented! panic though, which is why people were irritated and started creating issues about it in this repository.

So we could either remove the sub-command or just hand over to cargo test. After talking to @ascjones we concluded that it's probably best to just spawn a sub-process with cargo test until we have further ideas what to do with the sub-command (#280).

What's your opinion?

Robbepop · 2021-06-17T12:33:29Z

@Robbepop At the moment cargo contract --help shows the sub-command as:
test     Test the smart contract off-chain
If one invokes cargo contract test currently this results in an unimplemented! panic though, which is why people were irritated and started creating issues about it in this repository.

So we could either remove the sub-command or just hand over to cargo test. After talking to @ascjones we concluded that it's probably best to just spawn a sub-process with cargo test until we have further ideas what to do with the sub-command (#280).

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

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.

cmichi · 2021-06-17T12:49:28Z

Cargo.toml

@@ -3,7 +3,7 @@ members = [".", "metadata"]

 [package]
 name = "cargo-contract"
-version = "0.12.1"
+version = "0.13.0"


Suggested change

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 :-).

cmichi · 2021-06-17T12:50:07Z

CHANGELOG.md

+## [0.13.0] - 2021-05-27
+


Suggested change

## [0.13.0] - 2021-05-27

cmichi · 2021-06-17T12:50:30Z

README.md

@@ -87,6 +87,9 @@ toolchain of a directory by executing `rustup override set nightly` in it.
 Checks that the code builds as WebAssembly. This command does not output any `<name>.contract`
 artifact to the `target/` directory.

+##### `cargo contract test`
+
+Runs test suites defined for a smart contract - offchain.


Suggested change

Runs test suites defined for a smart contract - offchain.

Runs test suites defined for a smart contract off-chain.

cmichi · 2021-06-17T12:50:47Z

.gitlab-ci.yml

@@ -99,6 +99,7 @@ test-new-project-template:

    - 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


Great that you thought of adding it here!

cmichi · 2021-06-17T12:51:04Z

Cargo.lock

@@ -487,7 +487,7 @@ dependencies = [

 [[package]]
 name = "cargo-contract"
-version = "0.12.1"
+version = "0.13.0"


Suggested change

version = "0.13.0"

version = "0.12.1"

cmichi · 2021-06-17T12:51:36Z

src/cmd/test.rs

+#[derive(Debug, StructOpt)]
+#[structopt(name = "test")]
+pub struct TestCommand {
+    /// Path to the Cargo.toml of the contract to test


Suggested change

/// Path to the Cargo.toml of the contract to test

/// Path to the `Cargo.toml` of the contract to test

cmichi · 2021-06-17T12:51:45Z

src/cmd/test.rs

+use std::{convert::TryFrom, path::PathBuf};
+use structopt::StructOpt;
+
+/// Executes smart-contract tests offchain by delegating to `cargo test`.


Suggested change

/// Executes smart-contract tests offchain by delegating to `cargo test`.

/// Executes smart-contract tests off-chain by delegating to `cargo test`.

cmichi · 2021-06-17T12:52:08Z

src/cmd/test.rs

+    }
+}
+
+/// Executes cargo +nightly test.


Suggested change

/// Executes cargo +nightly test.

/// Executes `cargo +nightly test`.

cmichi · 2021-06-17T13:01:09Z

src/cmd/test.rs

+        with_new_contract_project(|manifest_path| {
+            let res = super::execute(&manifest_path, Verbosity::Default).expect("test failed");
+
+            assert!(res.stdout.len() > 0);


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.

cmichi · 2021-06-17T13:01:28Z

CHANGELOG.md

+## [0.13.0] - 2021-05-27
+
+### Added
+- Convenient offchain testing through `cargo contract test`


Suggested change

- Convenient offchain testing through `cargo contract test`

- Convenient off-chain testing through `cargo contract test`

chiefbiiko · 2021-06-21T15:35:35Z

thanks for the review
i have included your suggestions now

cmichi

Thanks for the PR!

chiefbiiko force-pushed the tüv branch from 263544d to c789f9d Compare May 27, 2021 07:40

Robbepop reviewed Jun 17, 2021

View reviewed changes

cmichi requested changes Jun 17, 2021

View reviewed changes

chiefbiiko force-pushed the tüv branch from c789f9d to 3d3fe2f Compare June 21, 2021 15:16

impl: test sub cmd with support for --manifest-path (use-ink#280)

7c8d0ce

chiefbiiko force-pushed the tüv branch from 3d3fe2f to 7c8d0ce Compare June 21, 2021 15:33

cmichi approved these changes Jul 3, 2021

View reviewed changes

cmichi merged commit 78d4d4e into use-ink:master Jul 3, 2021

cmichi mentioned this pull request Jul 3, 2021

Implement cargo contract test #280

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

impl: test sub cmd with support for --manifest-path (#280) #283

impl: test sub cmd with support for --manifest-path (#280) #283

chiefbiiko commented May 27, 2021

cla-bot-2021 bot commented May 27, 2021

Robbepop left a comment

cmichi commented Jun 17, 2021 •

edited

Loading

Robbepop commented Jun 17, 2021

cmichi left a comment

cmichi Jun 17, 2021

cmichi Jun 17, 2021

cmichi Jun 17, 2021

cmichi Jun 17, 2021

cmichi Jun 17, 2021

cmichi Jun 17, 2021

cmichi Jun 17, 2021

cmichi Jun 17, 2021

cmichi Jun 17, 2021

cmichi Jun 17, 2021

chiefbiiko commented Jun 21, 2021

cmichi left a comment

	Runs test suites defined for a smart contract - offchain.
	Runs test suites defined for a smart contract off-chain.

	/// Path to the Cargo.toml of the contract to test
	/// Path to the `Cargo.toml` of the contract to test

	/// 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.
	/// Executes `cargo +nightly test`.

	- Convenient offchain testing through `cargo contract test`
	- Convenient off-chain testing through `cargo contract test`

impl: test sub cmd with support for --manifest-path (#280) #283

impl: test sub cmd with support for --manifest-path (#280) #283

Conversation

chiefbiiko commented May 27, 2021

cla-bot-2021 bot commented May 27, 2021

Robbepop left a comment

Choose a reason for hiding this comment

cmichi commented Jun 17, 2021 • edited Loading

Robbepop commented Jun 17, 2021

cmichi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chiefbiiko commented Jun 21, 2021

cmichi left a comment

Choose a reason for hiding this comment

cmichi commented Jun 17, 2021 •

edited

Loading