amplify-cli icon indicating copy to clipboard operation
amplify-cli copied to clipboard

Use build script to build custom resources in build-custom-resources.ts

Open acusti opened this issue 2 years ago • 1 comments
trafficstars

Description of changes

instead of invoking node_modules/.bin/tsc directly to build custom resources (via path.join(targetDir, 'node_modules', '.bin', 'tsc')), we should instead use the defined build script in the resource’s package.json. this script is automatically created when a custom resource is added with amplify add custom.

using ${packageManager.executable} run build invokes the build script using the current package manager and works no matter what kind of package manager configuration is currently set up (e.g. with PnP). resolves #11889.

Issue #, if available

#11889

Description of how you validated changes

i verified that npm run build and yarn run build both work to build the cdk-stack.ts file into build/cdk-stack.js in the same way as executing the node_modules/.bin/tsc path works. i then used amplify-dev build and amplify-dev push locally in my repo where i first ran into #11851 and my custom resource built successfully and deployed with no errors.

Checklist

  • [x] PR description included
  • [x] yarn test passes

note about the tests that not all of the tests actually passed, but i believe all the failures are 100% unrelated to my changes. there were a few tests that failed with Exceeded timeout errors (e.g. should attempt setting up local instance of opensearch with custom configuration, which timed our at 90224 ms!), and the amplify-dynamodb-simulator:test command, which i believe failed because i don’t have the correct java setup (max retries hit for starting dynamodb emulator) and to be honest, i’d rather not deal with that. as a general note, running yarn test made my M1 MacBook pro unusable while they were running, causing the macOS “Your system has run out of application memory” dialog box to show up, despite me only having my terminal and safari open, so i’m not surprised that there were some timeouts. here was the final log result:

 >  NX   Ran target test for 28 projects and 40 task(s) they depend on (1h)
    ✔    64/68 succeeded [49 read from cache]

    ✖    4/68 targets failed, including the following:
         - nx run amplify-dynamodb-simulator:test
         - nx run @aws-amplify/amplify-opensearch-simulator:test
         - nx run @aws-amplify/amplify-appsync-simulator:test
         - nx run @aws-amplify/cli-internal:test

the test that failed the @aws-amplify/cli-internal:test command is src/__tests__/test-aborting.test.ts:

       Summary of all failing tests
        FAIL  src/__tests__/test-aborting.test.ts (56.958 s, 1206 MB heap size)
         ● test SIGINT with execute › case: run

           thrown: "Exceeded timeout of 5000 ms for a test.
           Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

              6 |   });
              7 |
           >  8 |   it('case: run', async () => {
                |   ^
              9 |     const input = { argv: ['/usr/local/bin/node', '/usr/local/bin/amplify-dev', '-v'], options: { v: true } };
             10 |     const mockExit = jest.fn();
             11 |

             at src/__tests__/test-aborting.test.ts:8:3
             at Object.<anonymous> (src/__tests__/test-aborting.test.ts:3:1)

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

acusti avatar Jan 26 '23 00:01 acusti

Codecov Report

Merging #11854 (5b5ab24) into dev (ec9a2ba) will increase coverage by 47.59%. The diff coverage is 45.83%.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@             Coverage Diff             @@
##              dev   #11854       +/-   ##
===========================================
+ Coverage    0.00%   47.59%   +47.59%     
===========================================
  Files        1296      842      -454     
  Lines      149743    38320   -111423     
  Branches     1296     7834     +6538     
===========================================
+ Hits            0    18239    +18239     
+ Misses     148447    18458   -129989     
- Partials     1296     1623      +327     
Impacted Files Coverage Δ
...ify-category-function/src/events/prePushHandler.ts 33.33% <0.00%> (+33.33%) :arrow_up:
...ib/S3AndCloudFront/helpers/configure-CloudFront.js 87.06% <ø> (+87.06%) :arrow_up:
...lify-category-hosting/lib/S3AndCloudFront/index.js 89.65% <ø> (+89.65%) :arrow_up:
...s/amplify-cli-core/src/errors/amplify-exception.ts 82.35% <ø> (+82.35%) :arrow_up:
...ackages/amplify-cli-core/src/help/commands-info.ts 100.00% <ø> (+100.00%) :arrow_up:
packages/amplify-cli-core/src/types.ts 100.00% <ø> (+100.00%) :arrow_up:
packages/amplify-cli/src/commands/console.ts 0.00% <0.00%> (ø)
packages/amplify-cli/src/commands/pull.ts 0.00% <0.00%> (ø)
...es/amplify-provider-awscloudformation/src/index.ts 60.20% <ø> (+60.20%) :arrow_up:
...c/extensions/amplify-helpers/auth-notifications.ts 41.00% <4.34%> (+41.00%) :arrow_up:
... and 24 more

... and 1271 files with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

codecov-commenter avatar May 23 '23 18:05 codecov-commenter