Skip to content
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

[E2E] Allow testing with live-chain state #1949

Merged
merged 17 commits into from
Nov 16, 2023
Merged

Conversation

SkymanOne
Copy link
Contributor

@SkymanOne SkymanOne commented Oct 25, 2023

Summary

Closes #1947

  • [n] y/n | Does it introduce breaking changes?
  • [n] y/n | Is it dependent on the specific version of cargo-contract or pallet-contracts?

Allows to run E2E tests against the live chain leveraging its live state

How to test

Due to the inability to run the test on GH workflow, here are instructions on how to run the test locally:

Copied from use-ink/ink-docs#283

  1. Clone chopsticks:
git clone https://github.com/AcalaNetwork/chopsticks
  1. Modify the dev.yml config file in the repo or create one from scratch that you can reference later:
endpoint: ws://127.0.0.1:9944
mock-signature-host: true
block: 1
db: ./db.sqlite

Note: In the example above chopsticks will be mirroring up until block 1 from the substrate-contracts-node. For real world use case you would want to use a different block number and this is the place where you can configure other variables such as a sudo key. Read the chopsticks docs for more info.

Basically produce two empty blocks by submitting 2 system.remark(0) extrinsics on polkadot.js

  1. Run chopsticks locally:
npx @acala-network/chopsticks@latest --config=configs/dev.yml
  1. Go to integration-tests/e2e-call-runtime and modify the test attribute:
...
#[ink_e2e::test(node_url = "ws://127.0.0.1:8000")]
...
  1. Run cargo test -F e2e-tests

Checklist before requesting a review

  • My code follows the style guidelines of this project
  • I have added an entry to CHANGELOG.md
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • Any dependent changes have been merged and published in downstream modules

@@ -150,9 +159,6 @@ where
Err(err) => {
let err = format!("Failed to connect to node rpc at {url}: {err}");
tracing::error!("{}", err);
proc.kill().map_err(|e| {
format!("Error killing substrate process '{}': {}", proc.id(), e)
})?;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accidental removal?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep

@paritytech-cicd-pr
Copy link

paritytech-cicd-pr commented Oct 25, 2023

🦑 📈 ink! Example Contracts ‒ Changes Report 📉 🦑

These are the results when building the integration-tests/* contracts from this branch with cargo-contract and comparing them to ink! master:

Contract Upstream Size (kB) PR Size (kB) Diff (kB) Diff (%) Change
basic-contract-caller 2.967 2.967 0 0
basic-contract-caller/other-contract 1.337 1.337 0 0
call-builder-return-value 8.735 8.735 0 0
call-runtime 1.769 1.769 0 0
conditional-compilation 1.209 1.209 0 0
contract-storage 7.171 7.171 0 0
contract-terminate 1.092 1.092 0 0
contract-transfer 1.444 1.444 0 0
custom-allocator 7.428 7.428 0 0
dns 7.249 7.249 0 0
e2e-call-runtime 1.058 1.058 0 0
e2e-runtime-only-backend 1.635 1.635 0 0
erc1155 13.962 13.962 0 0
erc20 6.687 6.687 0 0
erc721 9.64 9.64 0 0
events 4.763 4.763 0 0
flipper 1.393 1.393 0 0
incrementer 1.221 1.221 0 0
lang-err-integration-tests/call-builder-delegate 2.317 2.317 0 0
lang-err-integration-tests/call-builder 4.847 4.847 0 0
lang-err-integration-tests/constructors-return-value 1.773 1.773 0 0
lang-err-integration-tests/contract-ref 4.328 4.328 0 0
lang-err-integration-tests/integration-flipper 1.571 1.571 0 0
mapping-integration-tests 7.685 7.685 0 0
mother 9.508 9.508 0 0
multi-contract-caller 5.924 5.924 0 0
multi-contract-caller/accumulator 1.095 1.095 0 0
multi-contract-caller/adder 1.669 1.669 0 0
multi-contract-caller/subber 1.689 1.689 0 0
multisig 21.471 21.471 0 0
payment-channel 5.501 5.501 0 0
sr25519-verification 0.865 0.865 0 0
static-buffer 1.405 1.405 0 0
trait-dyn-cross-contract-calls 2.466 2.466 0 0
trait-dyn-cross-contract-calls/contracts/incrementer 1.305 1.305 0 0
trait-erc20 7.063 7.063 0 0
trait-flipper 1.209 1.209 0 0
trait-incrementer 1.37 1.37 0 0
upgradeable-contracts/delegator 2.908 2.908 0 0
upgradeable-contracts/delegator/delegatee 1.369 1.369 0 0
upgradeable-contracts/set-code-hash 1.464 1.464 0 0
upgradeable-contracts/set-code-hash/updated-incrementer 1.443 1.443 0 0
wildcard-selector 2.622 2.622 0 0

Link to the run | Last update: Wed Nov 15 22:27:35 CET 2023

@codecov-commenter
Copy link

codecov-commenter commented Oct 25, 2023

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Comparison is base (64efc07) 53.30% compared to head (200e672) 53.24%.
Report is 1 commits behind head on master.

Files Patch % Lines
crates/e2e/macro/src/codegen.rs 0.00% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1949      +/-   ##
==========================================
- Coverage   53.30%   53.24%   -0.07%     
==========================================
  Files         219      219              
  Lines        6838     6844       +6     
==========================================
- Hits         3645     3644       -1     
- Misses       3193     3200       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

CHANGELOG.md Outdated
- [E2E] Allow testing with live-chain state - [#1949](https://github.com/paritytech/ink/pull/1949)
- Stabilize `call_runtime` ‒ [#1749](https://github.com/paritytech/ink/pull/1749)
- Schema generation - [#1765](https://github.com/paritytech/ink/pull/1765)
- Add `set_block_number` to off-chain test api `Engine` - [#1806](https://github.com/paritytech/ink/pull/1806)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was it intentional to add all those unrelated ones here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, just a mistake during merge

@cmichi
Copy link
Collaborator

cmichi commented Oct 27, 2023

On my branch I had added the feature to more subxt deps than you. Did your tests with Chopsticks work with your PR?

@SkymanOne
Copy link
Contributor Author

SkymanOne commented Oct 30, 2023

On my branch I had added the feature to more subxt deps than you. Did your tests with Chopsticks work with your PR?

Yes. I have run tests with WS_PORT=8000 cargo test -F e2e-tests on e2e-call-runtime and flipper. Both succeeded, and logs were produced in chopsticks node.

@SkymanOne SkymanOne requested a review from cmichi October 30, 2023 11:15
@cmichi
Copy link
Collaborator

cmichi commented Oct 30, 2023

Yes. I have run tests with WS_PORT=8000 cargo test -F e2e-tests on e2e-call-runtime and flipper. Both succeeded, and logs were produced in chopsticks node.

Did you also run them with -F live-state-test?

@SkymanOne
Copy link
Contributor Author

Yes. I have run tests with WS_PORT=8000 cargo test -F e2e-tests on e2e-call-runtime and flipper. Both succeeded, and logs were produced in chopsticks node.

Did you also run them with -F live-state-test?

Yes, as you can see in git diff in this PR, e2e-call-runtime has live-state-test enabled by default

Copy link
Collaborator

@ascjones ascjones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you give a brief explanation of the changes and the setup, and how it works to enable testing live-chain state?

crates/e2e/src/node_proc.rs Outdated Show resolved Hide resolved
crates/e2e/macro/src/config.rs Outdated Show resolved Hide resolved
crates/e2e/Cargo.toml Outdated Show resolved Hide resolved
Copy link
Collaborator

@ascjones ascjones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

crates/e2e/macro/src/codegen.rs Outdated Show resolved Hide resolved
crates/e2e/macro/src/codegen.rs Outdated Show resolved Hide resolved
crates/e2e/macro/src/config.rs Outdated Show resolved Hide resolved
@SkymanOne SkymanOne merged commit fa6dd8e into master Nov 16, 2023
42 checks passed
@SkymanOne SkymanOne deleted the gn/e2e-live-state-test branch November 16, 2023 01:50
@SkymanOne SkymanOne mentioned this pull request Nov 30, 2023
@SkymanOne SkymanOne mentioned this pull request Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow E2E testing with live-chain state
5 participants