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

Containerize e2e tests #88

Closed
wants to merge 68 commits into from
Closed

Containerize e2e tests #88

wants to merge 68 commits into from

Conversation

rylew1
Copy link

@rylew1 rylew1 commented Oct 21, 2024

Ticket

navapbc/template-infra#735

Changes

  • containerize e2e tests with playwright image

Context

Opened on a template-infra => navapbc/template-infra#770

https://playwright.dev/docs/docker

output.mov

Preview environment

@rylew1 rylew1 marked this pull request as draft October 21, 2024 22:05
@rylew1 rylew1 requested a review from lorenyu October 22, 2024 17:45
@rylew1
Copy link
Author

rylew1 commented Oct 22, 2024

@lorenyu just checking in on general format of this before going further - a summary so far:

  • This works in CI and is able to upload the playwright-report folder to GH action artifacts
  • Works locally if you run using host.docker.internal (see run cmd below)
    • was just trying to accomplish this using a Dockerfile and not a Docker compose to port map things
    • Local command if server running on port 3001:
      docker run --name playwright-e2e-container -w /app playwright-e2e make e2e-test APP_NAME=app BASE_URL=http://host.docker.internal:3001

Copy link
Collaborator

@lorenyu lorenyu left a comment

Choose a reason for hiding this comment

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

Yeah general direction looks good!

.github/workflows/e2e-tests.yml Outdated Show resolved Hide resolved
.github/workflows/e2e-tests.yml Outdated Show resolved Hide resolved
e2e/Dockerfile Show resolved Hide resolved
.github/workflows/e2e-tests.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@doshitan doshitan left a comment

Choose a reason for hiding this comment

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

Caveat I haven't looked at the actual e2e stuff; just a few thoughts as I was scrolling through this PR, wouldn't call any of them blocking.

Makefile Outdated Show resolved Hide resolved
e2e/Dockerfile Outdated Show resolved Hide resolved
e2e/Dockerfile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Copy link
Collaborator

@lorenyu lorenyu left a comment

Choose a reason for hiding this comment

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

Added just a couple more nits. The suggestions from Tanner also seem good to do.

.github/workflows/e2e-tests.yml Show resolved Hide resolved
docs/e2e/e2e-checks.md Outdated Show resolved Hide resolved
docs/e2e/e2e-checks.md Outdated Show resolved Hide resolved
@rylew1
Copy link
Author

rylew1 commented Oct 28, 2024

I think we're close to being able to move this to template-infra for merging - let me know if there's anything else

@rylew1 rylew1 mentioned this pull request Oct 29, 2024
Copy link
Collaborator

@lorenyu lorenyu left a comment

Choose a reason for hiding this comment

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

some nits otherwise LGTM!

.github/workflows/pr-environment-checks.yml Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
docs/e2e/e2e-checks.md Outdated Show resolved Hide resolved
rylew1 added a commit to navapbc/template-infra that referenced this pull request Oct 30, 2024
## Ticket

#735

## Changes

- dockerize e2e tests with playwright
- Tested on `platform-test-nextjs` => navapbc/platform-test-nextjs#88
@rylew1 rylew1 closed this Oct 30, 2024
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