Skip to content

Conversation

@robacourt
Copy link
Contributor

These tests can occasionally fail with:

 Assertion failed, no matching message after 5000ms
     The process mailbox is empty.
     code: assert_receive :publish_finished

The race condition occurs if the TestSubscriber receives the :finish_processing_message message before it receives the publisher call, in which case it swallows the message since handle_info is not implemented on TestSubscriber. A GenServer would log that a message had been received and not handled, but as use GenServer was omitted from the TestSubscriber the message was swallowed silently.

This PR:

  • Adds use GenServer to the TestSubscriber as this is best practice (the race condition would have been obvious had it been included in the first place)
  • Waits for :message_received from each of the subscribers before sending :finish_processing_message. This eliminates the race condition.
  • Renames event to message for consistency

Credit to ChatGPT which worked out what the race condition was. It's solution was terrible, but the hard part was knowing what the race condition was, so that was super helpful.

@robacourt robacourt marked this pull request as ready for review June 25, 2025 12:58
@codecov
Copy link

codecov bot commented Jun 25, 2025

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
1785 1 1784 1460
View the full list of 1 ❄️ flaky tests
test/integration.test.ts > HTTP Sync > should correctly select columns for initial sync and updates

Flake rate in main: 22.50% (Passed 31 times, Failed 9 times)

Stack Traces | 5.01s run time
Error: Test timed out in 5000ms.
If this is a long-running test, pass a timeout value as the last argument or configure it globally with "testTimeout".

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

Copy link
Contributor

@msfstef msfstef left a comment

Choose a reason for hiding this comment

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

Very nice, that's what I'm talking about! I wonder if we have any other such "mock" processes that could benefit from implementing GenServer

@robacourt
Copy link
Contributor Author

Very nice, that's what I'm talking about! I wonder if we have any other such "mock" processes that could benefit from implementing GenServer

Good shout! I just checked and we don't currently. It would be nice if a static analysis tool like credo could ensure things like this. I might look into that at some point.

@robacourt robacourt merged commit 18e9a7a into main Jun 25, 2025
48 of 52 checks passed
@robacourt robacourt deleted the rob/fix-flakey-publisher-test branch June 25, 2025 14:50
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.

3 participants