test: add deploy test #891
Conversation
0618
requested review from
edwardfoyle,
sobolk,
Amplifiyer and
srquinn21
as code owners
|
| shell: bash | ||
| run: | | ||
| PACKAGE_MANAGER_EXECUTABLE=${{matrix.pkg-manager}} npm run test:dir packages/integration-tests/src/test-e2e/create_amplify.test.ts | ||
| PACKAGE_MANAGER_EXECUTABLE=${{matrix.pkg-manager}} npm run test:dir packages/integration-tests/src/e2e-flow/e2e_flow.test.ts |
There was a problem hiding this comment.
If we're going for separate test file in separate dir (i.e. not included in e2e run)
then perhaps we could just use this one https://github.com/aws-amplify/amplify-backend/blob/main/packages/integration-tests/src/test-live-dependency-health-checks/health_checks.test.ts
but adapt it to take package manager as input.
| import { testConcurrencyLevel } from '../test-e2e/test_concurrency.js'; | ||
| import { e2eToolingClientConfig } from '../e2e_tooling_client_config.js'; | ||
|
|
||
| type PackageManagerExecutable = 'npm' | 'yarn-classic' | 'yarn-modern' | 'pnpm'; |
There was a problem hiding this comment.
| type PackageManagerExecutable = 'npm' | 'yarn-classic' | 'yarn-modern' | 'pnpm'; | |
| type PackageManager = 'npm' | 'yarn-classic' | 'yarn-modern' | 'pnpm'; |
This is not an executable. We derive executable from this enumeration later.
| const concurrency = process.env.PACKAGE_MANAGER_EXECUTABLE?.startsWith('yarn') | ||
| ? 1 | ||
| : testConcurrencyLevel; | ||
|
|
||
| const packageManagerSetup = async ( | ||
| packageManagerExecutable: PackageManagerExecutable, | ||
| dir?: string | ||
| ) => { | ||
| const execaOptions = { | ||
| cwd: dir || os.homedir(), | ||
| stdio: 'inherit' as const, | ||
| }; |
There was a problem hiding this comment.
Could we refactor the code in such a way that process.env.PACKAGE_MANAGER_EXECUTABLE is read only once somewhere?
Options:
- Do that in
packageManagerSetupand havepackageManagerSetupreturn a type that provides all what test needs for further execution i.e.type PackageManagerTestConfiguration = { executable: string; maxConcurrency?: number } - Create
TestPackageManagerControllerFactoryand haveTestPackageManagerControllerand subclasses (like we did in create amplify) to encapsulate functionalities.
(I think 1. is good enough for tests).
Also. If this stays as packageManagerSetup then it should be renamed to setupPackageManager and perhaps live in its own file given volume.
| const { PACKAGE_MANAGER_EXECUTABLE = 'npm' } = process.env; | ||
| const packageManagerExecutable = PACKAGE_MANAGER_EXECUTABLE.startsWith('yarn') | ||
| ? 'yarn' | ||
| : PACKAGE_MANAGER_EXECUTABLE; |
There was a problem hiding this comment.
this part should be in packageManagerSetup (see also the other comment)
| // install package manager | ||
| if (PACKAGE_MANAGER_EXECUTABLE === 'yarn-classic') { | ||
| await execa('npm', ['install', '-g', 'yarn'], { stdio: 'inherit' }); | ||
| } else if (PACKAGE_MANAGER_EXECUTABLE === 'pnpm') { | ||
| await execa('npm', ['install', '-g', PACKAGE_MANAGER_EXECUTABLE], { | ||
| stdio: 'inherit', | ||
| }); | ||
| } | ||
|
|
||
| // nuke the npx cache to ensure we are installing packages from the npm proxy | ||
| if (PACKAGE_MANAGER_EXECUTABLE !== 'yarn-modern') { | ||
| await packageManagerSetup( | ||
| PACKAGE_MANAGER_EXECUTABLE as PackageManagerExecutable | ||
| ); | ||
| } |
There was a problem hiding this comment.
this too should be part of setup package manager.
| const initialStates = ['empty', 'module', 'commonjs'] as const; | ||
|
|
||
| initialStates.forEach((initialState) => { | ||
| void describe('installs expected packages and scaffolds expected files', () => { |
There was a problem hiding this comment.
we shouldn't repeat tests from create amplify if we're creating new file.
i.e. we should start with single test that does full create-amplify -> deployment flow here.
which means that we should:
- remove initialStates, and just start from
empty - remove most of assertions that repeat create amplify test, i.e. it should be similar to https://github.com/aws-amplify/amplify-backend/blob/main/packages/integration-tests/src/test-live-dependency-health-checks/health_checks.test.ts
| }); | ||
| }); | ||
|
|
||
| void describe('fails fast', () => { |
There was a problem hiding this comment.
we can remove this block too if we're testing in separate file.
This reverts commit 9f85439.
|
Continue on #901 |
Problem
e2e-flowtest, which is a copy oftest-e2e/create-amplify.testdeploytest tocreate-amplifyscriptIssue number, if available:
Changes
Corresponding docs PR, if applicable:
Validation
Checklist
run-e2elabel set.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.