Skip to content

Commit

Permalink
test(cwl): Fix TailLogGroup test not disposing its event listeners #6267
Browse files Browse the repository at this point in the history


## Problem
Currently, the TailLogGroup tests are passing, but emitting noisy errors
in the CI logs:
```
<...>
No LiveTail session found for URI: test-region:test-log-group:all: Error: No LiveTail session found for URI: test-region:test-log-group:all
    at D:\a\aws-toolkit-vscode\aws-toolkit-vscode\packages\core\src\awsService\cloudWatchLogs\commands\tailLogGroup.ts:94:19
    at AsyncLocalStorage.run (node:async_hooks:346:14)
<...>
```

The cause for this, is that the event listener for closing the tailing
session when Editor tabs close in TailLogGroup is not being disposed of.
Disposal happens
[here](https://github.com/aws/aws-toolkit-vscode/blob/master/packages/core/src/awsService/cloudWatchLogs/commands/tailLogGroup.ts#L80).
However, currently the "mock response stream" in the test blocks
indefinitely on an [un-resolvable
promise](https://github.com/aws/aws-toolkit-vscode/blob/master/packages/core/src/test/awsService/cloudWatchLogs/commands/tailLogGroup.test.ts#L67).
So the test ends, and this finally block never triggers.

## Solution
Instead of using an un-resolving promise to keep the mock responseStream
open, use an AbortController. When the tests no longer need the
functionality of the disposables, the test can fire the AbortController.
This causes the stream to exit exceptionally, which triggers the
`finally` block in TailLogGroup and runs disposal.

This stops the event listeners from running after the test is completed,
and has eliminated the noisy logs. I have tested this by running `npm
run test` from the CLI.
  • Loading branch information
keeganirby authored Dec 17, 2024
1 parent 04a5b68 commit 4ddea68
Showing 1 changed file with 19 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,21 @@ describe('TailLogGroup', function () {
getSessionUpdateFrame(false, `${testMessage}-2`, startTimestamp + 2000),
getSessionUpdateFrame(false, `${testMessage}-3`, startTimestamp + 3000),
]
// Returns the configured update frames and then indefinitely blocks.
// Returns the configured update frames and then blocks until an AbortController is signaled.
// This keeps the stream 'open', simulating an open network stream waiting for new events.
// If the stream were to close, the event listeners in the TailLogGroup command would dispose,
// breaking the 'closes tab closes session' assertions this test makes.
async function* generator(): AsyncIterable<StartLiveTailResponseStream> {
const controller = new AbortController()
const p = new Promise((resolve, reject) => {
controller.signal.addEventListener('abort', () => {
reject()
})
})
async function* generator() {
for (const frame of updateFrames) {
yield frame
}
await new Promise(() => {})
await p
}

startLiveTailSessionSpy = sandbox
Expand All @@ -84,10 +90,14 @@ describe('TailLogGroup', function () {
})

// The mock stream doesn't 'close', causing tailLogGroup to not return. If we `await`, it will never resolve.
// Run it in the background and use waitUntil to poll its state.
// Run it in the background and use waitUntil to poll its state. Due to the test setup, we expect this to throw
// after the abortController is fired at the end of the test.
void tailLogGroup(registry, testSource, codeLensProvider, {
groupName: testLogGroup,
regionName: testRegion,
}).catch((e) => {
const err = e as Error
assert.strictEqual(err.message.startsWith('Unexpected on-stream exception while tailing session:'), true)
})
await waitUntil(async () => registry.size !== 0, { interval: 100, timeout: 1000 })

Expand Down Expand Up @@ -121,6 +131,11 @@ describe('TailLogGroup', function () {
tabs = tabs.concat(getLiveTailSessionTabsFromTabGroup(tabGroup, sessionUri!))
})
await Promise.all(tabs.map((tab) => window.tabGroups.close(tab)))

// Before the test ends, signal the abort controller, interrupting the mock response stream. This
// causes `handleSessionStream` in TailLogGroup to throw, triggering the disposables to dispose.
controller.abort()

assert.strictEqual(registry.size, 0)
assert.strictEqual(stopLiveTailSessionSpy.calledOnce, true)
})
Expand Down

0 comments on commit 4ddea68

Please sign in to comment.