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.

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

add simple client tests #782

Merged
merged 12 commits into from
Sep 7, 2022
Merged

add simple client tests #782

merged 12 commits into from
Sep 7, 2022

Conversation

alharris-at
Copy link
Contributor

@alharris-at alharris-at commented Sep 7, 2022

Getting the initial rev of the test app into our repo, this isn't yet connected to our CCI suite in any capacity, but I'd like to treat that as a separate review, and get this merged effectively in isolation.

Description of changes

Taken From README

Using this test app

This test app has capabilities to deploy a backend amplify project, tear down said project, and execute a ui test suite against the running app using the full backend.

To facilitate easier dev/test cycles, the test there are 4 test lifecycles defined with custom targets to make it easier to develop.

npm run test will execute all of these lifecycles.
npm run test:setup will strictly setup the amplify app in your DEFAULT aws profile.
npm run test:teardown will execute an amplify delete on the project.
npm run test:execute will start the webapp, and run the cypress suite against it.
npm run test:watch will start the app, and open a cypress watch against it.

In addition to these targets, npm start can still be used to run the app locally (recommended after npm run test:setup to develop new test pages/cases), and npx cypress open can be used to interactively run/re-run the cypress suite, this is probably preferable to using just npm run test:watch but I don't have a strong opinion on that yet.

Test Structure

Both the amplify app setup, and basic CRUD+List+Observe functionality rely on test harnesses to reduce the per-test load of creating a UI that uses our API/DataStore clients, and managing basic lifecycle concerns like app setup, teardown, invoking cypress, etc.

Cypress suites are still written by hand in the cypress directory, though we could build helpers given the consistent structure of the UI due to these harnesses.

Backend setup code is available at api.test.ts, and top-level app pages can be found in the pages directory.

CLI Interactions

This project uses a stripped down version of the amplify-e2e-core shims for shelling out to Amplify. This is expected to be shortly refactored back to a shared lib, but the reason we don't use that directly is due to the large dep-tree it invokes, including the actual CLI source code, which causes issues with jest. This is also potentially mitigable, but while I was under the hood, I simplified the API a bit as well, which I'm happy with. The refactored bits list in the __tests__/utils directory, in addition to the test harness package, which is expected to be re-used across apps for consistency.

Issue #, if available

N/A

Description of how you validated changes

Tests running locally on a windows + mac laptop.

Checklist

  • PR description included
  • yarn test passes
  • Tests are changed or added
  • Relevant documentation is changed or added (and PR referenced)

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

@alharris-at alharris-at marked this pull request as ready for review September 7, 2022 19:45
@alharris-at alharris-at requested a review from a team as a code owner September 7, 2022 19:45
@AaronZyLee AaronZyLee merged commit 9ad36ac into main Sep 7, 2022
@AaronZyLee AaronZyLee deleted the add-simple-client-tests branch September 7, 2022 21:59
Copy link
Contributor

@phani-srikar phani-srikar left a comment

Choose a reason for hiding this comment

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

LGTM! One suggestion I have is to rename the app or directory structure to reflect that this is a "React" App.

permissionsBoundaryArn: undefined,
};

export const getCLIPath = (testingWithLatestCodebase = false) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I see that testingWithLatestCodebase is always false, so we're basically either using the path from the environment variables or the globally installed amplify. Just a question, do we have this set in the env variable? or we could also just always use the local binary after setup-dev.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I intend for this to get re-merged back into the core amplify-core-e2e stuff, so this WILL be used then I think. I'll need to revisit this in order to get the amplify-dev stuff working, and potentially the circle CI integration.

@alharris-at
Copy link
Contributor Author

LGTM! One suggestion I have is to rename the app or directory structure to reflect that this is a "React" App.

That's a good callout, it's a react app, not necessarily just a JS app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants