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

ci: setup e2e tests workflow and local webpack e2e test #508

Open
wants to merge 58 commits into
base: main
Choose a base branch
from

Conversation

zxl629
Copy link
Contributor

@zxl629 zxl629 commented Jan 22, 2025

Issue #, if available:

Description of changes:

Example pipeline run

This change is a part of the E2E test roll out plan for amplify-data repository. It can be summarized into two parts:

  1. Adds a sample app to test webpack bundler support.
  2. Adds webpack E2E test as a release blocker for amplify-data repository.

For the test, this change uses webpack to bundle a default Amplify app into a javascript file: main.bundle.js.

Amplify app:

import { generateClient } from 'aws-amplify/data';
import { Schema } from '@/amplify/data/resource';

const client = generateClient<Schema>();

Test Process in Git workflows:

  1. File Preparation: The bundled JavaScript file will be incorporated as the source script in the index.html file.
  2. Server Setup: GitHub runners will build the project and start a local development server. The server will be configured to listen on localhost:3000.
  3. Request Execution: The test will use the curl CLI tool to send a GET request to http://localhost:3000.
  4. Response Evaluation: A successful test will receive an HTTP status code 200, indicating that the page loaded correctly.
    Any other status code will result in a test failure.

This change also adds the necessary GitHub actions config, scripts, and testing dependencies. Once this PR is merged, tests will run in the pipeline.

This change also modifies the current documentation for E2E tests, providing detailed instructions for local e2e testing.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link

changeset-bot bot commented Jan 22, 2025

🦋 Changeset detected

Latest commit: 1ae4346

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

This PR includes changesets to release 0 packages

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

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

scripts/bundler-test.mjs Fixed Show fixed Hide fixed
@zxl629 zxl629 marked this pull request as ready for review January 22, 2025 21:07
@zxl629 zxl629 requested review from a team as code owners January 22, 2025 21:07
- name: Setup node and build the repository
uses: ./amplify-data/.github/actions/node-and-build
- name: Make script executable
run: chmod +x ./amplify-data/scripts/retry-npm-script.sh
Copy link
Member

Choose a reason for hiding this comment

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

Do we chmod scripts like this in other places? I thought the file permissions were checked in.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I had to add this step to avoid error like this. I thought the same way too, and I wonder if I missed some steps here since it should not be needed.

Comment on lines +10 to +23
const FRAMEWORKS = {
rollup: 'rollup',
webpack: 'webpack',
};

const BUILD_TYPE = {
dev: 'dev',
prod: 'prod',
};

const frameworkPort = {
[FRAMEWORKS.rollup]: '3000',
[FRAMEWORKS.webpack]: '3000',
};
Copy link
Member

Choose a reason for hiding this comment

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

It looks like we're using this script to fan out.

If these are long running, should we fan them out in workflows on separate instances?

If they are not long running, do we need to have them running concurrently? Wondering if we can simplify the logic to have the run synchronously if we need them running on the same test runner.

Mostly, there's a lot of logic here and I'm wondering if there's a different pattern to achieve the same or a similar outcome with less complexity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This script is called by the workflow here. The logic is mainly used to validate the inputs of the test and set up port numbers for it. At any time, only 1 framework with 1 build type is being tested. The concurrency is only used to set up the amplify app sample for a given test config, and run the test against it. This file is modified from the original test.js file in samples-staging repo, and it might be an overkill if we don't plan to add more complicated e2e tests in the future.

@@ -77,11 +80,13 @@
"jsdom": "^25.0.0",
"prettier": "^3.0.3",
"rimraf": "^5.0.5",
"serve": "^14.2.4",
Copy link
Member

Choose a reason for hiding this comment

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

Can we push this dependency down into the test repo and run a command from there to launch it instead of having this dep in the package root?

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.

2 participants