Skip to content

Commit 8a990b7

Browse files
authored
tests: enable e2e, retries, bug fixes (#4144)
* tests: disable fail fast for windows tests, enable e2e Failures for windows insiders will cancel stable test. For e2e tests, will we monitor for any throttling. * tests(e2e): add retries to CoCa client, fix bugs - Retry strategy chosen somewhat arbitrarily, but these settings don't seem to delay the test suite by much if at all. In recent runs with these, throttling has stopped. - Adds retries with jitter by utilizing retries in the SDK, see https://github.com/aws/aws-sdk-js/blob/3e616251947c73d5239178c167a9d73d985ca581/lib/util.js#L884. - Also fixes various bugs with the tests that causes errors to be thrown inadvertently, including type checking/verifty the code of some exceptions. In one case, this threw an AccessDeniedException when checking a dev env that was already deleted previously. * update comments
1 parent ca8a46d commit 8a990b7

File tree

4 files changed

+46
-32
lines changed

4 files changed

+46
-32
lines changed

.github/workflows/node.js.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ jobs:
5757
name: test Windows
5858
runs-on: windows-2019
5959
strategy:
60+
fail-fast: false
6061
matrix:
6162
node-version: [16.x]
6263
vscode-version: [stable, insiders]

src/shared/clients/codecatalystClient.ts

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
import { truncateProps } from '../utilities/textUtilities'
3131
import { SsoConnection } from '../../auth/connection'
3232
import { DevSettings } from '../settings'
33+
import { RetryDelayOptions } from 'aws-sdk/lib/config-base'
3334

3435
export interface CodeCatalystConfig {
3536
readonly region: string
@@ -119,16 +120,24 @@ function toBranch(
119120
}
120121
}
121122

123+
interface RetryOptions {
124+
retryDelayOptions?: RetryDelayOptions
125+
maxRetries?: number
126+
}
127+
122128
async function createCodeCatalystClient(
123129
connection: SsoConnection,
124-
regionCode = getCodeCatalystConfig().region,
125-
endpoint = getCodeCatalystConfig().endpoint
130+
regionCode: string,
131+
endpoint: string | AWS.Endpoint,
132+
retryOptions: RetryOptions
126133
): Promise<CodeCatalyst> {
127134
const c = await globals.sdkClientBuilder.createAwsService(CodeCatalyst, {
128135
region: regionCode,
129136
correctClockSkew: true,
130137
endpoint: endpoint,
131138
token: new TokenProvider(connection),
139+
retryDelayOptions: retryOptions.retryDelayOptions,
140+
maxRetries: retryOptions.maxRetries,
132141
} as ServiceConfigurationOptions)
133142

134143
return c
@@ -155,9 +164,10 @@ export type CodeCatalystClientFactory = () => Promise<CodeCatalystClient>
155164
export async function createClient(
156165
connection: SsoConnection,
157166
regionCode = getCodeCatalystConfig().region,
158-
endpoint = getCodeCatalystConfig().endpoint
167+
endpoint = getCodeCatalystConfig().endpoint,
168+
retryOptions: RetryOptions = {}
159169
): Promise<CodeCatalystClient> {
160-
const sdkClient = await createCodeCatalystClient(connection, regionCode, endpoint)
170+
const sdkClient = await createCodeCatalystClient(connection, regionCode, endpoint, retryOptions)
161171
const c = new CodeCatalystClientInternal(connection, sdkClient)
162172
await c.verifySession()
163173

src/test/techdebt.test.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,6 @@ describe('tech debt', function () {
4444
)
4545
})
4646

47-
it('stop skipping CodeCatalyst E2E Tests', function () {
48-
// https://issues.amazon.com/issues/IDE-10496
49-
assert(
50-
new Date() < new Date(2024, 1, 15),
51-
'Re-evaluate if we should still keep skipping CodeCatalyst E2E Tests'
52-
)
53-
})
54-
5547
it('stop not using latest python extension version in integration CI tests', function () {
5648
/**
5749
* The explicitly set version is done in {@link installVSCodeExtension}

src/testE2E/codecatalyst/client.test.ts

Lines changed: 31 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,14 @@ import { captureEventOnce } from '../../test/testUtil'
2929
import { toStream } from '../../shared/utilities/collectionUtils'
3030
import { toCollection } from '../../shared/utilities/asyncCollection'
3131
import { getLogger } from '../../shared/logger'
32-
import { isAwsError } from '../../shared/errors'
32+
import { isAwsError, ToolkitError } from '../../shared/errors'
3333
import {
3434
scopesCodeCatalyst,
3535
createBuilderIdProfile,
3636
isValidCodeCatalystConnection,
3737
SsoConnection,
3838
} from '../../auth/connection'
39+
import { hasKey } from '../../shared/utilities/tsUtils'
3940

4041
let spaceName: CodeCatalystOrg['name']
4142
let projectName: CodeCatalystProject['name']
@@ -86,7 +87,7 @@ let projectName: CodeCatalystProject['name']
8687
* integ tests, but using the ssh hostname that we get from
8788
* {@link prepareDevEnvConnection}.
8889
*/
89-
describe.skip('Test how this codebase uses the CodeCatalyst API', function () {
90+
describe('Test how this codebase uses the CodeCatalyst API', function () {
9091
let client: CodeCatalystClient
9192
let commands: CodeCatalystCommands
9293
let webviewClient: CodeCatalystCreateWebview
@@ -193,11 +194,11 @@ describe.skip('Test how this codebase uses the CodeCatalyst API', function () {
193194
assert.strictEqual(actualDevEnv.persistentStorage.sizeInGiB, 32)
194195
})
195196

196-
it('creates a Dev Environment using an existing branch', async function () {
197-
// TODO: The CC API does not provide a way to create a repository
198-
// due to this we'll want to revisit this test.
199-
// For now, we can manually create repository in the test project
200-
// and then continue this test from that point.
197+
it.skip('creates a Dev Environment using an existing branch', async function () {
198+
// TODO: Write this test now that an API is available in the SDK:
199+
// https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/CodeCatalyst.html#createSourceRepository-property
200+
// https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/CodeCatalyst.html#createSourceRepositoryBranch-property
201+
// For now, the repository is manually created in the account.
201202
})
202203

203204
it('prompts to install the ssh extension if not available', async function () {
@@ -354,16 +355,15 @@ describe.skip('Test how this codebase uses the CodeCatalyst API', function () {
354355
assert.ok(spaces.find(space => space.name === spaceName))
355356
})
356357

357-
it('lists all projects for the given user', async function () {
358+
// TODO: Re-add this test when the CoCa SDK offers a way to delete projects.
359+
it.skip('lists all projects for the given user', async function () {
358360
// Create lots of projects
359-
// const ephemeralProjectNames = []
360-
// for (let index = 0; index < 30; index++) {
361-
// ephemeralProjectNames.push(`ephemeral-project-${index}`)
362-
// }
363-
// await Promise.all(
364-
// ephemeralProjectNames.map(name => tryCreateTestProject(spaceName, name))
365-
// )
366-
// const projects = await client.listProjects({spaceName}).flatten().promise()
361+
const ephemeralProjectNames = []
362+
for (let index = 0; index < 30; index++) {
363+
ephemeralProjectNames.push(`ephemeral-project-${index}`)
364+
}
365+
await Promise.all(ephemeralProjectNames.map(name => tryCreateTestProject(spaceName, name)))
366+
await client.listProjects({ spaceName }).flatten().promise()
367367
})
368368

369369
function buildDevEnvSettings(
@@ -425,7 +425,15 @@ describe.skip('Test how this codebase uses the CodeCatalyst API', function () {
425425
*/
426426
async function createTestCodeCatalystClient(auth: Auth): Promise<CodeCatalystClient> {
427427
const conn = await useCodeCatalystSsoConnection(auth)
428-
return await createCodeCatalystClient(conn)
428+
return await createCodeCatalystClient(conn, undefined, undefined, {
429+
// Add retries for tests since many may be running in parallel in github CI.
430+
// AWS SDK adds jitter automatically.
431+
// https://github.com/aws/aws-sdk-js/blob/3e616251947c73d5239178c167a9d73d985ca581/lib/util.js#L884
432+
retryDelayOptions: {
433+
base: 1200, // ms
434+
},
435+
maxRetries: 5,
436+
})
429437
}
430438

431439
/**
@@ -457,7 +465,8 @@ describe.skip('Test how this codebase uses the CodeCatalyst API', function () {
457465
description: 'This project is autogenerated by the AWS Toolkit VSCode Integ Test.',
458466
})
459467
} catch (e) {
460-
if (isAwsError(e) && e.code === 'ConflictException') {
468+
if ((isAwsError(e) || e instanceof ToolkitError) && e.code === 'ConflictException') {
469+
getLogger().debug(`Tried to create test project but it already exists: ${spaceName}/${projectName}`)
461470
return client.getProject({
462471
spaceName,
463472
name: projectName,
@@ -578,8 +587,10 @@ describe.skip('Test how this codebase uses the CodeCatalyst API', function () {
578587
let devEnvBeingDeleted: DevEnvironment
579588
try {
580589
devEnvBeingDeleted = await client.getDevEnvironment({ spaceName, projectName, id })
581-
} catch (e) {
582-
if (e instanceof Error && e.name === AccessDeniedException.name) {
590+
} catch (e: any) {
591+
// Cannot use isAwsError() because the client actually returns a regular Error
592+
// with a 'code' property for this call.
593+
if (hasKey(e, 'code') && e.code === AccessDeniedException.name) {
583594
// This error is thrown because the dev env does not exist anymore
584595
// on the server. The name doesn't make it obvious IMO.
585596
return true

0 commit comments

Comments
 (0)