Feat: add e2e tests workflow with Playwright support - #73
Conversation
cedric-anne
left a comment
There was a problem hiding this comment.
Why not adding a dedicated step to the continuous-integration.yml workflow, like we do for other tests?
Kept separate because e2e needs its own DB/environment and a Playwright browser install (see the header comment in |
| - name: "Restore Playwright browser cache" | ||
| id: "playwright-cache" | ||
| uses: "actions/cache@v6" | ||
| with: | ||
| path: "/home/www-data/.cache/ms-playwright" | ||
| key: "playwright-chromium-${{ steps.playwright-version.outputs.version }}" |
There was a problem hiding this comment.
https://playwright.dev/docs/ci#caching-browsers
Caching browser binaries is not recommended, since the amount of time it takes to restore the cache is comparable to the time it takes to download the binaries. Especially under Linux, operating system dependencies need to be installed, which are not cacheable.
| - name: "Restore composer cache" | ||
| uses: "actions/cache@v6" | ||
| with: | ||
| path: "/var/www/glpi/plugins/${{ inputs.plugin-key }}/vendor" | ||
| key: "${{ inputs.plugin-key }}-composer-${{ hashFiles(format('{0}/composer.lock', inputs.plugin-key)) }}" |
There was a problem hiding this comment.
The sudo setfacl commands run before this command so it doesn't fix the files extracted from the cache. But maybe it is not needed? What do you think?
We could swap the order to avoid having to think about it ;)
| if: ${{ !cancelled() }} | ||
| uses: "actions/upload-artifact@v7" | ||
| with: | ||
| name: "playwright-report" |
There was a problem hiding this comment.
If this is run inside a matrix (which you put in the readme example), you'll need a unique name per report.
| run: | | ||
| git config --global --add safe.directory "/var/www/glpi/plugins/${{ inputs.plugin-key }}" | ||
|
|
||
| - name: "Restore composer cache" |
There was a problem hiding this comment.
This (and the install step) assume that every plugin has a composer file. Is that the case?
| uses: "actions/cache@v6" | ||
| with: | ||
| path: "/var/www/glpi/plugins/${{ inputs.plugin-key }}/vendor" | ||
| key: "${{ inputs.plugin-key }}-composer-${{ hashFiles(format('{0}/composer.lock', inputs.plugin-key)) }}" |
There was a problem hiding this comment.
This part looks completely different in continuous-integration.yml (specific cache folder, keys on php versions, fallback keys, ...).
Shouldn't it be identical? Which one is correct?
ccailly
left a comment
There was a problem hiding this comment.
I had already started a similar project to integrate Playwright tests into CI while I was working on the workflows plugin; in particular, I was waiting for that project to make progress before moving forward with my PR (#62).
For the past few weeks, I’ve continued to use it on the itilprocesses plugin, but I don’t necessarily have a preference between that PR and mine.
However, I’ve encountered some issues with Playwright’s screenshots/snapshots, which depend on the environment being used.
Since we’re pushing the use of the Makefile, it’s important to run the tests in the same way.
The Makefile uses the Docker image provided for this purpose: mcr.microsoft.com/playwright
AdrienClairembault
left a comment
There was a problem hiding this comment.
(I didn't check the code again, I'll let you guys handle it since I lack time and plugins are outside my scope).
Plugins can now run their Playwright e2e specs against a dedicated GLPI environment via a new reusable
e2e-tests.ymlworkflow, called either standalone or fanned out across the CI matrix.The README documents both usages with example snippets.