Skip to content

resolve issue with local faucet overlapping ports in tests / new api#7909

Merged
alexpyattaev merged 17 commits intoanza-xyz:masterfrom
puhtaytow:faucet-non-overlapping-1
Sep 10, 2025
Merged

resolve issue with local faucet overlapping ports in tests / new api#7909
alexpyattaev merged 17 commits intoanza-xyz:masterfrom
puhtaytow:faucet-non-overlapping-1

Conversation

@puhtaytow
Copy link
Copy Markdown

@puhtaytow puhtaytow commented Sep 5, 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

Originally resolved #7736 then reverted and the alternate PR is here, as suggested in #7736 (comment)

Summary of Changes

  • New API in faucet has been introduced:

    • run_local_faucet_with_unique_port_for_tests
    • run_local_faucet_with_config
    • run_local_faucet_for_tests
    • LocalFaucetConfig
  • Benchmarks has been migrated:

    • solana-accounts-cluster-bench
    • solana-bench-tps
  • Tests has been migrated:

    • solana-faucet
    • solana-dos
    • solana-cli

@puhtaytow puhtaytow marked this pull request as draft September 5, 2025 13:51
@mergify mergify Bot requested a review from a team September 5, 2025 13:52
@puhtaytow puhtaytow force-pushed the faucet-non-overlapping-1 branch from 13381c0 to 00a59d0 Compare September 5, 2025 14:17
@puhtaytow puhtaytow marked this pull request as ready for review September 5, 2025 14:28
@alexpyattaev alexpyattaev added the CI Pull Request is ready to enter CI label Sep 5, 2025
@anza-team anza-team removed the CI Pull Request is ready to enter CI label Sep 5, 2025
@alexpyattaev alexpyattaev added the CI Pull Request is ready to enter CI label Sep 5, 2025
@anza-team anza-team removed the CI Pull Request is ready to enter CI label Sep 5, 2025
@puhtaytow
Copy link
Copy Markdown
Author

puhtaytow commented Sep 5, 2025

I see that CI failed due to...

`dev-context-only-utils` must not be used as normal dependencies, but is by "([crate]: [dependency])":
    solana-cli: solana-faucet
🚨 Error: The command exited with status 1

which didn't happen by running locally /ci/test-checks.sh and more importantly, not sure what to do with it now.

--- edit
maybe cli/src/test_utils should be moved somewhere else?

@kskalski
Copy link
Copy Markdown

kskalski commented Sep 5, 2025

I guess you will see error when running
scripts/check-dev-context-only-utils.sh tree

Did you try adding separate dependency and dev-dependency, where only dev-dependency has feature DCOU set? I.e. this line in cli/Cargo.toml seems to be a problem:

solana-faucet = { workspace = true, features = ["dev-context-only-utils"] }

Comment thread cli/Cargo.toml
Comment thread cli/src/test_utils.rs Outdated
@puhtaytow puhtaytow marked this pull request as draft September 5, 2025 17:58
@puhtaytow puhtaytow force-pushed the faucet-non-overlapping-1 branch 10 times, most recently from e19223c to 38e61c3 Compare September 6, 2025 14:21
@puhtaytow puhtaytow marked this pull request as ready for review September 6, 2025 14:48
@puhtaytow puhtaytow force-pushed the faucet-non-overlapping-1 branch from 38e61c3 to 25411ad Compare September 6, 2025 14:50
@puhtaytow puhtaytow force-pushed the faucet-non-overlapping-1 branch 3 times, most recently from 382509b to 566c4cc Compare September 9, 2025 11:08
@alexpyattaev alexpyattaev added the CI Pull Request is ready to enter CI label Sep 9, 2025
@anza-team anza-team removed the CI Pull Request is ready to enter CI label Sep 9, 2025
@puhtaytow puhtaytow force-pushed the faucet-non-overlapping-1 branch 3 times, most recently from 5f77258 to 96d431b Compare September 9, 2025 11:38
@alexpyattaev alexpyattaev added the CI Pull Request is ready to enter CI label Sep 9, 2025
@anza-team anza-team removed the CI Pull Request is ready to enter CI label Sep 9, 2025
Copy link
Copy Markdown

@steviez steviez left a comment

Choose a reason for hiding this comment

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

Just one last minor thing and think we're good then

Comment thread cli/Cargo.toml Outdated
Comment thread cli/src/test_utils.rs Outdated
Comment thread faucet/src/faucet.rs
@puhtaytow puhtaytow force-pushed the faucet-non-overlapping-1 branch from 96d431b to e556d4c Compare September 9, 2025 16:22
@steviez steviez added the CI Pull Request is ready to enter CI label Sep 9, 2025
@anza-team anza-team removed the CI Pull Request is ready to enter CI label Sep 9, 2025
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.

LGTM

@alexpyattaev alexpyattaev merged commit cb0d00e into anza-xyz:master Sep 10, 2025
56 checks passed
@puhtaytow
Copy link
Copy Markdown
Author

Thank you guys! :)

@puhtaytow puhtaytow deleted the faucet-non-overlapping-1 branch September 10, 2025 09:11
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