Skip to content

chore: Restrict network-dependent helper to integration tests #4250

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

Merged
merged 3 commits into from
Jun 18, 2025

Conversation

callpraths
Copy link
Contributor

@callpraths callpraths commented Jun 13, 2025

Last step towards dropping network dependency from unit-tests.

This commit

  • migrates the last unit-test to use in-memory network connections.
  • restricts the GetFreePorts() helper with explicit network dependency to the integration package to discourage use from unit-tests in the future.

Closes #3923

@callpraths callpraths requested review from aleks-p, marcsanmi and a team as code owners June 13, 2025 05:21
@callpraths callpraths force-pushed the b3923/restrict-get-free-ports-pr branch from 9fbda8a to 4feca5b Compare June 13, 2025 05:23
@callpraths
Copy link
Contributor Author

callpraths commented Jun 16, 2025

Hiya,

Please let me know if this looks good for a review.


I started looking at #4127 next. @kolesnikovae does that sound like a useful next step?

Aside: is there a forum for such discussions, other than randomly asking on unrelated issues/PRs?
(:

Copy link
Contributor

@simonswine simonswine left a comment

Choose a reason for hiding this comment

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

LGTM

@callpraths Thank you for your work on this.

Generally we hang out in #pyroscope for more synchronous discussions. For all async a GitHub issue might be best and sharing it in that channel if we fail to get back to you in time.

@simonswine simonswine merged commit 77bd4b9 into grafana:main Jun 18, 2025
24 checks passed
@callpraths callpraths deleted the b3923/restrict-get-free-ports-pr branch June 18, 2025 17:29
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 this pull request may close these issues.

Flaky Test_Concurrency in the query backend client package
2 participants