Skip to content

Commit

Permalink
chore(cdk-lib): re-enable skipped tests (#26367)
Browse files Browse the repository at this point in the history 
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*
  • Loading branch information
@mrgrain
mrgrain authored Jul 16, 2023
1 parent c575dde commit 99fd917
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 41 deletions.
42 changes: 21 additions & 21 deletions packages/aws-cdk-lib/aws-lambda-nodejs/test/function.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ beforeEach(() => {
jest.clearAllMocks();
});

test.skip('NodejsFunction with .ts handler', () => {
test('NodejsFunction with .ts handler', () => {
// WHEN
new NodejsFunction(stack, 'handler1');

Expand All @@ -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',
Expand All @@ -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',
Expand All @@ -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');

Expand All @@ -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');

Expand Down Expand Up @@ -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);
Expand All @@ -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',
});
Expand All @@ -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',
Expand All @@ -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');

Expand All @@ -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,
Expand All @@ -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');

Expand Down
38 changes: 19 additions & 19 deletions packages/aws-cdk-lib/aws-lambda-nodejs/test/util.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,72 +12,72 @@ 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$/);
});
});

describe('exec', () => {
test.skip('normal execution', () => {
test('normal execution', () => {
const spawnSyncMock = jest.spyOn(child_process, 'spawnSync').mockReturnValue({
status: 0,
stderr: Buffer.from('stderr'),
Expand All @@ -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'),
Expand All @@ -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,
Expand All @@ -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'],
Expand All @@ -147,7 +147,7 @@ describe('extractDependencies', () => {
]);
});

test.skip('with transitive dependencies', () => {
test('with transitive dependencies', () => {
expect(extractDependencies(
path.join(__dirname, 'testpackage.json'),
['typescript'],
Expand All @@ -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: {
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk-lib/cloud-assembly-schema/test/schema.test.ts
View file
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
2 changes: 2 additions & 0 deletions tools/@aws-cdk/cdk-build-tools/config/eslintrc.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
};

0 comments on commit 99fd917

Please sign in to comment.