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

fix: bind to correct port for e2e tests #25758

Merged
merged 2 commits into from
Jan 7, 2025
Merged

Conversation

hiltontj
Copy link
Contributor

@hiltontj hiltontj commented Jan 7, 2025

  • The TestServer now binds to port 0 so that the host will assign a random port, then scrapes the logs to get what port was assigned. This will prevent flakes in our E2E tests in CI that have been occurring from time to time.
  • Changes initial log message on serve command start for naming from OSS to Core
  • Includes the bind address along with the server startup time log message

Closes #25423

@hiltontj hiltontj added the v3 label Jan 7, 2025
@hiltontj hiltontj self-assigned this Jan 7, 2025
This also fixes up some log messages on server start for naming
@hiltontj hiltontj force-pushed the hiltontj/log-server-addr branch from 135a1ba to a9b074b Compare January 7, 2025 16:25
@hiltontj
Copy link
Contributor Author

hiltontj commented Jan 7, 2025

Just force pushed a small change to the catalog log output to match https://github.com/influxdata/influxdb_pro/pull/353

@hiltontj
Copy link
Contributor Author

hiltontj commented Jan 7, 2025

Ah, sadly it looks like CI does not work with this change. Tests are all hanging: https://app.circleci.com/pipelines/github/influxdata/influxdb/43145/workflows/24f834e5-ca0b-4520-9957-e9d13bc78107/jobs/405026

Will need to re-evaluate.

@hiltontj
Copy link
Contributor Author

hiltontj commented Jan 7, 2025

Okay, looking good again after 26a82f7

@pauldix
Copy link
Member

pauldix commented Jan 7, 2025

Odd, why does that fix it?

@hiltontj
Copy link
Contributor Author

hiltontj commented Jan 7, 2025

Odd, why does that fix it?

It was because the TEST_LOG env var's value was being used as a log filter to the spawned server process, and passing anything that prevents the INFO log containing the server address breaks the change made by this PR (it hangs waiting to see the log with the server address in it).

I think it would make sense to make TEST_LOG not do that for this PR, since we need the logs to emit INFO from the influxdb3_server crate to capture the assigned port and that is a bit of a foot gun.

@hiltontj
Copy link
Contributor Author

hiltontj commented Jan 7, 2025

I opened #25760 - I don't think fixing that is a high priority - I felt like getting this in to prevent CI flakes from blocking is more important.

@hiltontj hiltontj merged commit 3efd490 into main Jan 7, 2025
13 checks passed
@hiltontj hiltontj deleted the hiltontj/log-server-addr branch January 7, 2025 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

E2E integration tests are flaky
2 participants