WIP: Playwright tests with shared repo#713
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #713 +/- ##
=======================================
Coverage 15.77% 15.77%
=======================================
Files 15 15
Lines 5577 5577
=======================================
Hits 880 880
Misses 4655 4655
Partials 42 42 ☔ View full report in Codecov by Sentry. |
hanzei
left a comment
hanzei
left a comment
There was a problem hiding this comment.
Some of the comments in mattermost-community/mattermost-plugin-todo#236 (review) apply here as well. Let's discuss and merge the PR first.
| # cache: "npm" | ||
| # cache-dependency-path: webapp/package-lock.json |
| - name: ci/checkout-mattermost-monorepo | ||
| run: | | ||
| git clone https://github.com/mattermost/mattermost.git ../mattermost |
There was a problem hiding this comment.
I wonder if the version should be opt-in here. Otherwise changes on mattermost could break the E2E tests in this repo. On the other hand, it might be good if we notice a breaking change early.
There was a problem hiding this comment.
@hanzei This is intentional to have it always use master, though I think we could have two workflows if we also want a pinned version, like we did with the Apps plugin.
could break the E2E tests in this repo
The main thing that was breaking things in the plugin e2e tests related to the monorepo where the Playwright helpers that the e2e tests were using, like getLastPost. The function implementations didn't change (because the general DOM structure of the webapp didn't change), but where the function is located can change at any time. These are also not necessarily meant to be consumed by other projects.
If the plugin e2e test fails because a behavior change in the webapp's functionality, I think we would want to see the test failure to see if it's something we need to address. If this does cause an issue, we'll handle it on a case-by-case basis.
To solve the issue of the helpers changing paths etc., the plan is to have these helpers intended to be used by plugin e2e tests defined in a separate repo, currently housed at mattermost-community/mattermost-plugin-e2e-test-utils#1
There was a problem hiding this comment.
This is intentional to have it always use master, though I think we could have two workflows if we also want a pinned version, like we did with the Apps plugin.
👍 for using the same pattern as the Apps plugin (Release on PR, master on master)
| await c.postMessage('/github me'); | ||
| await c.sendMessage(); | ||
| await postMessage('/github me', page); | ||
| // await c.sendMessage(); |
mickmister
changed the title
|
@hanzei Note that this PR and mattermost-community/mattermost-plugin-todo#236 (which is also using the mattermost-plugin-e2e-test-utils) repo are both WIP and proof of concept atm for sharing code. I've set them both as drafts and added |
hanzei
added
Work In Progress
|
Closing as we're going with a different approach mattermost-community/mattermost-plugin-e2e-test-utils#3 |
4 participants
Summary
Ticket Link