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.

Sign up for GitHub

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

Jump to bottom

fix(e2e): local e2e test hanging for sendYes #13010

Closed
wants to merge 5 commits into from
Closed

fix(e2e): local e2e test hanging for sendYes #13010

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
023528f
fix(amplify-e2e-core): amplifyPush
Jul 26, 2023
7bdc7f3
fix(amplify-e2e-tests): .env sample
Jul 26, 2023
295ec90
fix(amplify-e2e-tests): .env sample add comment
Jul 26, 2023
4ad7631
fix(amplify-e2e-core): change 'Are you sure you want to continue' back
Jul 26, 2023
8543186
Merge branch 'dev' into fix-e2e-local
Jul 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions packages/amplify-e2e-core/src/init/amplifyPush.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,12 @@ export const amplifyPush = async (cwd: string, testingWithLatestCodebase = false
const pushArgs = ['push', ...(opts?.minify ? ['--minify'] : [])];
// Test amplify push
await spawn(getCLIPath(testingWithLatestCodebase), pushArgs, { cwd, stripColors: true, noOutputTimeout: pushTimeoutMS })
.wait('Are you sure you want to continue?')
.wait('Are you sure you want to continue "amplify push"?')
Copy link
Member

Choose a reason for hiding this comment

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

is this correct change?
If e2e tests pass then it's fine I guess.
Otherwise the wait argument can be subset of prompt for example Are you sure you want to continue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, I accidentally commit this change..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since it passed the e2e test and it's sort of useful, I'll just leave it here

Copy link
Member

@sobolk sobolk Jul 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

all e2e tests? if pipeline is green good to keep this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not all e2e tests tho 🫢

Ok...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess it does matter https://github.com/aws-amplify/amplify-cli/blob/dev/packages/amplify-cli/src/extensions/amplify-helpers/push-resources.ts#L105

Copy link
Member

Choose a reason for hiding this comment

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

yeah.
if you want to capture both variants then either
.wait('Are you sure you want to continue')
OR .wait(/<some awesome regex to capture both/)
see

amplify-cli/packages/amplify-e2e-core/src/utils/nexpect.ts

Line 75 in 7bc0b56

wait: (expectation: string | RegExp, cb?: (data: string) => void) => ExecutionContext;

Copy link
Member

Choose a reason for hiding this comment

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

(not sure if we have 2 variants)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To make it simple, I just removed the change 😂 4ad7631

.sendYes()
.sendCarriageReturn()
.wait('Do you want to generate code for your newly created GraphQL API')
.sendConfirmNo()
.sendNo()
.sendCarriageReturn()
.wait(/.*/)
.runAsync();
};
Expand Down
8 changes: 3 additions & 5 deletions packages/amplify-e2e-tests/sample.env
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
AWS_ACCESS_KEY_ID=<your-access-key>
AWS_SECRET_ACCESS_KEY=<your-secret-access-key>
AWS_SESSION_TOKEN=<optional-session-token>
CLI_REGION=<region>

# Used for Auth Hosted UI
FACEBOOK_APP_ID=fbAppId
Expand All @@ -20,12 +21,9 @@ APPLE_KEY_ID=2QLZXKYJ8J
APPLE_PRIVATE_KEY=----BEGIN PRIVATE KEY-----MIGTAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBHkwdwIBAQQgIltgNsTgTfSzUadYiCS0VYtDDMFln/J8i1yJsSIw5g+gCgYIKoZIzj0DAQehRANCAASI8E0L/DhR/mIfTT07v3VwQu6q8I76lgn7kFhT0HvWoLuHKGQFcFkXXCgztgBrprzd419mUChAnKE6y89bWcNw----END PRIVATE KEY----
APPLE_PRIVATE_KEY_2=----BEGIN PRIVATE KEY-----MIGTAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBHkwdwIBAQQgIltgNsTgTfSzUadYiCS0VYtDDMFln/J8i1yJsSIw5g+gCgYIKoZIzj0DAQehRANCAASI8E0L/DhR/mIfTT07v3VwQu6q8I76lgn7kFhT0HvWoLuHKGQFcFkXXCgztgBrprzd419mUChAnKE6y89bWcNw----END PRIVATE KEY----

#Used for delete test
AWS_ACCESS_KEY_ID=<your-access-key>
AWS_SECRET_ACCESS_KEY=<your-secret-access-key>
CLI_REGION=<region>

# Used for cleanup script
CIRCLECI_TOKEN = '<your token>' # Token used for querying CircleCI to get the build details
CIRCLE_PROJECT_USERNAME='<your github username>'
CIRCLE_PROJECT_REPONAME='amplify-cli'

AMPLIFY_PATH=
Copy link
Member

Choose a reason for hiding this comment

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

Please add comment that explains what this does.

Something like # A path to Amplify CLI executable. When working locally it should be set to bin/amplify file in the workspace, for example <insert example here> if running regular e2e test. If working locally with migration test it should point to older version of amplify. Absolute paths are recommended.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 295ec90