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

Maintenance: run e2e tests in parallel #1512

Closed
1 of 2 tasks
am29d opened this issue Jun 19, 2023 · 4 comments · Fixed by #1747
Closed
1 of 2 tasks

Maintenance: run e2e tests in parallel #1512

am29d opened this issue Jun 19, 2023 · 4 comments · Fixed by #1747
Assignees
Labels
automation This item relates to automation completed This item is complete and has been merged/shipped internal PRs that introduce changes in governance, tech debt and chores (linting setup, baseline, etc.)

Comments

@am29d
Copy link
Contributor

am29d commented Jun 19, 2023

Summary

While we run e2e tests in a matrix:

    strategy:
      matrix:
        package: [logger, metrics, tracer, parameters]
        version: [14, 16, 18]
      fail-fast: false

it would be great to also be able to execute these tests during development.

Why is this needed?

To reduce feedback cycle time for e2e tests during development.

Which area does this relate to?

Automation

Solution

@dreamorosi already opened a discussion, and the pointers to RFCs. I don't have a solution yet, so any recommendation and contribution is appreciated.

Acknowledgment

Future readers

Please react with 👍 and your use case to help us understand customer demand.

@am29d am29d added triage This item has not been triaged by a maintainer, please wait internal PRs that introduce changes in governance, tech debt and chores (linting setup, baseline, etc.) labels Jun 19, 2023
@dreamorosi
Copy link
Contributor

If you run them with npm run test:e2e -ws it will run them sequentially for one runtime (Node.js 18).

If we want to run them in parallel we could write a bash script that does that like npm run test:e2e -w packages/tracer & npm run test:e2e -w packages/logger & ....

This however will mix all the stdout & stderr making it hard to read what's happening in case of multiple failures. This is also what happens with lerna, which is able to run things in parallel and is already available in the repo (but used only for version and release).

This is something we should keep our eyes on, however in most cases I'm running locally only specific groups/utilities and then run the full suite only on the repo. This makes it also easier to collaborate on results and troubleshooting when needed.

@dreamorosi dreamorosi added automation This item relates to automation blocked This item's progress is blocked by external dependency or reason and removed triage This item has not been triaged by a maintainer, please wait labels Jun 19, 2023
@dreamorosi
Copy link
Contributor

Been looking into this while working on other issues and running the tests in parallel with a fairly crude method caused me to hit rate limits on deployment.

Specifically, I ran three terminal sessions like this:

  • session 1: RUNTIME=nodejs14x npm run test:e2e -w packages/tracer
  • session 2: RUNTIME=nodejs16x npm run test:e2e -w packages/tracer
  • session 3: npm run test:e2e -w packages/tracer

I wonder if this will be an issue if we try to run multiple packages / runtimes combos concurrently.

@dreamorosi dreamorosi linked a pull request Oct 12, 2023 that will close this issue
9 tasks
@dreamorosi dreamorosi removed a link to a pull request Oct 12, 2023
9 tasks
@dreamorosi dreamorosi self-assigned this Oct 16, 2023
@dreamorosi dreamorosi added confirmed The scope is clear, ready for implementation and removed blocked This item's progress is blocked by external dependency or reason labels Oct 16, 2023
@dreamorosi
Copy link
Contributor

dreamorosi commented Oct 16, 2023

I have been spending some time investigating this and wanted to share some updates as well as a low hanging/low effort way of mitigating this in the short term.

The main way of running the integration tests in all the workspaces is via npm workspaces, and can be done with this command:

npm run test:e2e -ws
# ... many logs
# duration 10m 55s

This runs the tests one workspace at the time in the order that they appear in the package.json file under workspaces. Test files (aka cases) within a workspaces are already parallelized by Jest by default.

As I mentioned in one of the comments above, another option that is enabled (but not documented) in our repo would be to use Lerna:

npx lerna exec --no-bail --no-sort --stream --concurrency 8 -- npm run test:e2e
# alias npm run test:e2e:parallel
# ... many logs
lerna success exec Executed command in 11 packages: "npm run test:e2e"
# duration 3m 35s

The concurrency can be adjusted via the respective flag, but putting 8 includes all the utilities that have tests to this date thus consisting in maximum concurrency.

Running the tests this way cuts the time almost by 66%, however the experience is not great imo because the logs are all mixed (see below) and since they are streamed things like progress bars won't work but instead generate one line for each update:

@aws-lambda-powertools/metrics: arn:aws:cloudformation:eu-west-1:12345678901:stack/Metrics-18-x86-77c72-BasicFeatures-Decorators/0c06b3f0-6c0b-11ee-8eaf-02b78ef81927
@aws-lambda-powertools/idempotency:  ✅  Idempotency-18-x86-12cfb-makeHandlerIdempoten
@aws-lambda-powertools/tracer: arn:aws:cloudformation:eu-west-1: 12345678901:stack/Tracer-18-x86-a6e80-AllFeatures-AsyncDecorato/262b5c40-6c0b-11ee-9da3-0acf4e5b4307

Additionally, since we must use the --no-bail flag to ensure that test lifecycle to completes and the CloudFormation stacks are torn down, in case of failing tests the error messages can easily get lost in the stream and make the output useless (see below):

lerna ERR! Received non-zero exit code 1 during execution
lerna success exec Executed command in 11 packages: "npm run test:e2e"

However if you want a quick way of running all the tests and are reasonably confident that they will pass (aka you don't have to consume the logs), then this is the way to do it.

Both methods are now documented in the updated maintainers playbook that is being added in the linked PR.

Note

Below are some considerations on an alternative tool that I have been investigating but that I'm not yet set on. If you're not interested in reading about it, you can stop here.

Investigating other tools

I have been looking at several tools used in monorepos and trying some of them. One of them, called wireit stood out for now, but there are others that I want to try before settling on one.

I have tried wireit in a personal project that uses a monorepo with 3 workspaces and it looks promising, especially when it comes to caching and parallelizing scripts, however it requires a significant lift in terms of configuring and maintaining the wiring config.

For example, imagine you want to have some shared npm scripts that live in the main package.json file at the project's root, and then you have some scripts that are specific to some workspaces (i.e. frontend/package.json). Wireit allows you to specify dependencies between scripts (i.e. deploy requires build, etc.) and automatically handles parallelism whenever it's safe to do so.

flowchart LR
    npm-run-frontend:deploy-->deploy
    build-->exportCDKoutputs
    subgraph frontend
    deploy-->build
    end
    subgraph root
    exportCDKoutputs
    end
Loading

Blow an excerpt of the root package.json from the project I was mentioning that expresses the tree shown above:

{
  "scripts": {
    "exportCDKoutputs": "wireit"
  },
  "wireit": {
    "exportCDKoutputs": {
      "command": "ts-node ./scripts/exportCDKoutputs.ts",
      "files": [
        "./infrastructure/cdk.out/params.json",
        ".env"
      ],
      "output": [
        ".env"
      ],
      "clean": false
    },
  }
}

and this is another from the frontend/package.json file:

{
  "scripts": {
    "build": "rimraf dist && mkdir dist && sh build.sh",
    "deploy": "wireit"
  },
  "wireit": {
    "deploy": {
      "command": "export $(cat ../.env | xargs) && aws s3 sync dist s3://$WEB_STATIC_ASSETS_BUCKET_NAME --delete",
      "dependencies": [
        "../:exportCDKoutputs",
        "build"
      ],
      "WEB_STATIC_ASSETS_BUCKET_NAME": {
        "external": true
      }
    }
  }
}

Another interesting aspect of wireit is the caching, as you can see it allows you to specify files & output fields. These are used by the tool to determine whether the script should be run or it's safe to skip (aka return output from cache).

The reason why I'm still not sure about it however is the complexity of setting this up. The above scripts express one relationship between 3 scripts in one workspace, and as you can see it's very verbose and somewhat hard to reason about. Additionally, the caching features are enabled by default and it's very easy to shoot yourself in the foot if you don't know exactly what artefacts are being generated and how they are used by all the scripts in the dependency tree.

Nevertheless, I think it's something we could potentially consider in the future, but at the time I don't think we have enough bandwidth to focus on this, especially because the potential time savings are still unclear.

@github-actions
Copy link
Contributor

⚠️ 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.

@dreamorosi dreamorosi added completed This item is complete and has been merged/shipped and removed confirmed The scope is clear, ready for implementation labels Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automation This item relates to automation completed This item is complete and has been merged/shipped internal PRs that introduce changes in governance, tech debt and chores (linting setup, baseline, etc.)
Projects
Development

Successfully merging a pull request may close this issue.

2 participants