Skip to content

run local faucet with non-overlapping ports behind test/debug gate#7736

Merged
alexpyattaev merged 1 commit intoanza-xyz:masterfrom
puhtaytow:faucet-non-overlapping-ports-with-nextest
Sep 2, 2025
Merged

run local faucet with non-overlapping ports behind test/debug gate#7736
alexpyattaev merged 1 commit intoanza-xyz:masterfrom
puhtaytow:faucet-non-overlapping-ports-with-nextest

Conversation

@puhtaytow
Copy link
Copy Markdown

@puhtaytow puhtaytow commented Aug 27, 2025

Problem

Tests which engage ports binding are still randomly flaky.
While looking for source of the evil, i found that test_bench_tps_test_validator calls run_local_faucet which takes a port randomly.

This isn't correct behaviour as the randomly picked port might overlap with one of ports allocated thru unique_port_range_for_tests

Summary of Changes

  • Now, the run_local_faucet, Takes non-overlapping port for debug_assertions. Otherwise defaults to random port as before.

Comment thread faucet/src/faucet.rs Outdated
/// For integration tests. Listens on a random open port and reports the port to Sender.
/// If the NEXTEST environment variable is set, a non-overlapping port is allocated instead.
pub fn run_local_faucet(faucet_keypair: Keypair, per_time_cap: Option<u64>) -> SocketAddr {
let port: u16 = match std::env::var("NEXTEST") {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a better gate to use is debug_assertions. Using NEXTEST here will still allow this to break if running under plain cargo test.

@puhtaytow puhtaytow force-pushed the faucet-non-overlapping-ports-with-nextest branch 2 times, most recently from f229853 to 085a912 Compare August 27, 2025 16:03
@puhtaytow puhtaytow changed the title run local faucet with non-overlapping ports under nextests run local faucet with non-overlapping ports / behind test/debug gate Aug 27, 2025
@puhtaytow puhtaytow changed the title run local faucet with non-overlapping ports / behind test/debug gate run local faucet with non-overlapping ports behind test/debug gate Aug 27, 2025
@alexpyattaev alexpyattaev added the CI Pull Request is ready to enter CI label Aug 27, 2025
@anza-team anza-team removed the CI Pull Request is ready to enter CI label Aug 27, 2025
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Aug 27, 2025

Codecov Report

❌ Patch coverage is 0% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.1%. Comparing base (0c3bf69) to head (51c94f3).

Additional details and impacted files
@@            Coverage Diff            @@
##           master    #7736     +/-   ##
=========================================
- Coverage    83.1%    83.1%   -0.1%     
=========================================
  Files         808      808             
  Lines      356469   356479     +10     
=========================================
- Hits       296288   296270     -18     
- Misses      60181    60209     +28     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@alexpyattaev alexpyattaev added the CI Pull Request is ready to enter CI label Sep 2, 2025
@anza-team anza-team removed the CI Pull Request is ready to enter CI label Sep 2, 2025
@alexpyattaev alexpyattaev force-pushed the faucet-non-overlapping-ports-with-nextest branch from 085a912 to 51c94f3 Compare September 2, 2025 10:29
Copy link
Copy Markdown

@alexpyattaev alexpyattaev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good

@puhtaytow
Copy link
Copy Markdown
Author

This looks good

Thank you 🙏

@gregcusack gregcusack self-requested a review September 2, 2025 19:40
Copy link
Copy Markdown

@gregcusack gregcusack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! thank you!

@alexpyattaev alexpyattaev merged commit 0417f3d into anza-xyz:master Sep 2, 2025
54 checks passed
@gregcusack
Copy link
Copy Markdown

looks like this PR created this warning when compiling master with --release:

$ cargo build --release
warning: gen-syscall-list@3.1.0: (not a warning) Generating ../../platform-tools-sdk/sbf/syscalls.txt from ../src/lib.rs
warning: unused import: `solana_net_utils::sockets::unique_port_range_for_tests`
  --> faucet/src/faucet.rs:18:5
   |
18 |     solana_net_utils::sockets::unique_port_range_for_tests,
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: `#[warn(unused_imports)]` on by default

any chance you could fix if you have time please? @puhtaytow

@alexpyattaev
Copy link
Copy Markdown

looks like this PR created this warning when compiling master with --release:

We need to check for warnings in release builds in CI. It is by far not the first time these things slip by, impossible to catch them by hand every time.

Comment thread faucet/src/faucet.rs
@puhtaytow
Copy link
Copy Markdown
Author

puhtaytow commented Sep 3, 2025

looks like this PR created this warning when compiling master with --release:

$ cargo build --release
warning: gen-syscall-list@3.1.0: (not a warning) Generating ../../platform-tools-sdk/sbf/syscalls.txt from ../src/lib.rs
warning: unused import: `solana_net_utils::sockets::unique_port_range_for_tests`
  --> faucet/src/faucet.rs:18:5
   |
18 |     solana_net_utils::sockets::unique_port_range_for_tests,
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: `#[warn(unused_imports)]` on by default

any chance you could fix if you have time please? @puhtaytow

@gregcusack Sorry, I totally didn't think about it here is the fix #7847

steviez added a commit that referenced this pull request Sep 4, 2025
This reverts commit 0417f3d.

Switching behavior based on whether debug assertions are enabled or not
seems fragile, so we should take another approach to solving the
problem this PR aimed to address
@puhtaytow puhtaytow deleted the faucet-non-overlapping-ports-with-nextest branch September 4, 2025 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants