Fixed flaky unit test #5654
Fixed flaky unit test #5654
Conversation
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
| async fn login_test_guard() -> MutexGuard<'static, ()> { | ||
| LOGIN_TEST_MUTEX.get_or_init(|| Mutex::new(())).lock().await | ||
| } |
There was a problem hiding this comment.
You can use #[single(login)]
What is the underlying thing that needs to be locked? Is it an env var?
There was a problem hiding this comment.
It's the login server instance itself. It needs to bind to a specific localhost port, so if you attempt to start two instances concurrently, the second one will fail to bind.
There was a problem hiding this comment.
TIL about #[single(x)]. Nice!
There was a problem hiding this comment.
Maybe use login_port because there are other login tests that could run in parallel
2 participants
This PR fixes a test that is sporadically failing in CI.
The problem is that two unit tests (the older
login_and_cancel_chatgptand a recently addedlogin_chatgpt_includes_forced_workspace_query_param) exercise code paths that start the login server. The server binds to a hard-coded localhost port number, so attempts to start more than one server at the same time will fail. If these two tests happen to run concurrently, one of them will fail.To fix this, I've added a simple mutex. We can use this same mutex for future tests that use the same pattern.