New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[GH-124]: Integrating Playwright for E2E tests #228

Merged

mickmister merged 35 commits into mattermost-community:master from rahulsuresh-git:feat/adding-playwright-e2e

Nov 17, 2023

Contributor

rahulsuresh-git commented Nov 1, 2023 •

edited by mickmister

Loading

Summary

This PR introduces E2E testing for the Todo plugin using Playwright. It largely borrows from the implementation in the GitHub plugin https://github.com/mattermost/mattermost-plugin-github/blob/master/.github/workflows/playwright.yml. There are changes made to support the latest monorepo changes.

The tests themselves assert that the correct slash command suggestions are shown when you type /todo into the post text box.

Original PR
#231

Ticket Link

Fixes #124

rahulsuresh-git added 4 commits

November 1, 2023 10:04


          setup playwright using docs

c24bd0f


          fixed setup commands in setup-environment script

a3871d2


          added readme

389c10f


          added config for playwright

e52d248

mickmister self-requested a review

November 6, 2023 03:51

rahulsuresh-git added 5 commits

November 9, 2023 09:49


          Merge branch 'master' of https://github.com/rahulsuresh-git/mattermos…

c0d4dc3

…t-plugin-todo into feat/adding-playwright-e2e


          integrated playwright utils and added test to check available commands

ef99bdd


          removed changes from user.ts

cce3e55


          fixed TodoBot URL

ab195be


          removed a comment

15ea6cb

rahulsuresh-git marked this pull request as ready for review

November 9, 2023 19:53

rahulsuresh-git requested a review from larkox as a code owner

November 9, 2023 19:53

Contributor Author

rahulsuresh-git commented Nov 9, 2023 •

edited

Loading

Hi @mickmister and @larkox,
I've verified that the changes work by running a simple test that triggers the slash container popup when someone types /todo.

Here are the results:

Let me know if you have any concerns. I'd be happy to address.

rahulsuresh-git commented

View reviewed changes

e2e/playwright/support/init_test.ts Outdated Show resolved Hide resolved

rahulsuresh-git commented

View reviewed changes

e2e/playwright/tests/todo_plugin.spec.ts Outdated Show resolved Hide resolved

rahulsuresh-git added 2 commits

November 9, 2023 21:04


          improvements to message input function

54c3133


          fixed issue with cleanUpBotDMs

007367f

Contributor Author

rahulsuresh-git commented Nov 10, 2023

This PR is ready for review.


          added github workflow for playwright

23c6e32

codecov-commenter commented Nov 11, 2023 •

edited

Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8b25f12) 6.42% compared to head (e6af7c0) 6.42%.

Additional details and impacted files

@@          Coverage Diff           @@
##           master    #228   +/-   ##
======================================
  Coverage    6.42%   6.42%           
======================================
  Files          11      11           
  Lines        1712    1712           
======================================
  Hits          110     110           
  Misses       1594    1594           
  Partials        8       8

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

rahulsuresh-git mentioned this pull request

ARM Build. #211

Closed

rahulsuresh-git added 9 commits

November 15, 2023 17:24


          updated workflow with workflow_dispatch

6646dd0


          updated node version in .nvmrc

8d22541


          updated package-lock.json

8bc4127


          updated package-lock.json for node 16.13.1

98e0dc2


          updated package-lock.json for node 16.13.1

3a94bd3


          Merge branch 'feat/adding-playwright-e2e' of https://github.com/rahul…

63284a8

…suresh-git/mattermost-plugin-todo into feat/adding-playwright-e2e


          added python to workflow

721a285


          removed python from workflow and removed package-lock.json cache

4be1742


          attempts to fix workflow failure

0e9c707

rahulsuresh-git added 2 commits

November 16, 2023 03:05


          attempts to fix workflow failure

858d67e


          attempts to fix workflow failure

67351b4

hanzei added the 2: Dev Review label

rahulsuresh-git added 12 commits

November 16, 2023 13:07


          reverted workflow changes

f48fdb2


          reverted workflow changes

745a291


          disabled caching in workflow

3ac3aec


          Merge branch 'master' of https://github.com/rahulsuresh-git/mattermos…

a798f8d

…t-plugin-todo into feat/adding-playwright-e2e


          updated playwright workflow and node version to 16.20.2

ab0fcd9


          updated to node 18.17 and updated setup-environment

ae3e6d2


          updated workflow with new steps

f848c87


          changed checkout method for mattermost-monorepo

cf843ad


          fixed tsc errors

1a37d61


          commented out playwright ci/lint in workflow

8a698e3


          updated workflow to run on pull_request

dd496af


          updated workflow triggers

e6af7c0

mickmister mentioned this pull request

Fix e2e playwright directory linting #232

Closed

mickmister approved these changes

View reviewed changes

Contributor

mickmister left a comment

Thanks for this awesome contribution @rahulsuresh-git!!

LGTM 👍 Just some non-blocking suggestions and comments

.github/workflows/playwright.yml

Comment on lines +132 to +136

+                    # - name: ci/checkout-mattermost-monorepo
+                    #   uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0
+                    #   with:
+                    #     path: ../mattermost
+                    #     repository: mattermost/mattermost

Contributor

mickmister Nov 17, 2023

I think we can remove this block

.github/workflows/playwright.yml

Comment on lines +108 to +113

+                    - name: ci/setup-node
+                      uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c # v3.6.0
+                      with:
+                        node-version-file: ".nvmrc"
+                        # cache: "npm"
+                        # cache-dependency-path: webapp/package-lock.json

Contributor

mickmister Nov 17, 2023

Made an issue to address dependency caching #233

.github/workflows/playwright.yml

Comment on lines -3 to -6

-                workflow_run:
-                  workflows: ["ci"]
-                  types:
-                    - completed

Contributor

mickmister Nov 17, 2023

Note to other reviewers that one of the main reasons this was written this way before was so PRs coming from forks could use the OAuth token secrets for e2e testing. We're going to try to avoid the need to use real tokens going forward, by having a test harness around external services, that the e2e test can act as and make assertions.

.github/workflows/playwright.yml

Comment on lines +168 to +176

+                    - name: ci/tsc
+                      run: |
+                        cd e2e/playwright
+                        npm run tsc
+                    # - name: ci/lint
+                    #   run: |
+                    #     cd e2e/playwright
+                    #     npm run lint

Contributor

mickmister Nov 17, 2023

Since all of the other steps are descriptive:

Suggested change

      
                  - name: ci/tsc
          
                    run: |
          
                      cd e2e/playwright
          
                      npm run tsc
          
                  # - name: ci/lint
          
                  #   run: |
          
                  #     cd e2e/playwright
          
                  #     npm run lint
          
                  - name: ci/tsc-plugin-playwright
          
                    run: |
          
                      cd e2e/playwright
          
                      npm run tsc
          
                  # - name: ci/lint-plugin-playwright
          
                  #   run: |
          
                  #     cd e2e/playwright
          
                  #     npm run lint

.github/workflows/playwright.yml

Comment on lines +173 to +176

+                    # - name: ci/lint
+                    #   run: |
+                    #     cd e2e/playwright
+                    #     npm run lint

Contributor

mickmister Nov 17, 2023

Made a ticket to address this in a separate PR #232

We were receiving linting errors in the e2e code:

The task here is to fix the linting errors here:
https://github.com/rahulsuresh-git/mattermost-plugin-todo/actions/runs/6897786793/job/18766591926

/home/runner/work/mattermost-plugin-todo/mattermost-plugin-todo/e2e/playwright/playwright.config.ts
  4:1  error  Parsing error: The keyword 'import' is reserved

e2e/playwright/setup-environment

Comment on lines +1 to +4

+              #!/bin/sh
+              cd ../../../
+              git clone --depth 1 https://github.com/mattermost/mattermost.git
+              cd mattermost

Contributor

mickmister Nov 17, 2023

Note to other reviewers that this script is only to help set up the development environment, and is not used for CI. See this directory's README for more info

e2e/playwright/support/init_test.ts

Comment on lines +69 to +77

+              type TodoPluginSettings = {
+                  encryptionkey: string;
+                  webhooksecret: string;
+              }
+              const todoConfig: TodoPluginSettings = {
+                  encryptionkey: 'S9YasItflsENXnrnKUhMJkdosXTsr6Tc',
+                  webhooksecret: 'w7HfrdZ+mtJKnWnsmHMh8eKzWpQH7xET',
+              };

Contributor

mickmister Nov 17, 2023

The todo plugin does not have these settings. For the purpose of the test, we are not using plugin settings, so we can leave the settings blank for now, or comment it out so we know how to add them for later

e2e/playwright/support/init_test.ts

Comment on lines +60 to +67

+              // # Clear bot DM channel
+              test.beforeEach(async ({pw}) => {
+                  const {adminClient, adminUser} = await pw.getAdminClient();
+                  if (adminUser === null) {
+                      throw new Error('can not get adminUser');
+                  }
+                  await cleanUpBotDMs(adminClient, adminUser.id, botUsername);
+              });

Contributor

mickmister Nov 17, 2023

Note that this requires that tests must be run in serial. This allows every test to have a clean slate on the bot DM channel, with the same user logged in for each test

e2e/playwright/package.json

Comment on lines +22 to +23

		"eslint-plugin-react": "~7.32.2",
		"eslint-plugin-react-hooks": "~4.6.0",

Contributor

mickmister Nov 17, 2023

I think we can remove these too eslint plugins since we're not doing anything with React in this e2e test package

e2e/playwright/support/components/post.ts

Comment on lines +6 to +9

+              // TODO: migrate this helpers to webapp's ChannelsPost after monorepo migration
+              export const getBotTagFromPost = (post: ChannelsPost) => {
+                  return post.container.locator('.post__header').locator('.BotTag', {hasText: 'BOT'});
+              };

Contributor

mickmister Nov 17, 2023

I would just remove the todo comment. We will address this already when we extract the common code

mickmister requested review from saturninoabril, hanzei and toninis

November 17, 2023 02:38

This was referenced Nov 17, 2023

WIP: Playwright with shared repo, compared to open Playwright PR #235

Closed

WIP: Playwright with shared repo, compared with master #236

Draft

hanzei reviewed

View reviewed changes

Contributor

hanzei left a comment

I left my review at #236 (review). I think we can merge this PR and address the remaining concerns as part of #236.

larkox approved these changes

View reviewed changes

mickmister merged commit 65296b8 into mattermost-community:master

5 checks passed

Contributor

mickmister commented Nov 18, 2023

Awesome work on this @rahulsuresh-git!! It's incredible the effort you put into this and the result has already had a big impact on our team's direction on testing the plugin projects.

We had a past effort to introduce e2e tests to our plugin projects, though it was generally de-prioritized and left as a work-in-progress. Your efforts here to take that on and ensure a working framework has sparked momentum in our team to achieve better test confidence on each of the projects. Great work creating a solid foundation to build on top of 🚀 Thanks @rahulsuresh-git!

Contributor Author

rahulsuresh-git commented Nov 18, 2023

Thank you for the opportunity! @mickmister :)

hanzei added 4: Reviews Complete and removed 2: Dev Review labels

saturninoabril mentioned this pull request

feat(e2e): playwright base setup mattermost/mattermost-plugin-starter-template#208

Open

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

hanzei hanzei left review comments

mickmister mickmister approved these changes

larkox larkox approved these changes

saturninoabril Awaiting requested review from saturninoabril

toninis Awaiting requested review from toninis

Labels

4: Reviews Complete