Factor substrate node runner into separate crate #913
Conversation
…me and integration-tests
|
When #907 merges I'll remove the |
testing/substrate-runner/src/lib.rs
Outdated
| .arg("--dev") | ||
| .stdout(process::Stdio::piped()) | ||
| .stderr(process::Stdio::piped()) | ||
| .arg("--port=0") |
There was a problem hiding this comment.
why do we care about the libp2p port? :)
There was a problem hiding this comment.
I think the reasoning is to prevent any overlap with the port used for other substrate instances we spawn; I guess Substrate will pick a random port for us already if the one we give isn't any good, but probably just getting the kernel to pick an OK port straight off is a touch better anyway?
testing/substrate-runner/src/lib.rs
Outdated
| .stdout(process::Stdio::piped()) | ||
| .stderr(process::Stdio::piped()) | ||
| .arg("--port=0") | ||
| .arg("--rpc-port=0") |
There was a problem hiding this comment.
You could omit the port for both --rpc--port, --ws-port here because substrate would fallback to 0 anyway, this way it would be backward-compatible :)
There was a problem hiding this comment.
Hmm but ah true; backward compat is a good reason not to have them; I'll remove :)
| // Consume a stderr reader from a spawned substrate command and | ||
| // locate the port number that is logged out to it. | ||
| fn try_find_substrate_port_from_output(r: impl Read + Send + 'static) -> Option<u16> { | ||
| BufReader::new(r).lines().take(50).find_map(|line| { |
There was a problem hiding this comment.
maybe increase the number of a lines a bit to future proof?
| BufReader::new(r).lines().take(50).find_map(|line| { | |
| BufReader::new(r).lines().take(1024).find_map(|line| { |
There was a problem hiding this comment.
I guess I wanted it to fail reasonably quickly if for some reason it wasn't picking up the port; I started at 100 but I think I counted right now that it's something like 25 lines to get the port, so 50 was my "safe" number; maybe 100 to compromise a bit? It should never take that long for the port to be logged I'd hope :)
Co-authored-by: Alexandru Vasile <[email protected]>
Co-authored-by: Niklas Adolfsson <[email protected]>
So that we can re-use the logic for starting a test substrate node up in both the
test-runtimecrate (to hopefully avoid the retry errors we've been seeing a bit lately) and theintegration-testscrate which also spawns a node.I added support for @niklasad1's upcoming PR in terms of capturing stdout but not in terms of handling the invalid flag; perhaps I should! (but until then, this should still be a bit of an improvement I hope)
I also removed the Subxt dep on the
build.rsoftest-runtimein the hopes that it would mean it is triggered much less often (ie not every time Subxt is changed), but let's see aye!Closes #909