Repository navigation
test(server): hold dead-endpoint ports so parallel tests cannot bind them #3505
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
Changes from all commits
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,34 @@ | ||
| //! A loopback address whose connections are refused for the life of the test | ||
| //! process. Included with `#[path]` by each test that needs a dead endpoint. | ||
| //! | ||
| //! Binding `127.0.0.1:0`, reading the port and dropping the listener frees the | ||
| //! port for any parallel test to bind before the dial, which then gets a | ||
| //! response instead of a refusal. Here the port stays bound by a socket that | ||
| //! never listens: a connect to it is refused at once, and with neither | ||
| //! `SO_REUSEADDR` nor `SO_REUSEPORT` set no other socket can bind it while the | ||
| //! process lives. | ||
|
|
||
| #![allow(dead_code, clippy::expect_used)] | ||
|
|
||
| use std::net::SocketAddr; | ||
| use std::sync::OnceLock; | ||
|
|
||
| use tokio::net::TcpSocket; | ||
|
|
||
| static REFUSED: OnceLock<(TcpSocket, SocketAddr)> = OnceLock::new(); | ||
|
|
||
| /// The process-wide refused address. | ||
| pub fn refused_addr() -> SocketAddr { | ||
|
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. nit: The sweep for other copies of this bug found a site with the same assertion shape that stays unfixed, and the description defers it as a "follow-up to file" with no issue number, so there is nothing tracking it. |
||
| REFUSED | ||
| .get_or_init(|| { | ||
| let socket = TcpSocket::new_v4().expect("refused socket"); | ||
| socket.set_reuseaddr(false).expect("clear SO_REUSEADDR"); | ||
| socket.set_reuseport(false).expect("clear SO_REUSEPORT"); | ||
| socket | ||
| .bind(SocketAddr::from(([127, 0, 0, 1], 0))) | ||
| .expect("bind refused socket"); | ||
| let addr = socket.local_addr().expect("refused local addr"); | ||
| (socket, addr) | ||
| }) | ||
| .1 | ||
| } | ||
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.
nit: The helper is a
#[path]-included module underservices/ravel-server/tests/support/, so only ravel-server's own test tree can use it. The description lists eight further sites with this exact bug, and five of them are in other crates (ravel-object-store,ravel-sql,ravel-promql-difftest,ravel-bench,ravel-tracing-export), which means each follow-up has to copy this file rather than call it.Failure: no runtime failure; the cost lands on the follow-ups, which either duplicate the module or get skipped.
Evidence:
crates/ravel-test-supportalready exists as the workspace's dev-only shared test scaffolding (crates/ravel-test-support/src/lib.rs:1-11), ispublish = false, andservices/ravel-server/Cargo.toml:342already dev-depends on it. Puttingrefused_addr()there turns every follow-up into a dev-dependency line. It would needtokio(orsocket2) added to that crate's[dependencies], which today isparking_lotonly.