Skip to content

Commit e98620e

Browse files
test(ci): run tests in docker container (ionic-team#28893)
Issue number: Internal --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> The team currently faces two challenges: 1. Verifying visual changes locally is difficult because we cannot use the existing ground truths as they were generated in Linux environments and most of our team uses either macOS or Windows. While team members can generate ground truths in the correct environment, they need to remember to do that first before making changes. 2. Updating visual diffs is time consuming and can only be done by team members. Our GitHub Action runs the entire test suite which can take ~10 even if only a handful of screenshots are generated. Additionally, this job can only be run by team members meaning community contributors cannot update/add screenshots. This limits them to non-visual tasks when contributing. In the event that they do want to make visual changes, the team needs to copy all their code into a branch and manually run screenshot diffs for them. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - This PR introduces the ability to run all Playwright tests inside of a Docker container using an image with Playwright dependencies. The container will have access to the local project, so developers can make changes and then run tests in the container after the changes are compiled. This enables anyone to propose new screenshot changes. However, the "update screenshot" job will still be available for folks who do not want/are unable to use docker. - There are some typeface differences between GH Actions and the Docker image which is why there are a handful of screenshots that needed to be updated. One risk here is that the Playwright npm and Docker image versions must be kept in sync. As a result, I also updatRenovate to allow us to auto update the npm and Docker image versions at the same time. ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change: 1. Describe the impact and migration path for existing applications below. 3. Update the BREAKING.md file with the breaking change. 4. Add "BREAKING CHANGE: [...]" to the commit description when merging. See https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#footer for more information. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> ⚠️ There are still some issues I need to sort out with mounting the local project on Windows. However, using Ubuntu with the linux subsystem for windows can be used as a workaround. I'd like to merge this so we can start testing it in our day-to-day workflow and ironing out any bugs. --------- Co-authored-by: ionitron <[email protected]>
1 parent 02f89bb commit e98620e

File tree

44 files changed

+142
-17
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

44 files changed

+142
-17
lines changed

.github/workflows/actions/test-core-screenshot/action.yml

+4-4
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ runs:
2121
name: ionic-core
2222
path: ./core
2323
filename: CoreBuild.zip
24-
- name: Install Playwright Dependencies
25-
run: npm install && npx playwright install && npx playwright install-deps
24+
- name: Install Dependencies
25+
run: npm install
2626
shell: bash
2727
working-directory: ./core
2828
- id: clean-component-name
@@ -47,7 +47,7 @@ runs:
4747
shell: bash
4848
- name: Test
4949
if: inputs.update != 'true'
50-
run: npm run test.e2e ${{ steps.set-test-file.outputs.testFile }} -- --shard=${{ inputs.shard }}/${{ inputs.totalShards }}
50+
run: npm run test.e2e.docker.ci ${{ steps.set-test-file.outputs.testFile }} -- --shard=${{ inputs.shard }}/${{ inputs.totalShards }}
5151
shell: bash
5252
working-directory: ./core
5353
- name: Test and Update
@@ -69,7 +69,7 @@ runs:
6969
# which is why we not using the upload-archive
7070
# composite step here.
7171
run: |
72-
npm run test.e2e ${{ steps.set-test-file.outputs.testFile }} -- --shard=${{ inputs.shard }}/${{ inputs.totalShards }} --update-snapshots
72+
npm run test.e2e.docker.ci ${{ steps.set-test-file.outputs.testFile }} -- --shard=${{ inputs.shard }}/${{ inputs.totalShards }} --update-snapshots
7373
git add src/\*.png --force
7474
mkdir updated-screenshots
7575
cd ../ && rsync -R --progress $(git diff --name-only --cached) core/updated-screenshots

.github/workflows/actions/update-reference-screenshots/action.yml

+4-7
Original file line numberDiff line numberDiff line change
@@ -25,20 +25,17 @@ runs:
2525
# Configure user as Ionitron
2626
# and push only the changed .png snapshots
2727
# to the remote branch.
28-
# Screenshots are in .gitignore
28+
# Non-Linux screenshots are in .gitignore
2929
# to prevent local screenshots from getting
30-
# pushed to Git. As a result, we need --force
31-
# here so that CI generated screenshots can
32-
# get added to git. Screenshot ground truths
33-
# should only be added via this CI process.
30+
# pushed to Git.
3431
run: |
3532
git config user.name ionitron
3633
git config user.email [email protected]
3734
3835
# This adds an empty entry for new
3936
# screenshot files so we can track them with
4037
# git diff
41-
git add src/\*.png --force -N
38+
git add src/\*.png -N
4239
4340
if git diff --exit-code; then
4441
echo -e "\033[1;31m⚠️ Error: No new screenshots generated ⚠️\033[0m"
@@ -48,7 +45,7 @@ runs:
4845
else
4946
# This actually adds the contents
5047
# of the screenshots (including new ones)
51-
git add src/\*.png --force
48+
git add src/\*.png
5249
git commit -m "chore(): add updated snapshots"
5350
git push
5451
fi

.gitignore

+10-1
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,16 @@ core/www/
6868
# playwright
6969
core/test-results/
7070
core/playwright-report/
71-
core/**/*-snapshots
71+
72+
# ground truths generated outside of docker should not be committed to the repo
73+
core/**/*-snapshots/*
74+
75+
# new ground truths should only be generated inside of docker which will result in -linux.png screenshots
76+
!core/**/*-snapshots/*-linux.png
77+
78+
# these files are going to be different per-developer environment so do not add them to the repo
79+
core/docker-display.txt
80+
core/docker-display-volume.txt
7281

7382
# angular
7483
packages/angular/css/

core/Dockerfile

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
# Get Playwright
2+
FROM mcr.microsoft.com/playwright:v1.42.1
3+
4+
# Set the working directory
5+
WORKDIR /ionic

core/package.json

+6-1
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,12 @@
9494
"test.e2e.update-snapshots": "npm run test.e2e -- --update-snapshots",
9595
"test.watch": "jest --watch --no-cache",
9696
"test.treeshake": "node scripts/treeshaking.js dist/index.js",
97-
"validate": "npm run lint && npm run test && npm run build && npm run test.treeshake"
97+
"validate": "npm run lint && npm run test && npm run build && npm run test.treeshake",
98+
"docker.build": "docker build -t ionic-playwright .",
99+
"test.e2e.docker": "npm run docker.build && docker run -it --rm -e DISPLAY=$(cat docker-display.txt) -v $(cat docker-display-volume.txt) --ipc=host --mount=type=bind,source=./,target=/ionic ionic-playwright npm run test.e2e --",
100+
"test.e2e.docker.update-snapshots": "npm run test.e2e.docker -- --update-snapshots",
101+
"test.e2e.docker.ci": "npm run docker.build && docker run -e CI='true' --rm --ipc=host --mount=type=bind,source=./,target=/ionic ionic-playwright npm run test.e2e --",
102+
"test.report": "npx playwright show-report"
98103
},
99104
"author": "Ionic Team",
100105
"license": "MIT",

core/src/utils/test/playwright/docs/usage-instructions.md

+104-4

renovate.json5

+9
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,15 @@
2929
"core/package.json"
3030
]
3131
},
32+
{
33+
matchDatasources: ["docker"],
34+
matchPackageNames: ["mcr.microsoft.com/playwright"],
35+
groupName: "playwright",
36+
matchFileNames: [
37+
"core/Dockerfile"
38+
],
39+
versioning: "semver"
40+
},
3241
{
3342
matchPackagePatterns: ["ionicons"],
3443
groupName: "ionicons",

0 commit comments

Comments
 (0)