Skip to content

Commit

Permalink
chore: lint tests, fix misused promises in tests (#13022)
Browse files Browse the repository at this point in the history
* chore: lint tests

* chore: fix prettier
  • Loading branch information
sobolk authored Jul 28, 2023
1 parent 3e08fb9 commit c5b4ffa
Show file tree
Hide file tree
Showing 53 changed files with 159 additions and 134 deletions.
36 changes: 19 additions & 17 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,22 +102,26 @@ module.exports = {
},
{
// edit rules here to modify test linting
files: ['__tests__/**', '*.test.ts', '**/amplify-e2e-tests/**'],
plugins: ['jest'],
extends: ['plugin:jest/recommended'],
rules: {
'@typescript-eslint/unbound-method': 'off',
'jest/unbound-method': 'error',
'@typescript-eslint/no-explicit-any': 'off',
'spellcheck/spell-checker': 'off',
files: ['**/__tests__/**', '**/__test__/**', '*.test.ts', '**/amplify-e2e-*/**', '**/test/**', '**/tests/**', '**/__e2e__/**'],
parserOptions: {
project: ['tsconfig.tests.json'],
},
},
{
// disable spell checker in tests
files: ['**/__tests__/**', '**/__test__/**', '*.test.ts', 'packages/amplify-e2e-*/**', '**/test/**', '**/tests/**'],
plugins: ['jest'],
extends: ['plugin:jest/recommended'],
rules: {
'@typescript-eslint/ban-ts-comment': 'off',
'@typescript-eslint/ban-types': 'off',
'@typescript-eslint/no-empty-function': 'off',
'@typescript-eslint/no-explicit-any': 'off',
'@typescript-eslint/no-inferrable-types': 'off',
'@typescript-eslint/no-non-null-assertion': 'off',
'@typescript-eslint/no-unused-vars': 'off',
'consistent-return': 'off',
'import/no-extraneous-dependencies': 'off',
'no-constant-condition': 'off',
'no-restricted-syntax': 'off',
'no-useless-catch': 'off',
'no-useless-escape': 'off',
'no-var': 'off',
'prefer-const': 'off',
'spellcheck/spell-checker': 'off',
},
},
Expand All @@ -132,8 +136,6 @@ module.exports = {
// Files / paths / globs that shouldn't be linted at all
// (note that only .js, .jsx, .ts, and .tsx files are linted in the first place)
ignorePatterns: [
'**/__tests__/**',
'**.test.**',
'.eslintrc.js',
'scripts/',
'node_modules',
Expand All @@ -142,7 +144,6 @@ module.exports = {
'__mocks__',
'coverage',
'packages/**/lib',
'**/__e2e__/**',

// Forked package
'amplify-velocity-template',
Expand Down Expand Up @@ -207,6 +208,7 @@ module.exports = {
'/packages/amplify-category-custom/src/utils/generate-cfn-from-cdk.ts',
'/packages/amplify-environment-parameters/lib',
'/packages/amplify-opensearch-simulator/lib',
'/packages/amplify-nodejs-function-runtime-provider/src/__tests__/utils/handlerWithSyntaxError.js',

// Ignore CHANGELOG.md files
'/packages/*/CHANGELOG.md',
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
"link-dev": "mkdir -p .bin/ && cd packages/amplify-cli && ln -s \"$(pwd)/bin/amplify\" ../../.bin/amplify-dev && cd ../../",
"link-win": "node ./scripts/link-bin.js packages/amplify-cli/bin/amplify amplify-dev",
"lint-check-package-json": "yarn eslint --no-eslintrc --config .eslint.package.json.js '**/package.json'",
"lint-check": "yarn eslint . --ext .js,.jsx,.ts,.tsx --max-warnings=835",
"lint-check": "yarn eslint . --ext .js,.jsx,.ts,.tsx --max-warnings=773",
"lint-fix-package-json": "yarn lint-check-package-json --fix",
"lint-fix": "git diff --name-only --cached --diff-filter d | grep -E '\\.(js|jsx|ts|tsx)$' | xargs eslint --fix --quiet",
"mergewords": "yarn ts-node ./scripts/handle-dict-conflicts.ts",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ describe('WebsocketSubscriptionServer', () => {
server.start();
});

afterEach(() => {
server?.stop();
afterEach(async () => {
await server?.stop();
httpServer?.close();
});

Expand Down Expand Up @@ -390,7 +390,7 @@ describe('WebsocketSubscriptionServer', () => {
const data = {
onMessage: 'hello from iterator',
};
pubsub.publish('onMessage', data);
void pubsub.publish('onMessage', data);
const msg = await waitForMessage(client, MESSAGE_TYPES.GQL_DATA);
expect(msg).toEqual({
type: MESSAGE_TYPES.GQL_DATA,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,10 @@ describe('websocket subscription', () => {
expect(startSpy).toHaveBeenCalled();
});

it('should call websocket servers stop method when stop is called', () => {
it('should call websocket servers stop method when stop is called', async () => {
const stopSpy = jest.spyOn(WebsocketSubscriptionServer.prototype, 'stop');
const subs = new AppSyncSimulatorSubscriptionServer(simulatorContext, server, subscriptionPath);
subs.stop();
await subs.stop();
expect(stopSpy).toHaveBeenCalled();
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,5 +120,5 @@ test('Auth Input State -> validate cli payload manual payload', async () => {
test('Auth Input State -> validate cli payload manual payload to throw error', async () => {
const resourceName = 'mockResource';
const authState = new AuthInputState(mockContext, resourceName);
expect(authState.isCLIInputsValid()).rejects.toThrowError();
await expect(authState.isCLIInputsValid()).rejects.toThrowError();
});
Original file line number Diff line number Diff line change
Expand Up @@ -70,24 +70,24 @@ describe('generateCognitoStackResources', () => {
authProviders: [],
};

it('adds correct preSignUp lambda config and permissions', () => {
it('adds correct preSignUp lambda config and permissions', async () => {
const testApp = new cdk.App();
const cognitoStack = new AmplifyAuthCognitoStack(testApp, 'CognitoPreSignUpTriggerTest', { synthesizer: new AuthStackSynthesizer() });
cognitoStack.generateCognitoStackResources(props);
await cognitoStack.generateCognitoStackResources(props);
expect(cognitoStack.userPool?.lambdaConfig).toHaveProperty('preSignUp');
expect(cognitoStack.lambdaConfigPermissions).toHaveProperty('UserPoolPreSignupLambdaInvokePermission');
});

it('disables updateAttributeSetting when autoVerified attributes not present', () => {
it('disables updateAttributeSetting when autoVerified attributes not present', async () => {
const testApp = new cdk.App();
const cognitoStack = new AmplifyAuthCognitoStack(testApp, 'CognitoPreSignUpTriggerTest', { synthesizer: new AuthStackSynthesizer() });
const updatedProps = { ...props };
delete updatedProps.autoVerifiedAttributes;
cognitoStack.generateCognitoStackResources(updatedProps);
await cognitoStack.generateCognitoStackResources(updatedProps);
expect(cognitoStack.userPool?.userAttributeUpdateSettings).toBeUndefined();
});

it('correctly adds updateAttributeSetting when autoVerifiedAttributes attributes is TOTP', () => {
it('correctly adds updateAttributeSetting when autoVerifiedAttributes attributes is TOTP', async () => {
const testApp = new cdk.App();
// eslint-disable-next-line spellcheck/spell-checker
const cognitoStack = new AmplifyAuthCognitoStack(testApp, 'CognitoUpdateAttributesettingTest', {
Expand All @@ -97,7 +97,7 @@ describe('generateCognitoStackResources', () => {
...props,
userAutoVerifiedAttributeUpdateSettings: [AttributeType.PHONE_NUMBER],
};
cognitoStack.generateCognitoStackResources(updatedProps);
await cognitoStack.generateCognitoStackResources(updatedProps);
expect(cognitoStack.userPool?.userAttributeUpdateSettings).toMatchInlineSnapshot(`
{
"attributesRequireVerificationBeforeUpdate": [
Expand All @@ -107,7 +107,7 @@ describe('generateCognitoStackResources', () => {
`);
});

it('correctly adds updateAttributeSetting when autoVerifiedAttributes attributes is email', () => {
it('correctly adds updateAttributeSetting when autoVerifiedAttributes attributes is email', async () => {
const testApp = new cdk.App();
// eslint-disable-next-line spellcheck/spell-checker
const cognitoStack = new AmplifyAuthCognitoStack(testApp, 'CognitoUpdateAttributesettingTesting1', {
Expand All @@ -117,7 +117,7 @@ describe('generateCognitoStackResources', () => {
...props,
userAutoVerifiedAttributeUpdateSettings: [AttributeType.EMAIL],
};
cognitoStack.generateCognitoStackResources(updatedProps);
await cognitoStack.generateCognitoStackResources(updatedProps);
expect(cognitoStack.userPool?.userAttributeUpdateSettings).toMatchInlineSnapshot(`
{
"attributesRequireVerificationBeforeUpdate": [
Expand All @@ -131,7 +131,7 @@ describe('generateCognitoStackResources', () => {
expect(cognitoStack.lambdaConfigPermissions).toHaveProperty('UserPoolPreSignupLambdaInvokePermission');
});

it('correctly adds oauth properties on userpool client when oauthMetaData is defined', () => {
it('correctly adds oauth properties on userpool client when oauthMetaData is defined', async () => {
const testApp = new cdk.App();
// eslint-disable-next-line spellcheck/spell-checker
const cognitoStack = new AmplifyAuthCognitoStack(testApp, 'CognitoUpdateAttributesettingTesting1', {
Expand All @@ -142,7 +142,7 @@ describe('generateCognitoStackResources', () => {
oAuthMetadata:
'{"AllowedOAuthFlows":["code"],"AllowedOAuthScopes":["phone","email","openid","profile","aws.cognito.signin.user.admin"],"CallbackURLs":["https://localhost:3000/"]}',
};
cognitoStack.generateCognitoStackResources(updatedProps);
await cognitoStack.generateCognitoStackResources(updatedProps);
expect(cognitoStack.userPoolClientWeb).toHaveProperty('allowedOAuthFlows');
expect(cognitoStack.userPoolClientWeb).toHaveProperty('allowedOAuthScopes');
expect(cognitoStack.userPoolClientWeb).toHaveProperty('callbackUrLs');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ let mockContext = {

describe('import checks', () => {
test('throws amplify error when auth headless params are missing during pull', async () => {
expect(() => updateConfigOnEnvInit(mockContext, 'auth', 'Cognito')).rejects.toThrowErrorMatchingInlineSnapshot(
await expect(() => updateConfigOnEnvInit(mockContext, 'auth', 'Cognito')).rejects.toThrowErrorMatchingInlineSnapshot(
`"auth headless is missing the following inputParameters facebookAppIdUserPool, facebookAppSecretUserPool, loginwithamazonAppIdUserPool, loginwithamazonAppSecretUserPool, googleAppIdUserPool, googleAppSecretUserPool"`,
);
});
Expand All @@ -77,7 +77,7 @@ describe('update config when amplify pull headless command', () => {
test('throws amplify error when auth headless params are missing during pull', async () => {
mockContext.input.command = 'pull';
getOAuthObjectFromCognitoMock.mockResolvedValue(undefined);
expect(() => updateConfigOnEnvInit(mockContext, 'auth', 'Cognito')).rejects.toThrowErrorMatchingInlineSnapshot(
await expect(() => updateConfigOnEnvInit(mockContext, 'auth', 'Cognito')).rejects.toThrowErrorMatchingInlineSnapshot(
`"auth headless is missing the following inputParameters facebookAppIdUserPool, facebookAppSecretUserPool, loginwithamazonAppIdUserPool, loginwithamazonAppSecretUserPool, googleAppIdUserPool, googleAppSecretUserPool"`,
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ jest.mock('@aws-amplify/amplify-cli-core', () => ({
}));
test('migrate resource', async () => {
const resourceName = 'mockResource';
migrateResourceToSupportOverride(resourceName);
await migrateResourceToSupportOverride(resourceName);
const expectedPath = path.join('mockProjectPath', 'cli-inputs.json');
const expectedPayload = {
version: '1',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const buildFunction_mock = buildFunction as jest.MockedFunction<typeof buildFunc

describe('awscloudformation function provider', () => {
beforeEach(() => jest.clearAllMocks());
it('opens the correct service console', () => {
it('opens the correct service console', async () => {
const contextStub = {
amplify: {
getProjectMeta: () => ({
Expand All @@ -45,14 +45,14 @@ describe('awscloudformation function provider', () => {
}),
},
} as $TSContext;
openConsole(contextStub, ServiceName.LambdaFunction);
await openConsole(contextStub, ServiceName.LambdaFunction);
const openMock = open as any;
expect(openMock.mock.calls.length).toBe(1);
expect(openMock.mock.calls[0][0]).toMatchSnapshot();

openMock.mockClear();

openConsole(contextStub, ServiceName.LambdaLayer);
await openConsole(contextStub, ServiceName.LambdaLayer);
expect(openMock.mock.calls.length).toBe(1);
expect(openMock.mock.calls[0][0]).toMatchSnapshot();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,17 +185,17 @@ describe('Test reading the resource meta information', () => {
it('fails reading the meta information for non-existing resource', async () => {
const nonExistingMap = 'map12345';
const errorMessage = (resourceName: string): string => `Error reading Meta Parameters for ${resourceName}`;
expect(async () => await readResourceMetaParameters(ServiceName.Map, nonExistingMap)).rejects.toThrowError(
await expect(async () => await readResourceMetaParameters(ServiceName.Map, nonExistingMap)).rejects.toThrowError(
errorMessage(nonExistingMap),
);

const nonExistingPlaceIndex = 'placeIndex12345';
expect(async () => await readResourceMetaParameters(ServiceName.PlaceIndex, nonExistingPlaceIndex)).rejects.toThrowError(
await expect(async () => await readResourceMetaParameters(ServiceName.PlaceIndex, nonExistingPlaceIndex)).rejects.toThrowError(
errorMessage(nonExistingPlaceIndex),
);

const nonExistingGeofenceCollection = 'geofenceCollection12345';
expect(
await expect(
async () => await readResourceMetaParameters(ServiceName.GeofenceCollection, nonExistingGeofenceCollection),
).rejects.toThrowError(errorMessage(nonExistingGeofenceCollection));
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,6 @@ describe('channel-APNS', () => {
expect(enableData).toEqual(mockAPNSChannelResponseData(true, ChannelAction.ENABLE, mockPinpointResponseData.APNSChannelResponse));
});

// eslint-disable-next-line jest/no-focused-tests
test('enable unsuccessful', async () => {
prompterMock.pick.mockResolvedValueOnce('Certificate');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ describe('Test S3 transform generates correct CFN template', () => {
});
});

// eslint-disable-next-line jest/no-export
export class S3MockDataBuilder {
static mockBucketName = 'mock-stack-builder-bucket-name-99'; // s3 bucket naming rules allows alphanumeric and hyphens
static mockResourceName = 'mockResourceName';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ describe('no package Manager cases', () => {
it('error thrown when no package manager found', async () => {
(getPackageManager as jest.MockedFunction<typeof getPackageManager>).mockReturnValue(new Promise((resolve) => resolve(null)));
const projectRoot = path.join(__dirname, 'resources');
expect(
await expect(
async () =>
await AmplifyNodePkgDetector.getInstance({
projectRoot,
Expand All @@ -31,7 +31,7 @@ describe('parsing yarn lock files', () => {

(getPackageManager as jest.MockedFunction<typeof getPackageManager>).mockResolvedValue(yarnPackageManager);
const projectRoot = path.join(__dirname, 'resources');
expect(
await expect(
async () =>
await AmplifyNodePkgDetector.getInstance({
projectRoot,
Expand All @@ -46,7 +46,7 @@ describe('parsing yarn lock files', () => {
const amplifyDetectorProps: AmplifyNodePkgDetectorProps = {
projectRoot,
};
expect(async () =>
await expect(async () =>
(await AmplifyNodePkgDetector.getInstance(amplifyDetectorProps)).detectAffectedDirectDependencies('@aws-cdk/core'),
).rejects.toThrowErrorMatchingInlineSnapshot(`"yarn.lock parsing failed with an error: Invalid value type 1:16 in lockfile"`);
});
Expand Down Expand Up @@ -201,7 +201,7 @@ describe('parsing package lock files', () => {
(npmPackageManager as $TSAny).lockFile = 'package-lock-not-found.json';
(getPackageManager as jest.MockedFunction<typeof getPackageManager>).mockResolvedValue(npmPackageManager);
const projectRoot = path.join(__dirname, 'resources');
expect(
await expect(
async () =>
await AmplifyNodePkgDetector.getInstance({
projectRoot,
Expand All @@ -216,7 +216,7 @@ describe('parsing package lock files', () => {
const amplifyDetectorProps: AmplifyNodePkgDetectorProps = {
projectRoot,
};
expect(async () =>
await expect(async () =>
(await AmplifyNodePkgDetector.getInstance(amplifyDetectorProps)).detectAffectedDirectDependencies('@aws-cdk/core'),
).rejects.toThrowErrorMatchingInlineSnapshot(
`"package-lock.json parsing failed with an error: 'jsonString' argument missing or empty"`,
Expand Down Expand Up @@ -395,7 +395,7 @@ describe('parsing yarn2 lock files', () => {
const amplifyDetectorProps: AmplifyNodePkgDetectorProps = {
projectRoot,
};
expect(async () =>
await expect(async () =>
(await AmplifyNodePkgDetector.getInstance(amplifyDetectorProps)).detectAffectedDirectDependencies('@aws-cdk/core'),
).rejects.toThrowErrorMatchingInlineSnapshot(
`"yarn.lock parsing failed with an error: Unknown token: { line: 3, col: 2, type: 'INVALID', value: undefined } 3:2 in lockfile"`,
Expand Down
4 changes: 2 additions & 2 deletions packages/amplify-cli-core/src/__tests__/featureFlags.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -464,12 +464,12 @@ The following feature flags have validation errors:
process.env = { ...realProcessEnv };
});

test('initialization does not fail when process.env is not available', () => {
test('initialization does not fail when process.env is not available', async () => {
process.env = {};

expect(process.env).toEqual({});

provider.load();
await provider.load();
});

test('successfully parse every form of variables', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import * as fs from 'fs-extra';
import { hideSync } from 'hidefile';
import { setRegPendingDelete } from '../../utils/win-utils';
import { windowsPathSerializer } from '../testUtils/snapshot-serializer';
import * as path from 'path';

jest.mock('execa');
const execa_mock = execa as jest.Mocked<typeof execa>;
Expand Down Expand Up @@ -72,7 +71,7 @@ describe('uninstall packaged CLI on mac / linux', () => {
});

it('throws if it cannot remove the .amplify dir', async () => {
fs_mock.remove.mockImplementationOnce(async () => {
fs_mock.remove.mockImplementationOnce(() => {
throw new Error('fs remove did not work!');
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/* eslint-disable jest/no-interpolation-in-snapshots */
import * as fs from 'fs-extra';
import fetch, { Response } from 'node-fetch';
import { $TSContext } from '@aws-amplify/amplify-cli-core';
Expand Down Expand Up @@ -156,10 +155,10 @@ describe('run upgrade using packaged CLI', () => {

let movedBinToTemp = false;
fsMock.move
.mockImplementationOnce(async () => {
.mockImplementationOnce(() => {
movedBinToTemp = true;
})
.mockImplementationOnce(async () => {
.mockImplementationOnce(() => {
if (!movedBinToTemp) throw new Error('fs.move was not called before copying extracted file to bin location');
});

Expand Down
Loading

0 comments on commit c5b4ffa

Please sign in to comment.