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

chore: upgrade jest to 29 #2747

Merged
merged 26 commits into from
Aug 30, 2024
Merged

chore: upgrade jest to 29 #2747

merged 26 commits into from
Aug 30, 2024

Conversation

dpilch
Copy link
Member

@dpilch dpilch commented Aug 7, 2024

Description of changes

Upgrade Jest to 29. This is necessary to upgrade amplify-backend packages to GA versions.

  • Update Jest config to account for breaking changes in order to keep same functionality.
  • Lower some coverage thresholds that are reported differently with new version of jest.
  • Move jest.setTimeout out of beforeAll jest.setTimeout() is ignored if run async'ly in beforeAll() hook jestjs/jest#9359
  • Increase codebuild e2e test instance sizes. Set max workers to 5.
    • Jest has a memory issue when used with node version <= 18. [Bug]: Memory consumption issues on Node JS 16.11.0+ jestjs/jest#11956. We will need to upgrade to Node 20 soon.
    • I saw each test suite ranging between low 2.x GB and low 3.x GB. With 5 test suites per instance, tests are now around 80% memory consumption. It might be possible to set max workers to 6, but the instances would potentially be less stable.
    • Multi test instances are using BUILD_GENERAL1_LARGE and single test instances are using BUILD_GENERAL1_MEDIUM.
    • Instance resources
    • Instance cost
CDK / CloudFormation Parameters Changed

N/A

Issue #, if available

#2741

Description of how you validated changes

  • Unit test
  • E2E test

Checklist

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@dpilch dpilch requested a review from a team as a code owner August 7, 2024 16:06
Base automatically changed from aws-sdk-upgrade to main August 8, 2024 21:54
@palpatim palpatim requested a review from a team as a code owner August 8, 2024 21:54
stocaaro
stocaaro previously approved these changes Aug 12, 2024
Copy link
Member

@stocaaro stocaaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

I noted the timeout increase, which raises questions that are probably answerable, more than need changes.

@@ -4,7 +4,7 @@ import { sleep } from './sleep';
const defaultSettings: RetrySettings = {
times: Infinity,
delayMS: 1000 * 10, // 10 seconds
timeoutMS: 1000 * 60 * 20, // 20 minutes
timeoutMS: 1000 * 60 * 25, // 25 minutes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What did you encounter that caused the bump from 20 to 25 minutes on the timeout?

I would guess this means our e2e tests slowed down by ~25%, which might be enough to be worth an investigation. If you have already done the investigation, whats driving the increase and can we do better?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a performance degradation with newer versions of Jest when run with node < 20 (jestjs/jest#11956). We currently support Node 18. Node 18 will be EOL in 8 months, so we should look into upgrading to Node 20.

@dpilch dpilch marked this pull request as draft August 12, 2024 14:57
@dpilch dpilch force-pushed the jest-upgrade branch 2 times, most recently from 0baca8c to 9d262d6 Compare August 19, 2024 21:29
@dpilch dpilch force-pushed the jest-upgrade branch 8 times, most recently from 0abd8c3 to 9d36991 Compare August 27, 2024 16:41
@@ -6,7 +6,6 @@ env:
AMPLIFY_PATH: /root/.npm-global/lib/node_modules/@aws-amplify/cli-internal/bin/amplify
CI: true
CODEBUILD: true
NODE_OPTIONS: --max-old-space-size=8096
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were ignored.

@dpilch dpilch marked this pull request as ready for review August 29, 2024 16:54
scripts/split-e2e-tests.ts Outdated Show resolved Hide resolved
scripts/split-e2e-tests.ts Show resolved Hide resolved
@dpilch dpilch merged commit 38de325 into main Aug 30, 2024
6 checks passed
@dpilch dpilch deleted the jest-upgrade branch August 30, 2024 23:17
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

Successfully merging this pull request may close these issues.

4 participants