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

Test-runtime: Run Substrate node on port 0 for more reliable execution #909

Closed
jsdw opened this issue Apr 13, 2023 · 2 comments · Fixed by #913
Closed

Test-runtime: Run Substrate node on port 0 for more reliable execution #909

jsdw opened this issue Apr 13, 2023 · 2 comments · Fixed by #913
Assignees

Comments

@jsdw
Copy link
Collaborator

jsdw commented Apr 13, 2023

We've ran into issues a couple of times now (eg #908) where a nightly build fails because it tried to connect to a Substrate node 6 times without success.

The way that we start said Substrate node up is by selecting a port to use like so:

fn next_open_port() -> Option<u16> {
    match TcpListener::bind(("127.0.0.1", 0)) {
        Ok(listener) => {
            if let Ok(address) = listener.local_addr() {
                Some(address.port())
            } else {
                None
            }
        }
        Err(_) => None,
    }
}

And then telling Substrate to start using that port.

I wonder whether the port isn't freed up quickly enough sometimes, and so Substrate picks another random port to start on or something like that, leading to an inability to connect to it?

In any case, in our tests we take a different approach; we start Substrate with a --ws-port=0, and then parse the stdout until we find the actual port that it picked. This may be a more reliable approach to take, since we'll always try connecting on the actual port that it used, and won't run the issue of providing a port that ends up being in use. So, let's update our build code to do the same thing.

@niklasad1
Copy link
Member

It's probably a good idea but just a reminder I have opened paritytech/substrate#13384 which will break things :P

@jsdw
Copy link
Collaborator Author

jsdw commented Apr 13, 2023

It's probably a good idea but just a reminder I have opened paritytech/substrate#13384 which will break things :P

Ooh noted, thanks! I guess the stdout logs will be the same so it should be a trivial change to fix the commandline opt change from --ws-port to --rpc-port where needed! There will be some breaking though since we can't support both old and newer Substrate binaries at the same time I think

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 a pull request may close this issue.

2 participants