-
Notifications
You must be signed in to change notification settings - Fork 262
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
Ensure metadata is in sync with running node during tests #333
Changes from 13 commits
7b2b25c
fd278c3
b35d4f0
8322f75
389fb8a
9fd95f2
dd2c9a3
5851a7a
69856f0
b912bb7
128c40e
f1cf775
07af1ce
30b5902
61d2044
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
[package] | ||
name = "test-runtime" | ||
version = "0.1.0" | ||
edition = "2021" | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A |
||
[dependencies] | ||
subxt = { path = ".." } | ||
sp-runtime = { package = "sp-runtime", git = "https://github.com/paritytech/substrate/", branch = "master" } | ||
codec = { package = "parity-scale-codec", version = "2", default-features = false, features = ["derive", "full", "bit-vec"] } | ||
|
||
[build-dependencies] | ||
subxt = { path = ".." } | ||
async-std = { version = "1.9.0", features = ["attributes"] } | ||
sp-core = { package = "sp-core", git = "https://github.com/paritytech/substrate/", branch = "master" } |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,10 @@ | ||||||
# test-runtime | ||||||
|
||||||
The logic for this crate exists mainly in the `build.rs` file. | ||||||
|
||||||
At compile time, this crate will: | ||||||
- Spin up a local `substrate` binary (set the `SUBSTRATE_NODE_PATH` env var to point to a custom binary, otehrwise it'll look for `substrate` on your PATH). | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
- Obtain metadata from this node. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a thought: perhaps the substrate node should have a sub-command There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a good idea There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume that the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||
- Export the metadata and a `node_runtime` module which has been annotated using the `subxt` proc macro and is based off the above metadata. | ||||||
|
||||||
The reason for doing this is that our integration tests (which also spin up a Substrate node) can then use the generated `subxt` types from the exact node being tested against, so that we don't have to worry about metadata getting out of sync with the binary under test. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,144 @@ | ||
// Copyright 2019-2021 Parity Technologies (UK) Ltd. | ||
// This file is part of subxt. | ||
// | ||
// subxt is free software: you can redistribute it and/or modify | ||
// it under the terms of the GNU General Public License as published by | ||
// the Free Software Foundation, either version 3 of the License, or | ||
// (at your option) any later version. | ||
// | ||
// subxt is distributed in the hope that it will be useful, | ||
// but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
// GNU General Public License for more details. | ||
// | ||
// You should have received a copy of the GNU General Public License | ||
// along with subxt. If not, see <http://www.gnu.org/licenses/>. | ||
|
||
use std::{ | ||
env, | ||
fs, | ||
net::TcpListener, | ||
path::Path, | ||
process::Command, | ||
sync::atomic::{ | ||
AtomicU16, | ||
Ordering, | ||
}, | ||
thread, | ||
time, | ||
}; | ||
|
||
static SUBSTRATE_BIN_ENV_VAR: &str = "SUBSTRATE_NODE_PATH"; | ||
|
||
#[async_std::main] | ||
async fn main() { | ||
// Select substrate binary to run based on env var. | ||
let substrate_bin = | ||
env::var(SUBSTRATE_BIN_ENV_VAR).unwrap_or_else(|_| "substrate".to_owned()); | ||
|
||
// Run binary. | ||
let port = next_open_port() | ||
.expect("Cannot spawn substrate: no available ports in the given port range"); | ||
let cmd = Command::new(&substrate_bin) | ||
.arg("--dev") | ||
.arg("--tmp") | ||
.arg(format!("--rpc-port={}", port)) | ||
.spawn(); | ||
let mut cmd = match cmd { | ||
Ok(cmd) => cmd, | ||
Err(e) => { | ||
panic!("Cannot spawn substrate command '{}': {}", substrate_bin, e) | ||
} | ||
}; | ||
|
||
// Download metadata from binary; retry until successful, or a limit is hit. | ||
let metadata_bytes: sp_core::Bytes = { | ||
const MAX_RETRIES: usize = 20; | ||
let mut retries = 0; | ||
let mut wait_secs = 1; | ||
loop { | ||
if retries >= MAX_RETRIES { | ||
panic!("Cannot connect to substrate node after {} retries", retries); | ||
} | ||
let res = | ||
subxt::RpcClient::try_from_url(&format!("http://localhost:{}", port)) | ||
.await | ||
.unwrap() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This might panic if it can't connect There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah yes, I think I took it to be if the url was malformed, and was surprised to then have to move it after the await.. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, I did have a perusal through the code, and it's all OK as is (if the url is WS, it tries to establish a connection (hence the await), but if it's HTTP it won't try to connect and will only error if not an http(s) URL). I'll change to an expect to document this! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, I think we should kill |
||
.request("state_getMetadata", &[]) | ||
.await; | ||
match res { | ||
Ok(res) => { | ||
let _ = cmd.kill(); | ||
break res | ||
} | ||
_ => { | ||
thread::sleep(time::Duration::from_secs(wait_secs)); | ||
retries += 1; | ||
wait_secs += 1; | ||
} | ||
}; | ||
} | ||
}; | ||
|
||
// Save metadata to a file: | ||
let out_dir = env::var_os("OUT_DIR").unwrap(); | ||
let metadata_path = Path::new(&out_dir).join("metadata.scale"); | ||
fs::write(&metadata_path, &metadata_bytes.0).expect("Couldn't write metadata output"); | ||
|
||
// Write out our expression to generate the runtime API to a file. Ideally, we'd just write this code | ||
// in lib.rs, but we must pass a string literal (and not `concat!(..)`) as an arg to runtime_metadata_path, | ||
// and so we need to spit it out here and include it verbatim instead. | ||
let runtime_api_contents = format!( | ||
r#" | ||
#[subxt::subxt( | ||
runtime_metadata_path = "{}", | ||
generated_type_derives = "Debug, Eq, PartialEq" | ||
)] | ||
pub mod node_runtime {{ | ||
#[subxt(substitute_type = "sp_arithmetic::per_things::Perbill")] | ||
use sp_runtime::Perbill; | ||
}} | ||
"#, | ||
metadata_path | ||
.to_str() | ||
.expect("Path to metadata should be stringifiable") | ||
); | ||
let runtime_path = Path::new(&out_dir).join("runtime.rs"); | ||
fs::write(&runtime_path, runtime_api_contents) | ||
.expect("Couldn't write runtime rust output"); | ||
|
||
// Re-build if we point to a different substrate binary: | ||
println!("cargo:rerun-if-env-changed={}", SUBSTRATE_BIN_ENV_VAR); | ||
// Re-build if this file changes: | ||
println!("cargo:rerun-if-changed=build.rs"); | ||
} | ||
|
||
/// Returns the next open port, or None if no port found in range. | ||
fn next_open_port() -> Option<u16> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder whether we could extract Just an idea, don't need to do it here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had a very similar thought doing this! I didn't really fancy the amount of churn in this PR, but I can imagine it would be useful! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will be worth doing, separate PR probably better - must remember to remove the duplication here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why aren't we just binding to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is copied from code I wrote in haste, so most likely not the best way to do it. If that works then let's do it. Edit, we can probably use that or similar since this code was originally designed for all the tests which spin up in parallel so are all attempting to claim ports There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do like the In the other tests, we need to grab 3 ports by the looks of things, so it is a little trickier! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But yes, in this build script, it's probably a little more robust to scan for a port than rely on any specific stdout from substrate on the matter (but if there's a better way to see which port was used, that'd be great!). |
||
/// The start of the port range to scan. | ||
const START_PORT: u16 = 9900; | ||
/// The end of the port range to scan. | ||
const END_PORT: u16 = 10000; | ||
/// The maximum number of ports to scan before giving up. | ||
const MAX_PORTS: u16 = 1000; | ||
|
||
let next_port: AtomicU16 = AtomicU16::new(START_PORT); | ||
let mut ports_scanned = 0u16; | ||
loop { | ||
// Loop back from the beginning if needed | ||
let _ = next_port.compare_exchange( | ||
END_PORT, | ||
START_PORT, | ||
Ordering::SeqCst, | ||
Ordering::SeqCst, | ||
); | ||
let next = next_port.fetch_add(1, Ordering::SeqCst); | ||
if TcpListener::bind(("0.0.0.0", next)).is_ok() { | ||
return Some(next) | ||
} | ||
ports_scanned += 1; | ||
if ports_scanned == MAX_PORTS { | ||
return None | ||
} | ||
} | ||
} |
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 don't much like the repetition of this curl thing, but the alternate way of sharing artifacts between jobs works out to be about as verbose...
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.
:/