less flaky test

This commit is contained in:
Andrew Stanton-Nurse 2018-05-03 17:55:25 -07:00
parent 6f77eaca13
commit 1282fde338
2 changed files with 15 additions and 7 deletions

View File

@ -8,6 +8,7 @@ import { ConsoleLogger } from "../src/Utils";
import { TestHttpClient } from "./TestHttpClient"; import { TestHttpClient } from "./TestHttpClient";
import { asyncit as it, PromiseSource, delay } from "./Utils"; import { asyncit as it, PromiseSource, delay } from "./Utils";
import { HttpError, TimeoutError } from "../src/Errors"; import { HttpError, TimeoutError } from "../src/Errors";
import { AbortSignal } from "../src/AbortController";
describe("LongPollingTransport", () => { describe("LongPollingTransport", () => {
it("shuts down poll after timeout even if server doesn't shut it down on receiving the DELETE", async () => { it("shuts down poll after timeout even if server doesn't shut it down on receiving the DELETE", async () => {
@ -59,7 +60,6 @@ describe("LongPollingTransport", () => {
deleteReceived.resolve(); deleteReceived.resolve();
return new HttpResponse(202); return new HttpResponse(202);
}); });
const logMessages: string[] = [];
const transport = new LongPollingTransport(client, null, NullLogger.instance, false, 100); const transport = new LongPollingTransport(client, null, NullLogger.instance, false, 100);
await transport.connect("http://example.com", TransferFormat.Text); await transport.connect("http://example.com", TransferFormat.Text);
@ -115,9 +115,8 @@ describe("LongPollingTransport", () => {
// fake timers! // fake timers!
await delay(150); await delay(150);
expect(logMessages) // The pollAbort token should be left unaborted because we shut down gracefully.
.not expect(transport.pollAborted).toBe(false);
.toContain("(LongPolling transport) server did not terminate after DELETE request, canceling poll.");
}); });
} }
}); });

View File

@ -10,6 +10,7 @@ import { Arg, getDataDetail, sendMessage } from "./Utils";
const SHUTDOWN_TIMEOUT = 5 * 1000; const SHUTDOWN_TIMEOUT = 5 * 1000;
// Not exported from 'index', this type is internal.
export class LongPollingTransport implements ITransport { export class LongPollingTransport implements ITransport {
private readonly httpClient: HttpClient; private readonly httpClient: HttpClient;
private readonly accessTokenFactory: () => string | Promise<string>; private readonly accessTokenFactory: () => string | Promise<string>;
@ -22,6 +23,12 @@ export class LongPollingTransport implements ITransport {
private shutdownTimer: any; // We use 'any' because this is an object in NodeJS. But it still gets passed to clearTimeout, so it doesn't really matter private shutdownTimer: any; // We use 'any' because this is an object in NodeJS. But it still gets passed to clearTimeout, so it doesn't really matter
private shutdownTimeout: number; private shutdownTimeout: number;
private running: boolean; private running: boolean;
private stopped: boolean;
// This is an internal type, not exported from 'index' so this is really just internal.
public get pollAborted() {
return this.pollAbort.aborted;
}
constructor(httpClient: HttpClient, accessTokenFactory: () => string | Promise<string>, logger: ILogger, logMessageContent: boolean, shutdownTimeout?: number) { constructor(httpClient: HttpClient, accessTokenFactory: () => string | Promise<string>, logger: ILogger, logMessageContent: boolean, shutdownTimeout?: number) {
this.httpClient = httpClient; this.httpClient = httpClient;
@ -144,8 +151,8 @@ export class LongPollingTransport implements ITransport {
} }
} }
} finally { } finally {
// Trigger the poll aborted token so we don't set the shutdown timer // Indicate that we've stopped so the shutdown timer doesn't get registered.
this.pollAbort.abort(); this.stopped = true;
// Clean up the shutdown timer if it was registered // Clean up the shutdown timer if it was registered
if (this.shutdownTimer) { if (this.shutdownTimer) {
@ -185,9 +192,11 @@ export class LongPollingTransport implements ITransport {
this.logger.log(LogLevel.Trace, "(LongPolling transport) DELETE request accepted."); this.logger.log(LogLevel.Trace, "(LongPolling transport) DELETE request accepted.");
} finally { } finally {
// Abort the poll after the shutdown timeout if the server doesn't stop the poll. // Abort the poll after the shutdown timeout if the server doesn't stop the poll.
if (!this.pollAbort.aborted) { if (!this.stopped) {
this.shutdownTimer = setTimeout(() => { this.shutdownTimer = setTimeout(() => {
this.logger.log(LogLevel.Warning, "(LongPolling transport) server did not terminate after DELETE request, canceling poll."); this.logger.log(LogLevel.Warning, "(LongPolling transport) server did not terminate after DELETE request, canceling poll.");
// Abort any outstanding poll
this.pollAbort.abort(); this.pollAbort.abort();
}, this.shutdownTimeout); }, this.shutdownTimeout);
} }