powertools-lambda-typescript icon indicating copy to clipboard operation
powertools-lambda-typescript copied to clipboard

Bug (logger, tracer): End-to-End testing work only on eu-west-1 region

Open ijemmy opened this issue 1 year ago • 1 comments

Bug description

Running AWS_PROFILE=MY_PROFILE_NAME AWS_REGION=eu-central-1 npm run test:e2e doesn't put all test stacks on eu-central-1. Instead, it deploys all logger tests on eu-west-1 for logger and tracer

This is not consistent in all packages. E2E tests are deployed to eu-central-1 for metrics. Given that, I suspect that this from populateEnvironmentVariables.ts as both files are in logger and tracer, and it override process.env.AWS_REGION.

Stacks deployed on eu-west-1 image

Stacks deployed on eu-central-1 image

# For testing on individual package
# Deployed to incorrect region
cd packages/logger/tests/e2e
AWS_PROFILE=MY_PROFILE_NAME AWS_REGION=eu-central-1 npx jest --group=e2e/logger 
cd ../../../tracer/tests/e2e
AWS_PROFILE=MY_PROFILE_NAME AWS_REGION=eu-central-1 npx jest --group=e2e/tracer 

# Deployed to correct region 
cd ../../../metrics/tests/e2e
AWS_PROFILE=MY_PROFILE_NAME AWS_REGION=eu-central-1 npx jest --group=e2e/metrics/decorator

Expected Behavior

It should have deployed test stacks on eu-central-1

Current Behavior

It deploys logger and tracer test stacks on eu-west-1. Only metrics test stacks are deployed to eu-central-1

Possible Solution

A simple fix is to remove process.env.AWS_REGION assignment. However, this value is used for unit testing (at least in logger). The unit testing assume that the log output will have eu-west-1 as a region. So the options to fix this issue are either:

  1. Remove the assignment. And make all unit tests verify input based on current process.env.AWS_REGION instead of hardcoded value.
  2. Add if condition to the assignment, use eu-west-1 if running on unit testing mode or e2e testing mode when AWS_REGION is not provided.

Option 1 seems more attractive to me. But I am not 100% sure if this better. The implementor should evaluate theses two options or find a better one.

Steps to Reproduce

  1. cd packages/logger/tests/e2e
  2. DISABLE_TEARDOWN=false AWS_PROFILE=MY_PROFILE_NAME AWS_REGION=eu-central-1 npx jest --group=e2e/logger
  3. Check in console if it's deployed in eu-central-1. (It should not)

Repeat the same steps for tracer

Environment

  • Powertools version used: 1.2.1
  • Packaging format (Layers, npm): npm
  • AWS Lambda function runtime: N/A
  • Debugging logs: N/A

Related issues, RFCs

N/A

ijemmy avatar Sep 01 '22 13:09 ijemmy

The App class (example usage) accepts/considers a CDK_DEFAULT_REGION environment variable. Based on my understanding of the docs this variable should be considered when running a cdk deploy, although I don't know if it'll take precedence over the AWS_REGION one.

If that's not the case, we could modify the App init to look at these two variables in the order of preference we prefer.

However the behavior of populateEnvironmentVariables.ts that you mention seems like an unintended side effect and something that could cause problems in the future. Given that it's only mentioned in the Jest config and that it's not standard among the packages I can see how we'd forget about it in the future. I wouldn't mind being more explicit in the unit tests about which environment variables are set somewhere at the beginning of the tests (i.e. here) rather than relying on a file that might set other variables not being tested at all.

dreamorosi avatar Sep 01 '22 14:09 dreamorosi

⚠️ COMMENT VISIBILITY WARNING ⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.

github-actions[bot] avatar Jan 18 '23 11:01 github-actions[bot]