aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.72k stars 3.94k forks source link

(assert): snapshot testing is hindered by changing hashes #13468

Open danilofuchs opened 3 years ago

danilofuchs commented 3 years ago

Using ecsPatterns.ApplicationLoadBalancedFargateService to deploy a Docker asset, every time a single file is changed in the source folder, a new hash and asset is created. This means snapshot testing is not viable, as every minor change leads to a change in snapshot.

Reproduction Steps

Create a basic ecsPatterns.ApplicationLoadBalancedFargateService and add a test file with the contents:

// stack.test.ts
import { SynthUtils } from "@aws-cdk/assert";
import { App } from "@aws-cdk/core";

import { CDKStack } from "../lib/CDKStack";

const app = new App();

describe("Stack", () => {
    const stack = new CDKStack(app, "test");

    it("should match snapshot", () => {
        expect(SynthUtils.toCloudFormation(stack)).toMatchSnapshot();
    });
})

Create the snapshot with yarn test.

Change a single file in the source folder

Rerun the tests with yarn test

What did you expect to happen?

No change to snapshot

What actually happened?

Change on snapshot hash

image

Environment

Other

I expect to find some way of preserving the hash or omitting it from snapshot testing


This is :bug: Bug Report

eladb commented 3 years ago

This is actually the expected behavior. Your docker asset is part of your construct. If it changes, your snapshot is expected to change.

However I do acknowledge that this is not ideal from a workflow perspective.

@nija-at is working on some testing capabilities, and I am wondering if we can perhaps support "ignoring assets" as a feature of that framework. @nija-at what do you think?

Maybe we can repurpose this issue as a feature request?

nija-at commented 3 years ago

Agreed to mark it as a feature request. However, I am unable to work on this immediately.

exoego commented 2 years ago

I use https://www.npmjs.com/package/jest-cdk-snapshot to ignore Assets

expect(stack).toMatchCdkSnapshot({
    ignoreAssets: true,
  });

Hope similar feature is landed on official cdk assert module

gbvanrenswoude commented 1 year ago

Workaround, loosely based on @exoego's comment but without the need to install 3th party libraries:

import { SynthUtils } from '@aws-cdk/assert';
import * as cdk from 'aws-cdk-lib';
import { Stack, StageSynthesisOptions } from 'aws-cdk-lib';
import { Template } from 'aws-cdk-lib/assertions';
import { InfrastructureStack } from '../../src/lib/infrastructure-stack';

type Options = StageSynthesisOptions & {
    /**
     * Ignore Assets
     */
    ignoreAssets?: boolean;
    /**
     * Remove only resources of given types, e.g. subsetResourceTypes: ['AWS::SNS::Topic']
     */
    removeResourceTypes?: string[];
    /**
     * Remove only resources of given keys, e.g. subsetResourceKeys: ['TopicX']
     */
    removeResourceKeys?: string[];
    /**
     * Match only resources of given types, e.g. subsetResourceTypes: ['AWS::SNS::Topic']
     */
    subsetResourceTypes?: string[];
    /**
     * Match only resources of given keys, e.g. subsetResourceKeys: ['TopicX']
     */
    subsetResourceKeys?: string[];
    propertyMatchers?: { [property: string]: unknown };
};

const postProcessedStack = (stack: Stack, options: Options = {}) => {
    const { ignoreAssets = false, removeResourceTypes, removeResourceKeys, subsetResourceTypes, subsetResourceKeys, ...synthOptions } = options;

    const template = SynthUtils.toCloudFormation(stack, synthOptions);

    if (ignoreAssets && template.Resources) {
        if (template.Parameters) {
            template.Parameters = expect.any(Object);
        }

        // eslint-disable-next-line @typescript-eslint/no-explicit-any
        Object.values(template.Resources).forEach((resource: any) => {
            if (resource?.Properties?.Code) {
                resource.Properties.Code = expect.any(Object);
            }
        });
    }

    if (subsetResourceTypes && template.Resources) {
        for (const [key, resource] of Object.entries(template.Resources)) {
            // eslint-disable-next-line @typescript-eslint/no-explicit-any
            if (!subsetResourceTypes.includes((resource as any).Type)) {
                delete template.Resources[key];
            }
        }
    }

    if (subsetResourceKeys && template.Resources) {
        for (const [key] of Object.entries(template.Resources)) {
            if (!subsetResourceKeys.includes(key)) {
                delete template.Resources[key];
            }
        }
    }

    if (removeResourceTypes && template.Resources) {
        for (const [key, resource] of Object.entries(template.Resources)) {
            // eslint-disable-next-line @typescript-eslint/no-explicit-any
            if (removeResourceTypes.includes((resource as any).Type)) {
                delete template.Resources[key];
            }
        }
    }

    if (removeResourceKeys && template.Resources) {
        for (const [key] of Object.entries(template.Resources)) {
            if (removeResourceKeys.includes(key)) {
                delete template.Resources[key];
            }
        }
    }

    return Template.fromJSON(template);
};

describe('CDK Infra tests - master', () => {
    const app = new cdk.App();
    const templateMaster = postProcessedStack(
        new InfrastructureStack(app, 'some, 'stack', valueObject, {
            env: {
                account: something,
                region: something,
            },
        }),
        { ignoreAssets: true, removeResourceTypes: ['AWS::CloudFormation::Stack', 'Custom::AWSCDKOpenIdConnectProvider'] }, // removes assets for nested stacks e.d. as well
    );

    test('CDK_SNAPSHOT - Matches the snapshot', () => {
        expect(templateMaster).toMatchSnapshot();
    });

...
mmarthab commented 6 months ago

+1 this, if CDK could probably add a way to exclude some resources or resource properties during snapshot testing.