Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(cwl): Add LiveTailSession unit tests for start and stopLiveTail #6023

Open
wants to merge 1 commit into
base: feature/cwltail
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,8 @@ import * as vscode from 'vscode'

import assert from 'assert'
import { clearDocument, closeSession, tailLogGroup } from '../../../../awsService/cloudWatchLogs/commands/tailLogGroup'
import { LiveTailSessionLogEvent, StartLiveTailResponseStream } from '@aws-sdk/client-cloudwatch-logs'
import { LiveTailSessionRegistry } from '../../../../awsService/cloudWatchLogs/registry/liveTailSessionRegistry'
import { LiveTailSession } from '../../../../awsService/cloudWatchLogs/registry/liveTailSession'
import { asyncGenerator } from '../../../../shared/utilities/collectionUtils'
import {
TailLogGroupWizard,
TailLogGroupWizardResponse,
Expand All @@ -21,6 +19,7 @@ import { getTestWindow } from '../../../shared/vscode/window'
import { CloudWatchLogsSettings, uriToKey } from '../../../../awsService/cloudWatchLogs/cloudWatchLogsUtils'
import { installFakeClock } from '../../../testUtil'
import { DefaultAwsContext, ToolkitError } from '../../../../shared'
import { getTestResponseStream } from '../registry/liveTailSession.test'

describe('TailLogGroup', function () {
const testLogGroup = 'test-log-group'
Expand Down Expand Up @@ -68,7 +67,7 @@ describe('TailLogGroup', function () {
startLiveTailSessionSpy = sandbox
.stub(LiveTailSession.prototype, 'startLiveTailSession')
.callsFake(async function () {
return getTestResponseStream([
return getTestResponseStream(testLogGroup, [
{
message: testMessage,
timestamp: 876830400000,
Expand Down Expand Up @@ -195,27 +194,4 @@ describe('TailLogGroup', function () {
},
}
}

//Creates a test response stream. Each log event provided will be its own "frame" of the input stream.
function getTestResponseStream(logEvents: LiveTailSessionLogEvent[]): AsyncIterable<StartLiveTailResponseStream> {
const sessionStartFrame: StartLiveTailResponseStream = {
sessionStart: {
logGroupIdentifiers: [testLogGroup],
},
sessionUpdate: undefined,
}

const updateFrames: StartLiveTailResponseStream[] = logEvents.map((event) => {
return {
sessionUpdate: {
sessionMetadata: {
sampled: false,
},
sessionResults: [event],
},
}
})

return asyncGenerator([sessionStartFrame, ...updateFrames])
}
})
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,35 @@
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/
import * as sinon from 'sinon'
import assert from 'assert'
import { LiveTailSession } from '../../../../awsService/cloudWatchLogs/registry/liveTailSession'
import { StartLiveTailCommand } from '@aws-sdk/client-cloudwatch-logs'
import {
CloudWatchLogsClient,
LiveTailSessionLogEvent,
StartLiveTailCommand,
StartLiveTailResponse,
StartLiveTailResponseStream,
} from '@aws-sdk/client-cloudwatch-logs'
import { LogStreamFilterResponse } from '../../../../awsService/cloudWatchLogs/wizard/liveTailLogStreamSubmenu'
import { asyncGenerator } from '../../../../shared/utilities/collectionUtils'

describe('LiveTailSession', async function () {
const testLogGroupArn = 'arn:aws:test-log-group'
const testRegion = 'test-region'
const testFilter = 'test-filter'
const testAwsCredentials = {} as any as AWS.Credentials

let sandbox: sinon.SinonSandbox

beforeEach(function () {
sandbox = sinon.createSandbox()
})

afterEach(function () {
sandbox.restore()
})

it('builds StartLiveTailCommand: no stream Filter, no event filter.', function () {
const session = buildLiveTailSession({ type: 'all' }, undefined)
assert.deepStrictEqual(
Expand Down Expand Up @@ -65,6 +83,36 @@ describe('LiveTailSession', async function () {
)
})

it('startLiveTail sends command to CWL client with AbortSignal', async function () {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a lot of code to test very little. The test description sounds like it's testing that a parameter is passed somewhere.

const sendCommandMock = sandbox.stub(CloudWatchLogsClient.prototype, 'send').callsFake(function () {
return buildLiveTailCommandOutput(testLogGroupArn)
})
const session = buildLiveTailSession({ type: 'all' }, undefined)
const stream = await session.startLiveTailSession()

assert.strictEqual(sendCommandMock.calledOnce, true)
assert.strictEqual(
sendCommandMock.calledWith(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of mocking many things just to test that parameters are sent, can you stub fake responses and let the code actually work on the response. that tests the full codepath, instead of just tiny parts of it (and requiring lots of test code)

sinon.match.any,
sinon.match({
abortSignal: sinon.match.defined,
})
),
true
)
assert.strictEqual(session.isAborted, false)
assert.deepEqual(stream, buildLiveTailCommandOutput(testLogGroupArn).responseStream)
})

it('stopLiveTail fires AbortSignal and destroys CWL client', function () {
const destroyClientSpy = sandbox.spy(CloudWatchLogsClient.prototype, 'destroy')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does this need a spy? test the actual state. the client can expose some (readonly) state if needed.

do not use mocks and spies except as a last resort. it looks like it's the first resort in this tests, instead of the last resort.

const session = buildLiveTailSession({ type: 'all' }, undefined)
session.stopLiveTailSession()

assert.strictEqual(session.isAborted, true)
assert.strictEqual(destroyClientSpy.calledOnce, true)
})

function buildLiveTailSession(
logStreamFilter: LogStreamFilterResponse,
logEventFilterPattern: string | undefined
Expand All @@ -78,3 +126,40 @@ describe('LiveTailSession', async function () {
})
}
})

export function buildLiveTailCommandOutput(logGroupArn: string): StartLiveTailResponse {
return {
responseStream: getTestResponseStream(logGroupArn, [
{
message: 'testMessage',
timestamp: 876830400000,
},
]),
}
}

//Creates a test response stream. Each log event provided will be its own "frame" of the input stream.
export function getTestResponseStream(
logGroupIdentifier: string,
logEvents: LiveTailSessionLogEvent[]
): AsyncIterable<StartLiveTailResponseStream> {
const sessionStartFrame: StartLiveTailResponseStream = {
sessionStart: {
logGroupIdentifiers: [logGroupIdentifier],
},
sessionUpdate: undefined,
}

const updateFrames: StartLiveTailResponseStream[] = logEvents.map((event) => {
return {
sessionUpdate: {
sessionMetadata: {
sampled: false,
},
sessionResults: [event],
},
}
})

return asyncGenerator([sessionStartFrame, ...updateFrames])
}
Loading