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

Reprovision API Integration test backends #250

Merged
merged 12 commits into from
Dec 16, 2019
Merged

Reprovision API Integration test backends #250

merged 12 commits into from
Dec 16, 2019

Conversation

lawmicha
Copy link
Contributor

@lawmicha lawmicha commented Dec 10, 2019 •
edited
Loading

Major changes

This provisions the backend with the team's test account for

  • REST with IAM tests
  • GraphQL with UserPool tests
  • SyncBased API tests. These are currently working because it relies on Model.isSyncable which is currently always true
  • ModelBased API Tests. These are currently going to fail since API category thinks they are syncable.
  • All configuration files have been uploaded to S3.

Minor Changes

  • Moved the TODO GraphQL tests over to the GraphQL with User Pool tests since they both use the same schema
  • Updated PostNoSync model and CommentNoSync Model back to Post and Comment

Next up

  • Fix ModelBased API tests once we replace the check on Model.isSyncable with the check against Amplify.DataStore.isConfigured. Actual implementation details to be determined

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

@lawmicha lawmicha changed the title Reprovision API Integration tests 1 Reprovision API Integration tests backends Dec 11, 2019
@lawmicha lawmicha force-pushed the lawmicha/api-integ-tests-1 branch from 5d5fc1d to 4c4c990 Compare December 11, 2019 00:50
@lawmicha lawmicha marked this pull request as ready for review December 11, 2019 00:51
@lawmicha lawmicha requested review from palpatim and wooj2 December 11, 2019 00:51
@lawmicha lawmicha self-assigned this Dec 11, 2019
@lawmicha lawmicha added the api Issues related to the API category label Dec 11, 2019
@lawmicha lawmicha changed the title Reprovision API Integration tests backends Reprovision API Integration test backends Dec 11, 2019
palpatim
palpatim requested changes Dec 11, 2019
View reviewed changes
Copy link
Member

@palpatim palpatim left a comment

Choose a reason for hiding this comment

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

A few notes, but most importantly--you committed the configuration files without obfuscation, so you'll need to obfuscate them on the next commit, remove the Amplify projects that generated those resources, and regenerate files for S3.

AmplifyPlugins/API/APICategoryPlugin.xcodeproj/project.pbxproj Show resolved Hide resolved
AmplifyPlugins/API/AWSAPICategoryPluginFunctionalTests/GraphQLModelBased/.gitignore Outdated Show resolved Hide resolved
AmplifyPlugins/API/AWSAPICategoryPluginFunctionalTests/GraphQLModelBased/README.md Show resolved Hide resolved
...tegoryPluginFunctionalTests/GraphQLSyncBased/GraphQLSyncBasedTests-amplifyconfiguration.json Outdated Show resolved Hide resolved
...ns/API/AWSAPICategoryPluginIntegrationTests/GraphQL/GraphQLWithIAMIntegrationTests/README.md Outdated Show resolved Hide resolved
...ns/API/AWSAPICategoryPluginIntegrationTests/GraphQL/GraphQLWithIAMIntegrationTests/README.md Outdated Show resolved Hide resolved
...QLWithUserPoolIntegrationTests/GraphQLWithUserPoolIntegrationTests-amplifyconfiguration.json Outdated Show resolved Hide resolved
...raphQLWithUserPoolIntegrationTests/GraphQLWithUserPoolIntegrationTests-awsconfiguration.json Outdated Show resolved Hide resolved
...I/AWSAPICategoryPluginIntegrationTests/GraphQL/GraphQLWithUserPoolIntegrationTests/README.md Show resolved Hide resolved
@lawmicha
Copy link
Contributor Author

A few notes, but most importantly--you committed the configuration files without obfuscation, so you'll need to obfuscate them on the next commit, remove the Amplify projects that generated those resources, and regenerate files for S3.

doh. looks like i need to re-reprovision sync based test and graphql with userpool. I missed adding the gitignore at that directory level. i think a more scalable approach will be to update the gitignore at the top level with a pattern match to ignore all files that end with amplifyconfiguration.json and awsconfiguration.json

@lawmicha lawmicha force-pushed the lawmicha/api-integ-tests-1 branch from 52f367f to b838913 Compare December 11, 2019 16:17
@lawmicha lawmicha requested a review from palpatim December 11, 2019 16:56
palpatim
palpatim previously approved these changes Dec 13, 2019
View reviewed changes
Copy link
Member

@palpatim palpatim left a comment

Choose a reason for hiding this comment

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

Please confirm the new pattern, otherwise LGTM

.gitignore Show resolved Hide resolved
wooj2
wooj2 previously approved these changes Dec 13, 2019
View reviewed changes
Copy link
Contributor

@wooj2 wooj2 left a comment

Choose a reason for hiding this comment

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

minor comments, but largely looks fine, approve

AmplifyPlugins/API/AWSAPICategoryPluginFunctionalTests/GraphQLModelBased/README.md Outdated Show resolved Hide resolved
AmplifyPlugins/API/AWSAPICategoryPluginFunctionalTests/GraphQLModelBased/README.md Outdated Show resolved Hide resolved
AmplifyPlugins/API/AWSAPICategoryPluginFunctionalTests/GraphQLSyncBased/README.md Show resolved Hide resolved
...IntegrationTests/GraphQL/GraphQLWithIAMIntegrationTests/GraphQLWithIAMIntegrationTests.swift
*/

let user1 = "[email protected]"
let user1 = "[email protected]"
let password = "Abc123@@!!"
Copy link
Contributor

Choose a reason for hiding this comment

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

wait, that's the password i use for all my accounts.... :P

Copy link
Contributor Author

Choose a reason for hiding this comment

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

haha yeah, we should discuss some alternatives to how to set up tests which require users sign in. let's continue the conversation over in the storage PR (that pr builds on top of this one and i don't want to have to rebase things if any major changes)

@lawmicha lawmicha dismissed stale reviews from wooj2 and palpatim via 4b172c4 December 16, 2019 16:13
@lawmicha
Copy link
Contributor Author

lawmicha commented Dec 16, 2019 •
edited
Loading

odd, i pushed a new commit and it automatically "dismissed stale reviews" and now the approvals are gone.

might be worth to to go to Settings ▶ Branches ▶ Branch protection rules ▶ Edit ▶ Uncheck "Dismiss stale pull request approvals when new commits are pushed".

@lawmicha lawmicha mentioned this pull request Dec 16, 2019
Closed
@lawmicha lawmicha merged commit 20872a4 into master Dec 16, 2019
@palpatim palpatim deleted the lawmicha/api-integ-tests-1 branch December 24, 2019 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@palpatim palpatim palpatim approved these changes

@wooj2 wooj2 wooj2 left review comments

Assignees

@lawmicha lawmicha

Labels
api Issues related to the API category
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lawmicha @palpatim @wooj2