chore(logger): make Logger E2E works for all runtimes

[ijemmy]

Description of your changes

Currently, E2E only runs on a single (default) runtime from CDK Lambda module. With this change,test:e2e target runs tests in all runtime to detect any bugs from compatibility issue. It will create 1 stack of test Lambda functions per run time and execute the tests in parallel.

NodeJS10 has been deprecated. So we will be running on v12 and v14 for now.

Why this approach?

There are several options to achieve this. Here are the main criteria in selecting an approach.

1. Speed Able to deploy infrastructure (in beforeAll() block) concurrently.
2. Cognitive load for test writers Test writers can focus more on testing and don't get distracted by having to think of multiple runtimes.
3. Easy to read the log Logs from concurrent test runs (from multiple runtimes) will overlap with each other.

Here are the list of options I evaluated:

1. Use it.each() and describe.each() to iterate an array of runtimes. (link) This option doesn't work as Jest won't parallel the beforeAll() execution in the same file. It doubles the time to run the tests. In addition, test writers have to explicitly specify an array of run-time, more boilerplate code.
2. Have a custom script run-e2e.ts that starts subprocess to run concurrently . This works with Node's child_process's exec. However, the log coloring is lost. In additional, we need to maintain an additional script.
3. Use forEach to iterate describe blocks to run each runtime test independently. This also won't run beforeAll() in parallel. Jest that decides to run things in parallel or sequential. We have no control over it. describe and it basically put things in an internal queue and jest has some heuristic algorithm to decide. I find that it stops running in parallel when when several describe are called in the same file.
4. Use concurrently and pass runtime as an environment variable. I find this option cleanest and satisfy the 3 criteria above. The logs overlap with each other but we there is a prefix stating which runtime the log comes from.

How to verify this change

Check out this branch and run

cd packages/logger
AWS_PROFILE=<YOUR_PROFILE> AWS_REGION=eu-central-1 npm run test:e2e

Result from local run

[test:e2e:nodejs12x]
[test:e2e:nodejs12x] Test Suites: 3 passed, 3 of 8 total
[test:e2e:nodejs12x] Tests:       13 passed, 13 total
[test:e2e:nodejs12x] Snapshots:   0 total
[test:e2e:nodejs12x] Time:        115.722 s, estimated 136 s
[test:e2e:nodejs12x] Ran all test suites.
[test:e2e:nodejs12x] npm run test:e2e:nodejs12x exited with code 0
[test:e2e:nodejs14x]
[test:e2e:nodejs14x] Test Suites: 3 passed, 3 of 8 total
[test:e2e:nodejs14x] Tests:       13 passed, 13 total
[test:e2e:nodejs14x] Snapshots:   0 total
[test:e2e:nodejs14x] Time:        115.771 s, estimated 136 s
[test:e2e:nodejs14x] Ran all test suites.
[test:e2e:nodejs14x] npm run test:e2e:nodejs14x exited with code 0

Related issues, RFCs

#353

PR status

Is this ready for review?: YES
Is it a breaking change?: NO

Checklist

• My changes meet the tenets criteria
• I have performed a self-review of my own code
• I have commented my code where necessary, particularly in areas that should be flagged with a TODO, or hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• The code coverage hasn't decreased
• I have added tests that prove my change is effective and works
• New and existing unit tests pass locally and in Github Actions
• Any dependent changes have been merged and published in downstream module
• The PR title follows the conventional commit semantics

Breaking change checklist

• I have documented the migration process
• I have added, implemented necessary warnings (if it can live side by side)

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[ijemmy]

E2E test failed due to stack creation timeout: https://github.com/awslabs/aws-lambda-powertools-typescript/actions/runs/1847481128

I increased stack timeout to 300 seconds

[ijemmy]

E2E tests pass: https://github.com/awslabs/aws-lambda-powertools-typescript/runs/5202416034?check_suite_focus=true

[flochaz]

LGTM but another option would have been to leverage github actions matrix

[dreamorosi]

LGTM but another option would have been to leverage github actions matrix

That's a good point, it seems that the example shown in this section of the docs would be what we are looking for:

name: Node.js CI
on: [push]
jobs:
  build:
    runs-on: ubuntu-latest
    strategy:
      matrix:
       include:
         - node-version: 10.x
           site: "prod"
           datacenter: "site-a"
         - node-version: 12.x
           site: "dev"
           datacenter: "site-b"
    steps:
      - name: Echo site details
        env:
          SITE: ${{ matrix.site }}
          DATACENTER: ${{ matrix.datacenter }}
        run: echo $SITE $DATACENTER

The next section about concurrency also says that it's possible to specify concurrency, the only downside of this is that there's no guarantee that jobs will run in parallel as this is decided based on availability of the runners.

If we are okay with this then we could avoid bringing an additional dependency into the project (concurrently@^7.0.0) which in the long term could save us some headaches / overhead.

Other than this I like how you implemented it and the helpers / constants that you have added.

[ijemmy]

@flochaz @dreamorosi I have tried using matrix strategy. However, it breaks the e2e tests in metrics package as two two set of e2e tests for 2 node runtimes are running together with the same stack name. https://github.com/awslabs/aws-lambda-powertools-typescript/actions/runs/1853018538

I propose to go with concurrently for now. Then I'll create PRs to make stack id unique (+ refactor them to be similar to logger). Then I'll make another PR to use strategy matrix. Is that ok with you?

[dreamorosi]

@ijemmy thanks for trying out the other method, your proposal is fine by me and I'd prioritise in this order:

• having e2e test for all utils
• having all utils running on all runtimes
• optimising test harness

So with that said no need to tackle this immediately, thanks a ton for considering it tho!