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

feat: implement e2e tests workflow #227

Open
wants to merge 9 commits into
base: next
Choose a base branch
from

Conversation

huynguyen-hl
Copy link
Member

@huynguyen-hl huynguyen-hl commented Feb 28, 2025

What type of PR is this? (check all applicable)

  • πŸ• Feature
  • πŸ› Bug Fix
  • πŸ“ Documentation Update
  • 🎨 Style
  • πŸ§‘β€πŸ’» Code Refactor
  • πŸ”₯ Performance Improvements
  • βœ… Test
  • πŸ€– Build
  • πŸ” CI
  • πŸ“¦ Chore (Release)
  • ⏩ Revert

Description

This PR sets up Cypress E2E tests to run in both the Build and Test and Release workflows. The tests will be triggered whenever:

  • A commit is pushed to the main branch (release).
  • A PR is raised into the next branch.

Steps added to run E2E tests:

  • Start E2E Docker Compose.
  • Ensure IDR Service Readiness and Health (call the IDR health check API to ensure it’s ready to receive test requests).
  • Run E2E tests.
  • Stop Docker Compose.

In this PR, I’ve updated the resetData function to use the MinIO client library for deleting data in the identity-resolver-service-object-store service instead of the fs library for file system interactions. Additionally, I’ve renamed the function to clearObjectStore for better clarity. This removes the need to manually delete files or folders that are bind-mounted by the identity-resolver-service-object-store Docker Compose service, which would otherwise lead to a file system permission issue. Since the GitHub Actions environment runs under the runner user while the bind-mounted files and folders are owned by root, direct deletion is not possible.

Cypress E2E tests run successfully in a local environment because we use the same user (which has the highest privileges). However, if a different user were used locally, the same issue would occur.

Mobile & Desktop Screenshots/Recordings

Testing E2E pipeline in Github personal account:

image

Added tests?

  • πŸ‘ yes
  • πŸ™… no, because they aren't needed
  • πŸ™‹ no, because I need help

Added to documentation?

  • πŸ“– Mock App docs site
  • πŸ“œ README.md
  • πŸ“• storybook
  • πŸ™… no documentation needed

Copy link

github-actions bot commented Feb 28, 2025

Code Coverage Report

Lines Statements Branches Functions
Coverage: 82%
81.79% (3571/4366) 69.1% (859/1243) 73.54% (417/567)
Title Lines Statements Branches Functions
All packages Coverage: 82%
81.79% (3571/4366) 69.1% (859/1243) 73.54% (417/567)
Components Coverage: 84%
84.03% (516/614) 69.36% (120/173) 77.23% (95/123)
Mock app Coverage: 71%
71.89% (371/516) 53.23% (74/139) 68.14% (77/113)
Services Coverage: 81%
79.29% (1038/1309) 64.88% (255/393) 75.71% (106/140)
UNTP test suite Coverage: 85%
85.75% (1090/1271) 81.35% (144/177) 67.21% (41/61)
VC test suite Coverage: 100%
100% (20/20) 100% (2/2) 100% (4/4)
UNTP Playground Coverage: 87%
84.92% (569/670) 73.68% (266/361) 75.38% (98/130)

@huynguyen-hl huynguyen-hl marked this pull request as draft February 28, 2025 12:45
@huynguyen-hl huynguyen-hl force-pushed the feat/e2e-tests-workflow branch from b50eafd to ee2c74e Compare March 2, 2025 18:47
@absoludity
Copy link
Contributor

absoludity commented Mar 3, 2025

Hi @huynguyen-hl . This PR is in a Draft status (and has conflicts), and has some notes about things that aren't finished, so it's not clear if you're wanting a review yet?

  • Commented out cy.task('resetData') due to a file system permission conflict in the GitHub Actions environment. The minio_data folder is created by a Docker bind mount with root privileges, but the GitHub workflow runs under the runner user, which lacks permission to delete files inside minio_data (This is a temporary workaround until a better solution is implemented).

Can we just sudo to delete the files from within the runner? See https://docs.github.com/en/actions/using-github-hosted-runners/using-github-hosted-runners/about-github-hosted-runners#administrative-privileges

@huynguyen-hl
Copy link
Member Author

huynguyen-hl commented Mar 3, 2025

Thanks @absoludity for the early feedback. I tried using sudo, but it still resulted in permission errors. I also used the mxschmitt/action-tmate@v3 action to SSH into the GitHub Actions server for debugging and discovered that the root cause was the difference in folder ownership.

To address this, I took a different approach by using the MinIO client library to delete data from the identity-resolver-service-object-store service. This method works as intended without any permission issues.

@huynguyen-hl
Copy link
Member Author

huynguyen-hl commented Mar 3, 2025

I noticed that some test cases in both untp-playground/report-generation.cy.ts and untp-playground/vcdm-validation.cy.ts are using unavailable files, such as:

  • cypress/fixtures/credentials-e2e/valid-v2-enveloped-dpp.json
  • cypress/fixtures/credentials-e2e/invalid-schema-v2-enveloped-dpp.json
  • ...

This is why the E2E tests are failing. I believe we'll need a separate ticket to address this issue.

@huynguyen-hl huynguyen-hl marked this pull request as ready for review March 3, 2025 03:52
@ashleythedeveloper
Copy link
Collaborator

ashleythedeveloper commented Mar 3, 2025

I noticed that some test cases in both untp-playground/report-generation.cy.ts and untp-playground/vcdm-validation.cy.ts are using unavailable files, such as:

  • cypress/fixtures/credentials-e2e/valid-v2-enveloped-dpp.json
  • cypress/fixtures/credentials-e2e/invalid-schema-v2-enveloped-dpp.json
  • ...

This is why the E2E tests are failing. I believe we'll need a separate ticket to address this issue.

@huynguyen-hl, this is the fix I was referring to in our last standup. I've raised PR #229 to address this issue. I suggest to merge PR #229 first.

- name: Start E2E docker compose
run: SEEDING=true docker compose -f docker-compose.e2e.yml up -d

- name: Ensure IDR Service Readiness and Health
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @huynguyen-hl, is there a reason why we aren't using the built-in docker-compose health check functionality?

- name: Start E2E docker compose
run: SEEDING=true docker compose -f docker-compose.e2e.yml up -d

- name: Ensure IDR Service Readiness and Health
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above.

@@ -51,6 +51,52 @@ jobs:
VC test suite, ./packages/vc-test-suite/coverage/coverage-summary.json
UNTP Playground, ./packages/untp-playground/coverage/coverage-summary.json

- name: Start E2E docker compose
run: SEEDING=true docker compose -f docker-compose.e2e.yml up -d
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will GitHub cache the docker image between each run? If so, we need to build the image so the new functionality is present in the containers.

@@ -41,6 +41,52 @@ jobs:
- name: Run tests
run: yarn test

- name: Start E2E docker compose
run: SEEDING=true docker compose -f docker-compose.e2e.yml up -d
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above.


const execPromise = util.promisify(exec);
export default defineConfig({
env: {
idrBucketName: process.env.IDR_BUCKET_NAME || 'idr-bucket-1',
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure why we are diverging from the existing environment variable names for the IDR services.

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.

3 participants