Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enabling profiling also causes snapshot test instability #359

Open
mbonig opened this issue Jan 30, 2025 · 3 comments
Open

Enabling profiling also causes snapshot test instability #359

mbonig opened this issue Jan 30, 2025 · 3 comments
Assignees

Comments

@mbonig
Copy link

mbonig commented Jan 30, 2025

Expected Behavior

When snapshot testing a stack with a DD-enabled Lambda function, I'd expect the snapshot tests to remain stable. This is not currently possible without injecting the following code before any constructs are loaded:

jest.spyOn(require('child_process'), 'execSync').mockImplementation((_command) => {
  return 'ABCDEFG0123456';
});

(and is horribly fragile)

Ideally, a context variable should be available that would allow me to choose, during tests, to mock the hash value to create stability. This has been done with good success in other projects, like:

https://github.com/wheatstalk/cdk-intrinsic-validator?tab=readme-ov-file#tips

Actual Behavior

When enabling profiling on Lambda functions, a DD_TAG environment variable is added which includes the commit hash. This causes a constant churn if using snapshot tests.

Steps to Reproduce the Problem

  1. Create a stack with a lambda function and the DD construct with profiling enabled
  2. Create a snapshot test like:
test('Snapshot', () => {
  const app = new App();
  const stack = new SomeStack(app, 'test', {
    env: {
      account: '0123456789012',
      region: 'us-east-1',
    },
  });

  const template = Template.fromStack(stack);
  expect(template.toJSON()).toMatchSnapshot();
});

  1. Commit the code change and updated snapshot
  2. re-run the snapshot, test, it will now fail as the commit hash has changed.

Specifications

  • Datadog Lambda Layer version: nodeLayerVersion: 118, extensionLayerVersion: 68,
  • Node version: 22

Stacktrace

none...
@clifordshelton
Copy link

Hi Matthew! Thanks for reporting this issue. That sounds like a very useful suggestion. We'll prioritize this and keep you updated here

@clifordshelton clifordshelton self-assigned this Jan 30, 2025
avangelillo added a commit that referenced this issue Feb 14, 2025
Add the ability to override the git sha and repo url to make snapshot testing
easier.

JIRA: https://datadoghq.atlassian.net/browse/SVLS-6260

Github Issue: #359
avangelillo added a commit that referenced this issue Feb 14, 2025
Add the ability to override the git sha and repo url to make snapshot testing
easier.

JIRA: https://datadoghq.atlassian.net/browse/SVLS-6260

Github Issue: #359
avangelillo added a commit that referenced this issue Feb 19, 2025
* Add git sha and url overrides

Add the ability to override the git sha and repo url to make snapshot testing
easier.

JIRA: https://datadoghq.atlassian.net/browse/SVLS-6260

Github Issue: #359

* Fix test name

Fix duplicated test name from comment
@avangelillo
Copy link
Contributor

Hi @mbonig, as of 1.24.0 you can now override the git metadata from your test by calling the overrideGitMetadata function on the DatadogLambda object.

test('Snapshot', () => {
  const app = new App();
  const stack = new SomeStack(app, 'test', {
    env: {
      account: '0123456789012',
      region: 'us-east-1',
    },
  });

  // New line
  stack.datadogLambda.overrideGitMetadata("fake-sha");

  const template = Template.fromStack(stack);
  expect(template.toJSON()).toMatchSnapshot();
});

@mbonig
Copy link
Author

mbonig commented Feb 25, 2025

Hi @mbonig, as of 1.24.0 you can now override the git metadata from your test by calling the overrideGitMetadata function on the DatadogLambda object.

test('Snapshot', () => {
  const app = new App();
  const stack = new SomeStack(app, 'test', {
    env: {
      account: '0123456789012',
      region: 'us-east-1',
    },
  });

  // New line
  stack.datadogLambda.overrideGitMetadata("fake-sha");

  const template = Template.fromStack(stack);
  expect(template.toJSON()).toMatchSnapshot();
});

This is slightly annoying, as it requires me to expose the underlying lambda and I'd prefer not to do that.

Could this be a context variable instead? Something like:

 const app = new App({
    context: {
      'DataDog:DisableGitSha': true,
    },
  });

See a similar scenario here:
https://github.com/wheatstalk/cdk-intrinsic-validator?tab=readme-ov-file#tips

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants