Add e2e checks github actions workflow with prettier #875
Conversation
rylew1
changed the title
| run: make e2e-install-ci | ||
|
|
||
| - name: Check formatting | ||
| run: make e2e-format-check-native |
There was a problem hiding this comment.
I don't think we got to discussing the where or how this should run - we can go back to the google docs if necessary. I recently started a section about it there
Makefile
Outdated
| @@ -95,6 +103,21 @@ e2e-clean-report: ## Remove the local e2e report folders and content | |||
| rm -rf ./e2e/blob-report | |||
| rm -rf ./e2e/test-results | |||
|
|
|||
| e2e-format: ## Format code with autofix inside Docker | |||
| docker run --rm -v $(PWD)/e2e:/e2e $(E2E_NODE_IMAGE) sh -c "cd /e2e && npm run e2e-format" | |||
There was a problem hiding this comment.
Is there a way to get rid of cd /e2e? Would setting WORKDIR /e2e in the Dockerfile help?
There was a problem hiding this comment.
Reference comment #875 (comment)
Makefile
Outdated
| @@ -81,6 +86,9 @@ __check_defined = \ | |||
| # The e2e test image includes the test suite for all apps and therefore isn't specific to each app. | |||
| E2E_IMAGE_NAME := $(PROJECT_ROOT)-e2e | |||
|
|
|||
| # Define Node.js Docker image to use for e2e commands | |||
| E2E_NODE_IMAGE := node:22-alpine | |||
There was a problem hiding this comment.
Let's just use one image for e2e stuff not a separate one
There was a problem hiding this comment.
Combining your two suggestions above - we can use our existing ./e2e/Dockerfile - but I believe we need to override the ENTRYPOINT - this worked for me with the --entrypoint flag:
e2e-format-check: e2e-build ## Format check without autofix inside Docker
docker run --rm -v $(PWD)/e2e:/e2e --entrypoint sh $(E2E_IMAGE_NAME) -c "npm run e2e-format:check"
The ./e2e/Dockerfile was written just to run tests (npm run e2e-test) and its using the playwright image - which has a bunch of extra cross-browser testing libraries that aren't necessarily needed for checks. That's why I was thinking of using a separate simple node image to run checks.
There was a problem hiding this comment.
I'd be ok getting rid of ENTRYPOINT if that means we can simplify things.
Does it make the checks a lot slower to combine everything? I get the performance benefit but unless there's a significant different I don't really want to overoptimize.
| e2e-format-native: ## Format code with autofix natively | ||
| cd e2e && npm run e2e-format | ||
|
|
||
| e2e-install-ci: ## CI install dependencies |
There was a problem hiding this comment.
confusing that we have both e2e-install-ci and e2e-setup-ci can we combine them
There was a problem hiding this comment.
This issue is somewhat related to the comment above - I wanted a separate non-playwright install command for checks so i added e2e-install-ci.
If we wanted to again adapt our current setup - something like this might work:
e2e-setup-ci: ## Setup end-to-end tests for CI
cd e2e && npm ci
cd e2e && npm run e2e-setup
But this would be unnecessarily installing playwright for code quality checks - and unnecessarily running an extra npm ci for our create-report GHA job
…/github.com/navapbc/template-infra into rylew/Add-e2e-check-github-actions-workflow
Ticket
Resolves #813
Changes