fix(spanner): fix flakiness in testCreateSessionDeadlineExceeded#12944
fix(spanner): fix flakiness in testCreateSessionDeadlineExceeded#12944sakthivelmanii merged 1 commit intomainfrom
Conversation
The test `testCreateSessionDeadlineExceeded` in `MultiplexedSessionDatabaseClientMockServerTest` was flaky because it used a non-sticky exception for simulated `DEADLINE_EXCEEDED` on `CreateSession`. This exception could be consumed by background session creation tasks before the test could verify it. This change uses `ofStickyException` to ensure the exception persists until explicitly cleared by `mockSpanner.removeAllExecutionTimes()`, making the test robust against concurrent background activity. Fixes: #12890
sakthivelmanii
force-pushed
the
fix-12890
branch
from
d2ec7a5 to
f08f207
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the testCreateSessionDeadlineExceeded test to use a sticky exception when simulating RPC failures, which helps address flakiness. Feedback was provided regarding the need to ensure proper cleanup of the mock server state to prevent the sticky exception from leaking into and affecting subsequent tests.
| // Simulate a problem with the CreateSession RPC making it slow. | ||
| mockSpanner.setCreateSessionExecutionTime( | ||
| SimulatedExecutionTime.ofException(Status.DEADLINE_EXCEEDED.asRuntimeException())); | ||
| SimulatedExecutionTime.ofStickyException(Status.DEADLINE_EXCEEDED.asRuntimeException())); |
There was a problem hiding this comment.
Using ofStickyException ensures that the exception persists across multiple calls, which is necessary to fix the flakiness caused by background tasks. However, because sticky exceptions do not clear themselves after being triggered, it is critical to ensure that mockSpanner.removeAllExecutionTimes() is called in a finally block or a teardown method (e.g., @After) to prevent this failure from leaking into subsequent tests that share the same mock server instance.
References
- The implementation must be exception-safe to prevent resource leaks or state contamination, meaning all opened resources or mock states should be closed or cleared even if exceptions occur.
There was a problem hiding this comment.
We are already calling mockSpanner.removeAllExecutionTimes() below which should be suffice.
2 participants
The test
testCreateSessionDeadlineExceededinMultiplexedSessionDatabaseClientMockServerTestwas flaky because it used a non-sticky exception for simulatedDEADLINE_EXCEEDEDonCreateSession. This exception could be consumed by background session creation tasks before the test could verify it.This change uses
ofStickyExceptionto ensure the exception persists until explicitly cleared bymockSpanner.removeAllExecutionTimes(), making the test robust against concurrent background activity.Fixes: #12890