Skip to content

Commit

Permalink
fix: fix E2E test failures in Ink contracts(use-ink#1875)
Browse files Browse the repository at this point in the history
This commit addresses the issue where e2e-tests for ink contracts
are failing even with the correct substrate-contracts-node setup.

Change made
===========
- I've Introduced `tokio::task::spawn_blocking` to offload the
  task of reading the substrate process's output to a separate
  thread since the previous approach wasn't working.
- I've used `std::sync::mpsc` channel to relay the extracted port
  from the reader thread back to the main async context.
- I've improved the `find_substrate_port_from_output` function to
  ensure continuous reading of process output, in order to prevent
  potential blockages.

Note:
=====
For details on how to test this fix, please read the PR write up.

Impact:
=======
This commit addresses issue use-ink#1875
  • Loading branch information
0xf333 committed Aug 14, 2023
1 parent 5ec7c34 commit 79debad
Showing 1 changed file with 54 additions and 31 deletions.
85 changes: 54 additions & 31 deletions crates/e2e/src/node_proc.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (C) Parity Technologies (UK) Ltd.
// Copyright 2018-2022 Parity Technologies (UK) Ltd.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand All @@ -24,12 +24,15 @@ use std::{
Read,
},
process,
sync::mpsc,
};
use subxt::{
Config,
OnlineClient,
};

use tokio::task;

/// Spawn a local substrate node for testing.
pub struct TestNodeProcess<R: Config> {
proc: process::Child,
Expand Down Expand Up @@ -134,8 +137,12 @@ where

// Wait for RPC port to be logged (it's logged to stderr):
let stderr = proc.stderr.take().unwrap();
let port = find_substrate_port_from_output(stderr);
let url = format!("ws://127.0.0.1:{port}");
let port_future =
tokio::task::spawn_blocking(move || find_substrate_port_from_output(stderr));
let port = port_future
.await
.map_err(|_| "Failed to spawn blocking task".to_string())?;
let url = format!("ws://127.0.0.1:{}", port.await);

// Connect to the node with a `subxt` client:
let client = OnlineClient::from_url(url.clone()).await;
Expand All @@ -159,37 +166,53 @@ where
}
}

// Consume a stderr reader from a spawned substrate command and
// locate the port number that is logged out to it.
fn find_substrate_port_from_output(r: impl Read + Send + 'static) -> u16 {
BufReader::new(r)
.lines()
.find_map(|line| {
let line =
line.expect("failed to obtain next line from stdout for port discovery");

// does the line contain our port (we expect this specific output from
// substrate).
let line_end = line
.rsplit_once("Listening for new connections on 127.0.0.1:")
.or_else(|| {
line.rsplit_once("Running JSON-RPC WS server: addr=127.0.0.1:")
})
.or_else(|| line.rsplit_once("Running JSON-RPC server: addr=127.0.0.1:"))
.map(|(_, port_str)| port_str)?;
/// This function is used to parse the output of the substrate node to find the port
/// For more details please refer to the PR write up https://github.com/paritytech/ink/pull/1876
async fn find_substrate_port_from_output(r: impl Read + Send + 'static) -> u16 {
// This creates a channel to communicate the port number back to the main context.
let (tx, rx) = mpsc::channel();

// Using tokio::task to spawn a new task for reading
task::spawn_blocking(move || {
let port = BufReader::new(r)
.lines()
.find_map(|line| {
let line = line
.expect("failed to obtain next line from stdout for port discovery");

// does the line contain our port (we expect this specific output from
// substrate).
let line_end = line
.rsplit_once("Listening for new connections on 127.0.0.1:")
.or_else(|| {
line.rsplit_once("Running JSON-RPC WS server: addr=127.0.0.1:")
})
.or_else(|| {
line.rsplit_once(r"Running JSON-RPC server: addr=127.0.0.1:")
})
.map(|(_, port_str)| port_str)?;

// trim non-numeric chars from the end of the port part of the line.
let port_str = line_end.trim_end_matches(|b: char| !b.is_ascii_digit());

// expect to have a number here (the chars after '127.0.0.1:') and parse
// them into a u16.
let port_num = port_str.parse().unwrap_or_else(|_| {
panic!("valid port expected for tracing line, got '{port_str}'")
});

// trim non-numeric chars from the end of the port part of the line.
let port_str = line_end.trim_end_matches(|b: char| !b.is_ascii_digit());
Some(port_num)
})
.expect("We should find a port before the reader ends");

// expect to have a number here (the chars after '127.0.0.1:') and parse them
// into a u16.
let port_num = port_str.parse().unwrap_or_else(|_| {
panic!("valid port expected for tracing line, got '{port_str}'")
});
// This sends the port via the channel
tx.send(port)
.expect("Failed to send port from the reader task");
});

Some(port_num)
})
.expect("We should find a port before the reader ends")
// This receives the port from the channel
rx.recv()
.expect("Failed to receive port from the reader task")
}

#[cfg(test)]
Expand Down

0 comments on commit 79debad

Please sign in to comment.