Skip to content

Commit 13f1d3b

Browse files
committed
Fix flaky tests on TransactionExecutor suite
The TransactionExecutor tests were mocking the global setTimeout and clearTimeout. This mocks were conflicting with other calls to this api in the browser tests. So, the calls to the set/clear were not being the expected in some situations. Replace the global mock for injecting the functions resolve this issue in a less intrusive way.
1 parent ee4baf7 commit 13f1d3b

File tree

3 files changed

+206
-206
lines changed

3 files changed

+206
-206
lines changed

packages/core/src/internal/transaction-executor.ts

+17-3
Original file line numberDiff line numberDiff line change
@@ -33,19 +33,30 @@ type Resolve<T> = (value: T | PromiseLike<T>) => void
3333
type Reject = (value: any) => void
3434
type Timeout = ReturnType<typeof setTimeout>
3535

36+
interface Dependencies {
37+
setTimeout: typeof setTimeout
38+
clearTimeout: typeof clearTimeout
39+
}
40+
3641
export class TransactionExecutor {
3742
private readonly _maxRetryTimeMs: number
3843
private readonly _initialRetryDelayMs: number
3944
private readonly _multiplier: number
4045
private readonly _jitterFactor: number
4146
private _inFlightTimeoutIds: Timeout[]
47+
private readonly _setTimeout: typeof setTimeout
48+
private readonly _clearTimeout: typeof clearTimeout
4249
public pipelineBegin: boolean
4350

4451
constructor (
4552
maxRetryTimeMs?: number | null,
4653
initialRetryDelayMs?: number,
4754
multiplier?: number,
48-
jitterFactor?: number
55+
jitterFactor?: number,
56+
dependencies: Dependencies = {
57+
setTimeout,
58+
clearTimeout
59+
}
4960
) {
5061
this._maxRetryTimeMs = _valueOrDefault(
5162
maxRetryTimeMs,
@@ -64,6 +75,9 @@ export class TransactionExecutor {
6475
DEFAULT_RETRY_DELAY_JITTER_FACTOR
6576
)
6677

78+
this._setTimeout = dependencies.setTimeout
79+
this._clearTimeout = dependencies.clearTimeout
80+
6781
this._inFlightTimeoutIds = []
6882
this.pipelineBegin = false
6983

@@ -99,7 +113,7 @@ export class TransactionExecutor {
99113

100114
close (): void {
101115
// cancel all existing timeouts to prevent further retries
102-
this._inFlightTimeoutIds.forEach(timeoutId => clearTimeout(timeoutId))
116+
this._inFlightTimeoutIds.forEach(timeoutId => this._clearTimeout(timeoutId))
103117
this._inFlightTimeoutIds = []
104118
}
105119

@@ -119,7 +133,7 @@ export class TransactionExecutor {
119133

120134
return new Promise<T>((resolve, reject) => {
121135
const nextRetryTime = this._computeDelayWithJitter(retryDelayMs)
122-
const timeoutId = setTimeout(() => {
136+
const timeoutId = this._setTimeout(() => {
123137
// filter out this timeoutId when time has come and function is being executed
124138
this._inFlightTimeoutIds = this._inFlightTimeoutIds.filter(
125139
id => id !== timeoutId

packages/neo4j-driver/test/internal/timers-util.js

+22-30
Original file line numberDiff line numberDiff line change
@@ -15,47 +15,37 @@
1515
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
1616
* See the License for the specific language governing permissions and
1717
* limitations under the License.
18+
*/
19+
20+
/**
21+
* This is a liter mock which only creates mocked functions to work
22+
* as timeouts.
1823
*/
19-
class SetTimeoutMock {
24+
class TimeoutsMock {
2025
constructor () {
21-
this._clearState()
26+
this.clearState()
27+
// bind it to be used as standalone functions
28+
this.setTimeout = this.setTimeout.bind(this)
29+
this.clearTimeout = this.clearTimeout.bind(this)
2230
}
2331

24-
install () {
25-
this._originalSetTimeout = global.setTimeout
26-
global.setTimeout = (code, delay) => {
27-
if (!this._paused) {
28-
code()
29-
this.invocationDelays.push(delay)
30-
}
31-
return this._timeoutIdCounter++
32-
}
33-
34-
this._originalClearTimeout = global.clearTimeout
35-
global.clearTimeout = id => {
36-
this.clearedTimeouts.push(id)
32+
setTimeout (code, delay) {
33+
if (!this._paused) {
34+
code()
35+
this.invocationDelays.push(delay)
3736
}
37+
return this._timeoutIdCounter++
38+
}
3839

39-
return this
40+
clearTimeout (id) {
41+
this.clearedTimeouts.push(id)
4042
}
4143

4244
pause () {
4345
this._paused = true
4446
}
4547

46-
uninstall () {
47-
global.setTimeout = this._originalSetTimeout
48-
global.clearTimeout = this._originalClearTimeout
49-
this._clearState()
50-
}
51-
52-
setTimeoutOriginal (code, delay) {
53-
return this._originalSetTimeout.call(null, code, delay)
54-
}
55-
56-
_clearState () {
57-
this._originalSetTimeout = null
58-
this._originalClearTimeout = null
48+
clearState () {
5949
this._paused = false
6050
this._timeoutIdCounter = 0
6151

@@ -64,4 +54,6 @@ class SetTimeoutMock {
6454
}
6555
}
6656

67-
export const setTimeoutMock = new SetTimeoutMock()
57+
export {
58+
TimeoutsMock
59+
}

0 commit comments

Comments
 (0)