Skip to content

Commit

Permalink
Improve racy subscribeToMore tests.
Browse files Browse the repository at this point in the history
The use of setTimeout in tests is often an invitation to race conditions
and timing-sensitive outcomes.

This raciness became particularly problematic thanks to changes (between
Node 10 and Node 12) in the timing of Promise callbacks with respect to
setTimeout callbacks.

These changes also leave all tests passing when cherry-picked onto master,
so I'm confident I am *not* changing the tests just to fix my PR #6221, at
the expense of backwards compatibility.
  • Loading branch information
benjamn committed May 5, 2020
1 parent 4fe4dd5 commit 761624d
Showing 1 changed file with 41 additions and 35 deletions.
76 changes: 41 additions & 35 deletions src/__tests__/subscribeToMore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,23 @@ describe('subscribeToMore', () => {
link,
});

const obsHandle = client.watchQuery<typeof req1['result']['data']>({
type TData = typeof req1['result']['data'];
const obsHandle = client.watchQuery<TData>({
query,
});

const sub = obsHandle.subscribe({
next(queryResult) {
next(queryResult: TData) {
latestResult = queryResult;
counter++;
if (++counter === 3) {
sub.unsubscribe();
expect(latestResult).toEqual({
data: { entry: { value: 'Amanda Liu' } },
loading: false,
networkStatus: 7,
});
resolve();
}
},
});

Expand All @@ -94,20 +103,15 @@ describe('subscribeToMore', () => {
},
});

setTimeout(() => {
sub.unsubscribe();
expect(counter).toBe(3);
expect(stripSymbols(latestResult)).toEqual({
data: { entry: { value: 'Amanda Liu' } },
loading: false,
networkStatus: 7,
});
resolve();
}, 15);

for (let i = 0; i < 2; i++) {
wSLink.simulateResult(results[i]);
let i = 0;
function simulate() {
const result = results[i++];
if (result) {
wSLink.simulateResult(result);
setTimeout(simulate, 10);
}
}
simulate();
});

itAsync('calls error callback on error', (resolve, reject) => {
Expand Down Expand Up @@ -351,18 +355,25 @@ describe('subscribeToMore', () => {
link,
});

const obsHandle = client.watchQuery<
typeof typedReq['result']['data'],
typeof typedReq['request']['variables']
>({
type TData = typeof typedReq['result']['data'];
type TVars = typeof typedReq['request']['variables'];
const obsHandle = client.watchQuery<TData, TVars>({
query,
variables: { someNumber: 1 },
});

const sub = obsHandle.subscribe({
next(queryResult) {
next(queryResult: TData) {
latestResult = queryResult;
counter++;
if (++counter === 3) {
sub.unsubscribe();
expect(latestResult).toEqual({
data: { entry: { value: 'Amanda Liu' } },
loading: false,
networkStatus: 7,
});
resolve();
}
},
});

Expand All @@ -380,19 +391,14 @@ describe('subscribeToMore', () => {
},
});

setTimeout(() => {
sub.unsubscribe();
expect(counter).toBe(3);
expect(stripSymbols(latestResult)).toEqual({
data: { entry: { value: 'Amanda Liu' } },
loading: false,
networkStatus: 7,
});
resolve();
}, 15);

for (let i = 0; i < 2; i++) {
wSLink.simulateResult(results[i]);
let i = 0;
function simulate() {
const result = results[i++];
if (result) {
wSLink.simulateResult(result);
setTimeout(simulate, 10);
}
}
simulate();
});
});

0 comments on commit 761624d

Please sign in to comment.