From 99fd9170e74016c7f6988d86189e527a0fe6709b Mon Sep 17 00:00:00 2001 From: Momo Kornher Date: Sun, 16 Jul 2023 14:58:28 +0100 Subject: [PATCH] chore(cdk-lib): re-enable skipped tests (#26367) Some tests were previously skipped. Un-skip them and put a linter rule in place that prevents this. Closes #25137 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../aws-lambda-nodejs/test/function.test.ts | 42 +++++++++---------- .../aws-lambda-nodejs/test/util.test.ts | 38 ++++++++--------- .../cloud-assembly-schema/test/schema.test.ts | 2 +- .../cdk-build-tools/config/eslintrc.js | 2 + 4 files changed, 43 insertions(+), 41 deletions(-) diff --git a/packages/aws-cdk-lib/aws-lambda-nodejs/test/function.test.ts b/packages/aws-cdk-lib/aws-lambda-nodejs/test/function.test.ts index 67c58101d8152..0915016eded17 100644 --- a/packages/aws-cdk-lib/aws-lambda-nodejs/test/function.test.ts +++ b/packages/aws-cdk-lib/aws-lambda-nodejs/test/function.test.ts @@ -31,7 +31,7 @@ beforeEach(() => { jest.clearAllMocks(); }); -test.skip('NodejsFunction with .ts handler', () => { +test('NodejsFunction with .ts handler', () => { // WHEN new NodejsFunction(stack, 'handler1'); @@ -41,11 +41,11 @@ test.skip('NodejsFunction with .ts handler', () => { Template.fromStack(stack).hasResourceProperties('AWS::Lambda::Function', { Handler: 'index.handler', - Runtime: 'nodejs14.x', + Runtime: Match.stringLikeRegexp('nodejs'), }); }); -test.skip('NodejsFunction with overridden handler - no dots', () => { +test('NodejsFunction with overridden handler - no dots', () => { // WHEN new NodejsFunction(stack, 'handler1', { handler: 'myHandler', @@ -57,11 +57,11 @@ test.skip('NodejsFunction with overridden handler - no dots', () => { Template.fromStack(stack).hasResourceProperties('AWS::Lambda::Function', { Handler: 'index.myHandler', // automatic index. prefix - Runtime: 'nodejs14.x', + Runtime: Match.stringLikeRegexp('nodejs'), }); }); -test.skip('NodejsFunction with overridden handler - with dots', () => { +test('NodejsFunction with overridden handler - with dots', () => { // WHEN new NodejsFunction(stack, 'handler1', { handler: 'run.sh', @@ -73,11 +73,11 @@ test.skip('NodejsFunction with overridden handler - with dots', () => { Template.fromStack(stack).hasResourceProperties('AWS::Lambda::Function', { Handler: 'run.sh', // No index. prefix - Runtime: 'nodejs14.x', + Runtime: Match.stringLikeRegexp('nodejs'), }); }); -test.skip('NodejsFunction with .js handler', () => { +test('NodejsFunction with .js handler', () => { // WHEN new NodejsFunction(stack, 'handler2'); @@ -87,7 +87,7 @@ test.skip('NodejsFunction with .js handler', () => { })); }); -test.skip('NodejsFunction with .mjs handler', () => { +test('NodejsFunction with .mjs handler', () => { // WHEN new NodejsFunction(stack, 'handler3'); @@ -144,13 +144,13 @@ test('NodejsFunction with container env vars', () => { })); }); -test.skip('throws when entry is not js/ts', () => { +test('throws when entry is not js/ts', () => { expect(() => new NodejsFunction(stack, 'Fn', { entry: 'handler.py', })).toThrow(/Only JavaScript or TypeScript entry files are supported/); }); -test.skip('accepts tsx', () => { +test('accepts tsx', () => { const entry = path.join(__dirname, 'handler.tsx'); fs.symlinkSync(path.join(__dirname, 'function.test.handler1.ts'), entry); @@ -162,35 +162,35 @@ test.skip('accepts tsx', () => { fs.unlinkSync(entry); }); -test.skip('throws when entry does not exist', () => { +test('throws when entry does not exist', () => { expect(() => new NodejsFunction(stack, 'Fn', { entry: 'notfound.ts', })).toThrow(/Cannot find entry file at/); }); -test.skip('throws when entry cannot be automatically found', () => { - expect(() => new NodejsFunction(stack, 'Fn')).toThrow(/Cannot find handler file .*function.test.Fn.ts, .*function.test.Fn.js or .*function.test.Fn.mjs/); +test('throws when entry cannot be automatically found', () => { + expect(() => new NodejsFunction(stack, 'Fn')).toThrow(/Cannot find handler file .*function\.test\.Fn\.ts.*function\.test\.Fn\.js.*function\.test\.Fn\.mjs/); }); -test.skip('throws with the wrong runtime family', () => { +test('throws with the wrong runtime family', () => { expect(() => new NodejsFunction(stack, 'handler1', { runtime: Runtime.PYTHON_3_8, })).toThrow(/Only `NODEJS` runtimes are supported/); }); -test.skip('throws with non existing lock file', () => { +test('throws with non existing lock file', () => { expect(() => new NodejsFunction(stack, 'handler1', { depsLockFilePath: '/does/not/exist.lock', })).toThrow(/Lock file at \/does\/not\/exist.lock doesn't exist/); }); -test.skip('throws when depsLockFilePath is not a file', () => { +test('throws when depsLockFilePath is not a file', () => { expect(() => new NodejsFunction(stack, 'handler1', { depsLockFilePath: __dirname, })).toThrow(/\`depsLockFilePath\` should point to a file/); }); -test.skip('resolves depsLockFilePath to an absolute path', () => { +test('resolves depsLockFilePath to an absolute path', () => { new NodejsFunction(stack, 'handler1', { depsLockFilePath: './package.json', }); @@ -200,7 +200,7 @@ test.skip('resolves depsLockFilePath to an absolute path', () => { })); }); -test.skip('resolves entry to an absolute path', () => { +test('resolves entry to an absolute path', () => { // WHEN new NodejsFunction(stack, 'fn', { entry: 'aws-lambda-nodejs/lib/index.ts', @@ -211,7 +211,7 @@ test.skip('resolves entry to an absolute path', () => { })); }); -test.skip('configures connection reuse for aws sdk', () => { +test('configures connection reuse for aws sdk', () => { // WHEN new NodejsFunction(stack, 'handler1'); @@ -224,7 +224,7 @@ test.skip('configures connection reuse for aws sdk', () => { }); }); -test.skip('can opt-out of connection reuse for aws sdk', () => { +test('can opt-out of connection reuse for aws sdk', () => { // WHEN new NodejsFunction(stack, 'handler1', { awsSdkConnectionReuse: false, @@ -235,7 +235,7 @@ test.skip('can opt-out of connection reuse for aws sdk', () => { }); }); -test.skip('NodejsFunction in a VPC', () => { +test('NodejsFunction in a VPC', () => { // GIVEN const vpc = new Vpc(stack, 'Vpc'); diff --git a/packages/aws-cdk-lib/aws-lambda-nodejs/test/util.test.ts b/packages/aws-cdk-lib/aws-lambda-nodejs/test/util.test.ts index fc2b286f48b0d..1414ead7d7933 100644 --- a/packages/aws-cdk-lib/aws-lambda-nodejs/test/util.test.ts +++ b/packages/aws-cdk-lib/aws-lambda-nodejs/test/util.test.ts @@ -12,64 +12,64 @@ describe('callsites', () => { }); describe('findUp', () => { - test.skip('Starting at process.cwd()', () => { + test('Starting at process.cwd()', () => { expect(findUp('README.md')).toMatch(/aws-cdk-lib\/README.md$/); }); - test.skip('Non existing file', () => { + test('Non existing file', () => { expect(findUp('non-existing-file.unknown')).toBe(undefined); }); - test.skip('Starting at a specific path', () => { + test('Starting at a specific path', () => { expect(findUp('util.test.ts', path.join(__dirname, 'integ-handlers'))).toMatch(/aws-lambda-nodejs\/test\/util.test.ts$/); }); - test.skip('Non existing file starting at a non existing relative path', () => { + test('Non existing file starting at a non existing relative path', () => { expect(findUp('not-to-be-found.txt', 'non-existing/relative/path')).toBe(undefined); }); - test.skip('Starting at a relative path', () => { + test('Starting at a relative path', () => { expect(findUp('util.test.ts', 'aws-lambda-nodejs/test/integ-handlers')).toMatch(/aws-lambda-nodejs\/test\/util.test.ts$/); }); }); describe('findUpMultiple', () => { - test.skip('Starting at process.cwd()', () => { + test('Starting at process.cwd()', () => { const files = findUpMultiple(['README.md', 'package.json']); expect(files).toHaveLength(2); expect(files[0]).toMatch(/aws-cdk-lib\/README\.md$/); expect(files[1]).toMatch(/aws-cdk-lib\/package\.json$/); }); - test.skip('Non existing files', () => { + test('Non existing files', () => { expect(findUpMultiple(['non-existing-file.unknown', 'non-existing-file.unknown2'])).toEqual([]); }); - test.skip('Existing and non existing files', () => { + test('Existing and non existing files', () => { const files = findUpMultiple(['non-existing-file.unknown', 'README.md']); expect(files).toHaveLength(1); expect(files[0]).toMatch(/aws-cdk-lib\/README\.md$/); }); - test.skip('Starting at a specific path', () => { + test('Starting at a specific path', () => { const files = findUpMultiple(['util.test.ts', 'function.test.ts'], path.join(__dirname, 'integ-handlers')); expect(files).toHaveLength(2); expect(files[0]).toMatch(/aws-lambda-nodejs\/test\/util\.test\.ts$/); expect(files[1]).toMatch(/aws-lambda-nodejs\/test\/function\.test\.ts$/); }); - test.skip('Non existing files starting at a non existing relative path', () => { + test('Non existing files starting at a non existing relative path', () => { expect(findUpMultiple(['not-to-be-found.txt', 'not-to-be-found2.txt'], 'non-existing/relative/path')).toEqual([]); }); - test.skip('Starting at a relative path', () => { + test('Starting at a relative path', () => { const files = findUpMultiple(['util.test.ts', 'function.test.ts'], 'aws-lambda-nodejs/test/integ-handlers'); expect(files).toHaveLength(2); expect(files[0]).toMatch(/aws-lambda-nodejs\/test\/util\.test\.ts$/); expect(files[1]).toMatch(/aws-lambda-nodejs\/test\/function\.test\.ts$/); }); - test.skip('Files on multiple levels', () => { + test('Files on multiple levels', () => { const files = findUpMultiple(['README.md', 'util.test.ts'], path.join(__dirname, 'integ-handlers')); expect(files).toHaveLength(1); expect(files[0]).toMatch(/aws-lambda-nodejs\/test\/util\.test\.ts$/); @@ -77,7 +77,7 @@ describe('findUpMultiple', () => { }); describe('exec', () => { - test.skip('normal execution', () => { + test('normal execution', () => { const spawnSyncMock = jest.spyOn(child_process, 'spawnSync').mockReturnValue({ status: 0, stderr: Buffer.from('stderr'), @@ -103,7 +103,7 @@ describe('exec', () => { spawnSyncMock.mockRestore(); }); - test.skip('non zero status', () => { + test('non zero status', () => { const spawnSyncMock = jest.spyOn(child_process, 'spawnSync').mockReturnValue({ status: 999, stderr: Buffer.from('error occured'), @@ -118,7 +118,7 @@ describe('exec', () => { spawnSyncMock.mockRestore(); }); - test.skip('with error', () => { + test('with error', () => { const spawnSyncMock = jest.spyOn(child_process, 'spawnSync').mockReturnValue({ error: new Error('bad error'), status: 0, @@ -136,7 +136,7 @@ describe('exec', () => { }); describe('extractDependencies', () => { - test.skip('with dependencies referenced in package.json', () => { + test('with dependencies referenced in package.json', () => { const deps = extractDependencies( path.join(__dirname, 'testpackage.json'), ['@aws-cdk/aws-lambda', '@aws-cdk/core'], @@ -147,7 +147,7 @@ describe('extractDependencies', () => { ]); }); - test.skip('with transitive dependencies', () => { + test('with transitive dependencies', () => { expect(extractDependencies( path.join(__dirname, 'testpackage.json'), ['typescript'], @@ -157,14 +157,14 @@ describe('extractDependencies', () => { }); }); - test.skip('with unknown dependency', () => { + test('with unknown dependency', () => { expect(() => extractDependencies( path.join(__dirname, 'testpackage.json'), ['unknown'], )).toThrow(/Cannot extract version for module 'unknown'/); }); - test.skip('with file dependency', () => { + test('with file dependency', () => { const pkgPath = path.join(__dirname, 'package-file.json'); fs.writeFileSync(pkgPath, JSON.stringify({ dependencies: { diff --git a/packages/aws-cdk-lib/cloud-assembly-schema/test/schema.test.ts b/packages/aws-cdk-lib/cloud-assembly-schema/test/schema.test.ts index 7b66dd89fc9d1..ad935bbe73789 100644 --- a/packages/aws-cdk-lib/cloud-assembly-schema/test/schema.test.ts +++ b/packages/aws-cdk-lib/cloud-assembly-schema/test/schema.test.ts @@ -1,6 +1,6 @@ import { generateSchema, SCHEMAS } from '../scripts/update-schema'; -test.skip('if this test fails, run "yarn update-schema"', () => { +test('if this test fails, run "yarn update-schema"', () => { // when we compare schemas we ignore changes the // description that is generated from the ts docstrings. diff --git a/tools/@aws-cdk/cdk-build-tools/config/eslintrc.js b/tools/@aws-cdk/cdk-build-tools/config/eslintrc.js index 4eef922ecae44..d37fadbc92722 100644 --- a/tools/@aws-cdk/cdk-build-tools/config/eslintrc.js +++ b/tools/@aws-cdk/cdk-build-tools/config/eslintrc.js @@ -214,5 +214,7 @@ module.exports = { "jest/valid-expect": "off", // expect from '@aws-cdk/assert' can take a second argument "jest/valid-title": "off", // A little over-zealous with test('test foo') being an error. "jest/no-identical-title": "off", // TEMPORARY - Disabling this until https://github.com/jest-community/eslint-plugin-jest/issues/836 is resolved + 'jest/no-disabled-tests': 'error', // Skipped tests are easily missed in PR reviews + 'jest/no-focused-tests': 'error', // Focused tests are easily missed in PR reviews }, };