feat: add plugin-directory support for playwright#683
Closed
oshirohugo wants to merge 5 commits into
Closed
Conversation
Contributor
|
Hi Hugo, I just came across this PR. I actually opened another PR last week that tackles the same issue but with a slightly different approach: #696. One advantage of that approach is that it keeps the plugin-action and plugin-ci-workflows APIs in sync. It’s still a draft for now, as I’m waiting on the SLO team to assist with testing. |
Contributor
Author
|
@sunker tks for letting me know Erick. I will close this PR then :D. |
Contributor
Author
|
Closing this PR in favor of #696 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When a plugin lives in a subdirectory (e.g. monorepos using
plugin-directory), the Playwright workflow failed to resolve the correct Grafana versions to test against. Thee2e-versionaction always looked forsrc/plugin.jsonat the repo root, ignoringplugin-directory. The only workaround was to manually duplicate the Grafana version range viarun-playwright-with-grafana-dependency, which is easy to forget and gets stale.This PR eliminates the workaround by automatically reading
grafanaDependencyfrom the plugin's ownsrc/plugin.json(insideplugin-directory) when neither an explicitgrafana-dependencyinput nor a root-levelsrc/plugin.jsonapplies.A secondary bug was also fixed: the "Install Playwright Browsers" step was missing
working-directory, causingnpxto run from the repo root (where nonode_modulesexists in a monorepo layout), which silently installed a different playwright version globally and downloaded the wrong Chromium revision.Closes #553.
Changes
playwright.yml— auto-readgrafanaDependencyfromplugin-directory/src/plugin.jsonAdded a
Read Grafana dependency from plugin.jsonstep that runs whenplugin-directory != '.'and no explicitgrafana-dependencyis provided. It readsgrafanaDependencyfrom${plugin-directory}/src/plugin.jsonusingjqand passes it as a fallback to thee2e-versionaction. This makes the workflow self-contained for monorepo plugins without requiring any manual input.playwright.yml— fix missingworking-directoryon browser install stepThe
Install Playwright Browsersstep lackedworking-directory: ${{ inputs.plugin-directory }}. When the plugin is in a subdirectory,npxran from the repo root (no localnode_modules), fell back to downloading the latestplaywrightfrom the registry, and installed the wrong Chromium revision — causing all E2E test jobs to fail with "Executable doesn't exist". Added the missingworking-directoryto match every other npm step in the job.Tests
We checked that the changes didn't cause any issue when running without providing the plugin directory:
https://github.com/grafana/plugins-drone-to-gha/actions/runs/24903292302
https://github.com/grafana/plugins-drone-to-gha/actions/runs/24902780309
We checked that the changes worked when provinding the plugin directory:
https://github.com/grafana/plugins-drone-to-gha/actions/runs/24901502734