twentyhq / twenty

Building a modern alternative to Salesforce, powered by the community.
https://twenty.com
GNU Affero General Public License v3.0
15.76k stars 1.72k forks source link

Introduction to E2E Playwright testing #6641

Open BOHEUS opened 3 weeks ago

BOHEUS commented 3 weeks ago

This issue is a parent for all future PR and issues

Description

Since more and more issues/requests appear for E2E tests, here are some things which need to be addressed before progressing:

While this shouldn't prevent from creating issues/requests for tests, this must be addressed ASAP so technical debt will be minimised

Based on all of the things described above (some of them are taken from here where I described them earlier), here's checklist

### TO DO
- [ ] Create structure and modules for each main view of the app as well as API
- [ ] Create components with locators without CSS or text references
- [ ] Create .env with all needed variables
- [ ] Create playwright.config.ts with all required options
- [ ] Analyse and create a cleaner which can run after each test and before/after all tests
- [ ] ~~Add Github workflow for running tests in CI~~ On hold
- [ ] Add proper documentation so there will be continuity in tests in case someone won't have time to maintain
- [ ] Add proper drivers for OS shell in case test will require running shell command for e.g. restarting database
- [ ] Check the scrolling behaviour during tests
- [ ] Add storage states for saving auth data after login

This checklist may be updated in the future

Materials:

BOHEUS commented 3 weeks ago

Please assign this task to me as I don't have enough permissions to do it by myself

BOHEUS commented 3 weeks ago

Cleaner can be done in 2 ways:

First option is time-consuming due to fact resetting database can take about 1-2 min, when run after each test, the amount of time spend on waiting until commands are finished might be higher than on the tests itself but the amount of code is insignificant. (OS shell driver required)

Second option is better as it allows to test additional cases like user deleting some objects within one test and the time spent on deleting objects created during the test is lower but the amount of code required to secure all cases is significantly higher compared to first option.

Both approaches should be used, first one in case there are scenarios requiring to delete objects created by default (e.g. after first login to workspace) or in case something bad happens, second by default.

BOHEUS commented 3 weeks ago

In the future when integration tests with Google will be implemented, instead of using the UI for accessing the mails and calendar, tests should use the API Google provides, this way, it'll be easier to implement (in case the account will be secured using 2FA, it'll be easier to work with) and faster (compared to normal access with UI) with the same result (question is how email can be checked whether its' content [formatted HTML] is the same as in the mockups, using built-in assert in TS?)

Of course, if the app will support at some point in the future other mail and calendar providers such as Microsoft or Apple, the general rule of thumb is to use API for reasons stated above if possible, if not (like for Proton Mail, there is Python module maintained by the company but not JS/TS library https://github.com/ProtonMail/proton-python-client), then module for specific provider should be created tests should be mocked as creating a module for provider's calendar will take too much time.

API documentation reference:

BOHEUS commented 3 weeks ago

Playwright offers a way to test REST API using built-in request class so API testing can be done within one framework. While the documentation doesn't explicitly say that it's possible to test GraphQL API, one search and blog post later it turns out to be possible without bigger problems (except for the query mashed into 1 line).

However, many people are using Postman for (automated) API testing which supports REST and GraphQL API without bigger problems (additionally, it can get GraphQL schema from the link easily) as well as additional features such as workflows, tests within API requests, exporting environments and more. While this option would be great, it's only nice to have as supporting both solutions will take too much time and effort unless better solution will be found in the future. For now, it's better to use only Playwright.

Documentation reference:

BOHEUS commented 3 weeks ago

When using app, user has to log in to its account in order to perform any action. In tests, the same logic must be saved (logging in as the first step) but that brings up a problem which is logging before each test. There are 2 solutions for this: either leave this as if and log in before every test and log out after every test (after cleaning all changes and reverting the workspace to state before the test) or use global setup (before all tests) and storage state (before every test).

The first solution mimics the user's behaviour but adds additional ~20 sec to time execution of each test, while the second solution saves that time.

The question is what about different users when role based access will be added. Storage states (second solution) seem like a good option.

Documentation:

lucasbordeau commented 2 weeks ago

@BOHEUS E2E would be really useful for example when we refactor scrolling behavior like this issue : https://github.com/twentyhq/twenty/pull/6645/

BOHEUS commented 2 weeks ago

@lucasbordeau added item to Tasklist, by default Playwright scrolls by itself but if that's not enough, it can be done by simulating mouse wheel behaviour or scrolling enough to see specific element on screen. Either way, to check and simulate this, I need to add several objects as I'm running tests from fresh setup each time.

Documentation:

BOHEUS commented 2 weeks ago

When interacting with page, users are interacting with it by clicking on elements, e.g. clicking on button to complete the transaction or on link to redirect to seemingly normal page only to be rickrolled. In tests, those elements can be located using several options such as .locator(), .getByPlaceholder(), .getByTitle() and more but it can be also done in another way. Since HTML is basically a XML-based node tree, XPath can be used to traverse through this tree and find specific element.

But XPath should be used if built-in locators into Playwright fail as last resort.

Documentation:

BOHEUS commented 2 weeks ago

Since Twenty added webhooks (essentially allowing for integration with different, not officially supported apps with Zapier), these should be tested if are working. Playwright does not offer any way to test them (nor does any other framework), thus the third-party provider should be used for this. There are plenty of sites offering intercepting/receiving webhooks, notably one of them being https://webhook.site

TBD if verifying webhooks should be done within the E2E tests.

BOHEUS commented 2 weeks ago

Found issue #5583 with interesting solution for Postman testing, if it works like described there, then should it be used together with Playwright? Writing Playwright tests for API is a good option considering fact there are no tests for API and when done correctly, they can be used inside tests, e.g. test checking if object created by API is visible in UI, has correct data, etc.

BOHEUS commented 2 weeks ago

When somewhere in the future, the need for load performance tests will appear, we should test the app under extreme conditions (one example is given in #4817). The problem appears with:

  1. generating such massive amount of data (as importing data is now very limited, one user reported problems when importing data with 4k rows to workspace #6647)
  2. testing on such huge workspace as by this point the basic assumption is more than 1 user has an account and is using app concurrently

Problem 1. can be solved by:

Problem 2. can be solved by creating realistic guidelines for such cases and modifying test to use x independent contexts (or pages if that's possible) simulating users (but here's catch, how to write test in such way that each "user" will perform on different objects so it won't lead to overwriting the same data over and over?)

BOHEUS commented 2 days ago

Something worth noting, normally when writing tests, it's normal to use the CSS selector to locate specific element IF the app doesn't change classes, in Twenty most components have 1 class which change depending on theme whether it's dark or light, essentially making them impossible to use in tests as locators. We should never resort to using CSS for this reason unless there's very specific reason (even then, we could opt out for XPath).

Also, while writing all necessary components for tests, it's possible to find a component by text inside component, but this will become a problem when localization tests will be introduced. All components in tests must be language- and theme-agnostic so there won't be a need to refactor a huge chunk of code just to allow writing localization tests and/or testing in different mode than light/dark mode.