test: add deploy test #901

0618 · 2024-01-10T21:26:16Z

Problem

Move #891 to here because IAM role can't configure credentials on a test from fork.

add e2e-flow test, which is a copy of test-e2e/create-amplify.test
add deploy test to create-amplify script
make the deploy test work

Issue number, if available:

Changes

Corresponding docs PR, if applicable:

Validation

Checklist

If this PR includes a functional change to the runtime behavior of the code, I have added or updated automated test coverage for this change.
If this PR requires a change to the Project Architecture README, I have included that update in this PR.
If this PR requires a docs update, I have linked to that docs PR above.
If this PR modifies E2E tests, makes changes to resource provisioning, or makes SDK calls, I have run the PR checks with the run-e2e label set.

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

This reverts commit 9f85439.

changeset-bot · 2024-01-10T21:26:19Z

🦋 Changeset detected

Latest commit: 4c4069b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages

Name	Type
@aws-amplify/backend-deployer	Patch
@aws-amplify/backend-cli	Patch
@aws-amplify/sandbox	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

This reverts commit 4950b7d.

sobolk · 2024-01-18T16:18:40Z

packages/backend-deployer/src/cdk_deployer.ts

+    private readonly packageManager: string
+  ) {
+    this.packageManager =
+      this.packageManager === 'npm' ? 'npx' : this.packageManager;
+  }


A note for future follow up when we plumb this from CLI.

The deployer only cares about "package manager executable". So perhaps this resolution should happen in CLI.

Or, perhaps we should start thinking about moving package manager abstraction (controller?) to platform-core and have PackageManagerController injected here from CLI?

Yea, we should move the abstruction to platform-core. I'll do it in a follow-up PR.

added a comment d33f115

sobolk · 2024-01-18T16:20:01Z

...ages/create-amplify/src/package-manager-controller/yarn_modern_package_manager_controller.ts

+    await this.addDependencies(this.projectRoot);
+    await super.installDependencies(packageNames, type);


nice :-)
This is why we invented that abstraction.

sobolk · 2024-01-18T16:22:04Z

packages/integration-tests/src/process-controller/process_controller.ts

+/**
+ *  packageManagerExecute - Factory function that returns a ProcessController for the specified package manager
+ */
+export const packageManagerExecute = (


Suggested change

export const packageManagerExecute = (

export const runWithPackageManager = (

done d33f115

sobolk · 2024-01-18T16:23:37Z

packages/integration-tests/src/process-controller/process_controller.ts

+  const command = execaSync('npx', ['which', packageManagerExecutable], {
+    cwd: dir,
+  }).stdout.trim();
+  if (!command) {
+    throw new Error('Unable to locate packageManager binary');
+  }


that doesn't seem to be used.

done d33f115

sobolk · 2024-01-18T16:24:01Z

packages/integration-tests/src/process-controller/process_controller.ts

+  const command = execaSync('npx', ['which', packageManagerExecutable], {
+    cwd: dir,
+  }).stdout.trim();
+  if (!command) {
+    throw new Error('Unable to locate packageManager binary');
+  }


we can remove this too.

done d33f115

sobolk · 2024-01-18T16:25:39Z

packages/integration-tests/src/process-controller/process_controller.ts

+/**
+ * packageManagerCli - Factory function that returns a ProcessController for the specified package manager
+ */
+export const packageManagerCli = (


Suggested change

export const packageManagerCli = (

export const runPackageManager = (

packageManagerCli is consistent with amplifyCLI and cdkCli though

done d33f115

sobolk · 2024-01-18T23:52:07Z

packages/backend-deployer/src/cdk_deployer.ts

+    private readonly backendLocator: BackendLocator,
+    private readonly packageManager: string
+  ) {
+    // TODO: use PackageManagerController to get the executable.


I wonder if it's going to be get the executable or perhaps packageManagerController.executeWithPackageManager(command, args).
e.g. packageManagerController.executeWithPackageManager('tsc', tscArgs);

That sounds a good idea 👍

edwardfoyle

Left some nits and a couple comments that should be addressed, but I'm fine if they go in a follow up PR

edwardfoyle · 2024-01-19T18:15:53Z

.changeset/honest-pumas-fold.md

Just a suggestion for future reviews on feature branches: I think it's fine to merge main directly into the feature branch without a PR, then make PRs on top of those merges. This cuts down on PR diffs that are from changes that have already been merged to main. This will also make your life easier when merging the feature branch back to main as it will greatly reduce the number of merge conflicts.

edwardfoyle · 2024-01-19T18:23:52Z

packages/integration-tests/src/package_manager_sanity_checks.test.ts

+ * TODO: This test will be refactored to use the test from health-checks.test.ts and run with different package managers.
+ */
+
+void describe('create-amplify and pipeline deploy', async () => {


nit: perhaps describe('getting started happy path')

Done add3437

edwardfoyle · 2024-01-19T18:26:31Z

packages/integration-tests/src/package_manager_sanity_checks.test.ts

+    await execa('npm', ['run', 'clean:npm-proxy'], { stdio: 'inherit' });
+    await execa('npm', ['run', 'vend'], { stdio: 'inherit' });


Doesn't matter yet since there's only one test here, but starting/stopping the npm proxy should probably go in before() and after() rather than beforeEach() and afterEach()

Done add3437

edwardfoyle · 2024-01-19T18:30:17Z

packages/integration-tests/src/process-controller/process_controller.ts

+/**
+ *  runWithPackageManager - Factory function that returns a ProcessController for the specified package manager
+ */


The difference between this function and the one below is very subtle. I'd fill out these JSDocs a little more to explain the difference

Done 4c4069b

edwardfoyle · 2024-01-19T18:32:12Z

packages/integration-tests/src/setup_package_manager.ts

+  await execa('npm', ['install', '-g', packageManager], {
+    stdio: 'inherit',
+  });
+  await execa(packageManager, ['--version']);


shouldn't we be using runPackageManager for these commands?

Not needed since packageManager = 'pnpm' in this case

updated after approval

* feat: create amplify skip prompts for `--yes` option (#507) * feat: add env ci support * test: update unit tests * feat: create amplify --yes option * chore: remove un-used code * feat: support Panage Manager env var * chore: add comments * chore: remove env.CI * chore: update changeset * chore: rename PACKAGE_MANAGER_EXECUTABLE * fix: tests * fix npm -> npx * feat: init e2e test workflow for Package Manager Support (#533) * test: init e2e flow test * fix GH hash * use dynamic pkg manager * setup nodejs with npm * setup more pkg managers * install pkg managers * fix yarn1 and change step name * fix yarn1 * fix yarn1 * split yarn1 into 2 steps * fix yarn1 * fix yarn1 win * exclude bun on windows * yarn1 windows * setup yarn3 * yarn3 * yarn3 yarnPath * yarn use 3rd party action * try yarn3 * set yarn 3.6.x * set yarn berry * set yarn stable and pass --yes * feat: add env ci support * test: update unit tests * feat: create amplify --yes option * chore: remove un-used code * feat: support Panage Manager env var * chore: add comments * chore: remove env.CI * chore: update changeset * chore: rename PACKAGE_MANAGER_EXECUTABLE * fix: tests * use Env Var PACKAGE_MANAGER_EXECUTABLE * fix npm -> npx, remove bun * fix npm -> npx * change -- --yes * chore: update package.lock * chore: remove @Alpha * fix: clean up yarn * fix: change yarn1 to yarn * chore: rename yarn step * chore: use env * fix: yarn-stable env var * add comments * chore: update sample app with new imports (#541) * feat: use "use client"; directive in generated react components (#540) * fix: flatten prop types auth (#534) * fix: flatten auth login with types * chore: update snapshots * chore: update api * chore: add changeset * chore: add TODO --------- Co-authored-by: Amplifiyer <51211245+Amplifiyer@users.noreply.github.com> Co-authored-by: Spencer Stolworthy <sstol@amazon.com> Co-authored-by: awsluja <110861985+awsluja@users.noreply.github.com> * fix: pnpm init and env var (#563) * fix: npm_project_initializer to use env var * fix: pnpm init * chore: update package.lock * chore: update package.lock * test: use GH workflow for e2e test of create amplify (#636) * feat: setup GH workflow for create amplify * chore: changeset * fix: add pkg type for modern yarn * fix npm install * feat: test Node v20 and Node v19 * feat: test Node v20 and yarn-stable with Node 19 * feat: exclude yarn-stable * feat: include node 20 and exclude node 19 * feat: use node 20 for others, use node 19 for yarn-stable * chore: add TODO to use Node 20 * fix: e2e tests (#771) * chore: change branch to poc/pms-create-amplify * fix: gitIgnore test * fix: refactor e2e for pms * chore: update package.lock * chore: update package.lock * temp: refactor * temp: run 1 test * temp: fix npx * temp: install packages for yarn * temp: update initial_project_file_generator * temp: install ts for yarn * temp: ignore node_modules and yarn.lock * temp: create yarn.lock for yarn stable * temp: fix yarn-stable init * temp: fix --help * fix: assert for gitignore * chore: change yarn stable install * fix: not install yarn stable globally * add @yarnpkg/sdks base * yarn stable use node-modules * remove emoji * remove @yarnpkg/sdks * yarn stable use node 18.18 * yarn stable use node 20 * chore: re-enable all tests * Revert "chore: re-enable all tests" This reverts commit f7a167b. * chore: uncomment test * chore: enable all initialStatues * test: change concurrencyLevel for yarn and yarn stable * add yarn-stable to test fails fast * chore: update changeset * chore: change workflow trigger * chore: yarn not install typescript in root folder * chore: update package.lock * Revert "chore: yarn not install typescript in root folder" This reverts commit 0d83dd0. * chore: remove comment * chore: fix typo * chore: rename yarn-classic and yarn modern * fix(create_amplify.test): after merge main * fix the concurrency for yarn classic and modern, and fix expect * feat: package manager support create amplify (#793) * chore: update package.lock * init * feat: dynamically get package manager * chore: change workflow trigger * chore: change workflow trigger * chore: rename npm functions * chore: update output log * fix: pnpm cache clear command * chore: refactor packageManager * fix: remove pnpm store * chore: create package_manager file * chore: change workflow trigger * chore: refactor PackageManager Controller and Initializer * fix: update types * chore: refactor to use factory * chore: remove try catch from the controllers * chore: change type * chore: use abstract class * fix tests * fix: PackageManagerBase * chore: refactor PackageManagerControllerFactory * chore: refactor packageJsonExists * chore: cleanup code * fix: projectJsonExists * remove package_manager file * chore: refactor package_manager props into package manager controllers * chore: rename packageManagerControllerFactory * chore: make projectRoot, userAgent, getPackageManagerName private * chore: refactor ensureInitialized * chore: refactor PackageManagerController to extend PackageManagerControllerFactory * Revert "chore: refactor PackageManagerController to extend PackageManagerControllerFactory" This reverts commit 7ddc64f. * chore: refactor to inject PackageManagerControllerFactory to xPackageManagerController * chore: move initialProjectFileGenerator to packageManagerController * fix: initializeAMplifyFolder * fix: generateInitialProjectFiles * fix: template path * chore: refactor getPackageManagerController yarn-modern * chore: create getWelcomeMessage * Revert "chore: refactor getPackageManagerController yarn-modern" This reverts commit 28da8a9. * chore: refactor generateInitialProjectFiles for yarn-modern * chore: cleanup code * chore: remove PackageManagerControllerFactory index * chore: remove ensureInitialized * Revert "chore: remove ensureInitialized" This reverts commit 96483ce. * Revert "Revert "chore: remove ensureInitialized"" This reverts commit 7c59c80. * temp: add initializeProject to PackageManagerController * temp: revert InitialProjectFileGenerator * fix: InitialProjectFileGenerator * test: restore initial_project_file_generator * move installDependencies and getWelcomeMessage to PackageManagerController * add addLockFile * add JSDocs and resolve some review comments * fix: create_amplify.test * chore: add addTypescript * chore: refactor generateInitialProjectFiles * Update packages/create-amplify/src/package-manager-controller/package_manager_controller_factory.ts Co-authored-by: Edward Foyle <foyleef@amazon.com> * chore: refactor generateInitialProjectFiles again * chore: refactor welcomeMessage * test: add package_manager_controller_factory * test: fix test types * chore: comments update * chore: handle process.env.npm_config_user_agent undefined * chore: move addLockFile and addTypescript into initializeTsConfig * test: refactor packageManagerControllerFactory * fix: yarn initializeTsConfig * fix: yarn modern initializeTsConfig * fix: amplify_project_creator test * chore: refactor contructor * chore: update package.lock * fix: pnpm init, remove --debug, etc * test: add test for NpmPackageManagerController * fix build error * chore: refactor NpmPackageManagerController * test: add test for xPackageManagerController * chore: convert fs to fsp * fix yarn modern --------- Co-authored-by: Edward Foyle <foyleef@amazon.com> * chore: update package-lock * test: add deploy test (#901) * chore: add e2e_flow.test * add deploy test but don't work * comment out deploy test * fix: e2e test assert * chore: update package.lock * add deploy test * use before and after * change amplifyCli to execa * change test trigger * Revert "change amplifyCli to execa" This reverts commit 9f85439. * setupProfile * Revert "Revert "change amplifyCli to execa"" This reverts commit 4950b7d. * remove fail test and initialStates * install @aws-amplify/backend-deployer * chore: update changeset * chore: set nodeLinker in test project * fix execaOptions syntax error * fix: npx in cdk deployer * chore: update changeset * fix: yarn modern build * chore: update package-lock * chore: remove some asserts * chore: remove synth * install packages to fix yarn * remove create-amplify help * simplify before and after * remove one packageManagerSetup * refactor packageManagerSetup * fix yarn- * move yarn add to setupPackageManager * fix type error * Revert "fix type error" and "move yarn add to setupPackageManager" This reverts commit d88ef61. * try fix pnpm for windows by adding @AWS-SDK+credential-providers * remove the pnpm patch * add nodir to dictionary * move setupPackageManager to a new file * rename PACKAGE_MANAGER_EXECUTABLE to PACKAGE_MANAGER * refactor packageManagerSetup to use switch * refactor packageManagerSetup to initializeX * use beforeEach and afterEach * add TODO comment * temp * simplify yarn-modern treatments * misc changes * add TODO comments * exclude pnpm on windows * Revert "exclude pnpm on windows" This reverts commit dd7b423. * exclude pnpm on windows * resolve review comments * fix syntax error * address review comments * move this.packageManager logic to ctor * fix yarn-modern * add packageManagerCli and packageManagerExecutable * fix yarn * address review comments * rename package_manager_sanity_check * update workflow yml * chore: update package-lock * move npm proxy to before and after * change comments * feat(backend-deploy): use packageManagerController (#947) * fix logger * fix logLevel * change workflow trigger to include poc/pms-cli-core * move package_manager_controller to cli-core * chore: update changeset * move npm_config_user_agent to getPackageManagerName and add runWithPackageManager * fix build error * modify executeWithDebugLogger * add getPackageManagerCommandArgs * refactor cdk_deployer to use package manager * install cli-core * fix execaChildProcess type * use PackageManagerControllerFactory in cdkDeployer * fix amplify/amplify path * chore: update API.md * clean up package-manager-controller export * update API.md * add type PackageManagerController * use PackageManagerController type from plugin-types * rename PackageManagerControllerBase and use plugin-types * change workflow trigger to exclude poc/pms-cli-core * refactor BackendDeployerFactory to take PackageManagerController * clean up getPackageManagerCommandArgs * add projectRoot and rename packageManagerControllerBase file * set projectRoot default value * rename execute_with_debugger_logger * use execa Options * Revert "use execa Options" This reverts commit 30cd61e. * rename executeChildProcessWithPackageManager to executeCommand * make BackendDeployerFactory param mandatory * make BackendDeployerFactory param mandatory * remove DependencyType * remove @aws-amplify/platform-core from cli-core * remove some default projectRoot * remove all default projectRoot * inject printer to factory * update package-lock * fix cli-core version in backend-deployer * move @aws-amplify/cli-core to devDep * add getCommand * change execa option type * change projectRoot to private * rename projectRoot to cwd * change ./ to cwd * remove @aws-amplify/cli-core from backend-deployer * patch dependencies_validator for execa * fix getCommand, cdk_deployer.test, amplify_project_creator.test * fix initial_project_file_generator.test * fix executeWithDebugLogger test * fix YarnModernPackageManagerController test * fix packageManagerControllerFactory test * fix lint error * Update packages/cli-core/src/package-manager-controller/package_manager_controller_factory.ts Co-authored-by: Kamil Sobol <sobkamil@amazon.com> * remove never used packageManagerExecutable * update changeset * feat: add Package Manager test to health_checks (#968) * add package-manager-tests to health-checks * remove poc-e2e-flow-test * add run_package_manager_tests_tests condition * fix run_package_manager_tests_tests condition * remove amplify-backend_windows-latest_8-core * use re-usable action for run-e2e * Revert "remove amplify-backend_windows-latest_8-core" This reverts commit 547834a. * Revert "use re-usable action for run-e2e" This reverts commit 3159e01. * revert re-usable action * Update .changeset/new-timers-warn.md Co-authored-by: Kamil Sobol <sobkamil@amazon.com> * clean up commented code * use amplify-backend_windows-latest_8-core * Update .changeset/new-timers-warn.md Co-authored-by: Edward Foyle <foyleef@amazon.com> * remove inaccurate JSdoc * remove integration-tests from changeset --------- Co-authored-by: Amplifiyer <51211245+Amplifiyer@users.noreply.github.com> Co-authored-by: Spencer Stolworthy <sstol@amazon.com> Co-authored-by: awsluja <110861985+awsluja@users.noreply.github.com> Co-authored-by: Edward Foyle <foyleef@amazon.com> Co-authored-by: Kamil Sobol <sobkamil@amazon.com>

0618 added 12 commits January 8, 2024 19:14

chore: add e2e_flow.test

7806583

add deploy test but don't work

5ae2da1

Merge branch 'poc/package-manager-support' into poc/pms-deploy-v2

2a6a861

comment out deploy test

d3cf45b

fix: e2e test assert

590a32c

chore: update package.lock

5ef40d1

add deploy test

93689a0

use before and after

b083c3c

change amplifyCli to execa

9f85439

change test trigger

aa3e912

Revert "change amplifyCli to execa"

4950b7d

This reverts commit 9f85439.

setupProfile

212c8c1

0618 requested review from edwardfoyle, sobolk, Amplifiyer and srquinn21 as code owners January 10, 2024 21:26

0618 mentioned this pull request Jan 10, 2024

test: add deploy test #891

Closed

5 tasks

0618 added 12 commits January 10, 2024 14:07

Revert "Revert "change amplifyCli to execa""

24e75f6

This reverts commit 4950b7d.

remove fail test and initialStates

8dfe98f

install @aws-amplify/backend-deployer

e1a8cad

Merge branch 'poc/package-manager-support' into poc/pms-deploy-v2

1429e06

chore: update changeset

11284e2

chore: set nodeLinker in test project

08f1316

fix execaOptions syntax error

403b21d

fix: npx in cdk deployer

3abdc40

chore: update changeset

e1bca3b

fix: yarn modern build

80e6fb0

Merge branch 'poc/package-manager-support' into poc/pms-deploy-v2

1402090

chore: update package-lock

2bd018c

0618 added 6 commits January 17, 2024 18:45

fix syntax error

318141d

address review comments

60d33bc

move this.packageManager logic to ctor

e13058e

fix yarn-modern

038ac85

add packageManagerCli and packageManagerExecutable

222249a

fix yarn

7b2b0f6

sobolk reviewed Jan 18, 2024

View reviewed changes

0618 added 2 commits January 18, 2024 09:48

address review comments

d33f115

rename package_manager_sanity_check

8e6cc14

sobolk approved these changes Jan 18, 2024

View reviewed changes

update workflow yml

18b72f9

sobolk reviewed Jan 18, 2024

View reviewed changes

0618 added 2 commits January 18, 2024 16:32

Merge branch 'poc/package-manager-support' into poc/pms-deploy-v2

89a6f32

chore: update package-lock

3ba4b8d

edwardfoyle previously approved these changes Jan 19, 2024

View reviewed changes

0618 added 2 commits January 19, 2024 12:58

move npm proxy to before and after

add3437

change comments

4c4069b

edwardfoyle approved these changes Jan 19, 2024

View reviewed changes

0618 merged commit ff9c6aa into poc/package-manager-support Jan 19, 2024
15 checks passed

0618 deleted the poc/pms-deploy-v2 branch January 19, 2024 21:44

This was referenced Jan 23, 2024

test(package-manager-support): combine package manager checks with health checks #941

Closed

feat(backend-deploy): use packageManagerController #947

Merged

0618 mentioned this pull request Jan 30, 2024

feat: package manager support #960

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: add deploy test #901

test: add deploy test #901

0618 commented Jan 10, 2024 •

edited

Loading

changeset-bot bot commented Jan 10, 2024 •

edited

Loading

sobolk Jan 18, 2024

0618 Jan 18, 2024

0618 Jan 18, 2024

sobolk Jan 18, 2024

sobolk Jan 18, 2024 •

edited

Loading

0618 Jan 18, 2024

sobolk Jan 18, 2024

0618 Jan 18, 2024

sobolk Jan 18, 2024

0618 Jan 18, 2024

sobolk Jan 18, 2024

0618 Jan 18, 2024

0618 Jan 18, 2024

sobolk Jan 18, 2024

0618 Jan 19, 2024

edwardfoyle left a comment

edwardfoyle Jan 19, 2024

edwardfoyle Jan 19, 2024

0618 Jan 19, 2024

edwardfoyle Jan 19, 2024

0618 Jan 19, 2024

edwardfoyle Jan 19, 2024

0618 Jan 19, 2024

edwardfoyle Jan 19, 2024

0618 Jan 19, 2024

		await this.addDependencies(this.projectRoot);
		await super.installDependencies(packageNames, type);

	export const packageManagerExecute = (
	export const runWithPackageManager = (

	export const packageManagerCli = (
	export const runPackageManager = (

		await execa('npm', ['run', 'clean:npm-proxy'], { stdio: 'inherit' });
		await execa('npm', ['run', 'vend'], { stdio: 'inherit' });

test: add deploy test #901

test: add deploy test #901

Conversation

0618 commented Jan 10, 2024 • edited Loading

Problem

Changes

Validation

Checklist

changeset-bot bot commented Jan 10, 2024 • edited Loading

🦋 Changeset detected

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sobolk Jan 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edwardfoyle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0618 commented Jan 10, 2024 •

edited

Loading

changeset-bot bot commented Jan 10, 2024 •

edited

Loading

sobolk Jan 18, 2024 •

edited

Loading