From c22268b5221a7b65f1f57ad6d4dbb552306a0d6c Mon Sep 17 00:00:00 2001 From: Inna Ianko Date: Thu, 12 Oct 2023 15:27:21 +0300 Subject: [PATCH] [INF-1427] Add e2e style guide. (#36) * [INF-1427] Add e2e style guide. * Updates after code review. * Updates after code review 2. * Updates after code review 3. * Updates after code review 4. * Fix links --------- Co-authored-by: Inna Ianko --- README.md | 3 +- e2e-testing.md | 412 +++++++++++++++++++++++++++++++++++++++++++++++++ testing.md | 60 +++---- 3 files changed, 434 insertions(+), 41 deletions(-) create mode 100644 e2e-testing.md diff --git a/README.md b/README.md index ed8ead9..1bd6346 100644 --- a/README.md +++ b/README.md @@ -7,6 +7,7 @@ This document contains style guides used at Mate academy. For easier markdown ed - [java](./java/java.html) - [SQL](./sql/style-guide-link.md) - [Testing](./testing.md) +- [E2E Testing](./e2e-testing.md) - [LMS Tasks tests](./lms-tasks-tests.md) - [Mate API](./mate-api.md) -- [Mate Frontend](./mate-frontend.md) \ No newline at end of file +- [Mate Frontend](./mate-frontend.md) diff --git a/e2e-testing.md b/e2e-testing.md new file mode 100644 index 0000000..ccad540 --- /dev/null +++ b/e2e-testing.md @@ -0,0 +1,412 @@ +# E2E Testing Style Guide + +[go/e2e-testing-style-guide](http://go/e2e-testing-style-guide) + +- [1\. Factory classes](#1-factory-classes) + - [1.1. Use Factory classes for Entities only](#11-use-factory-classes-for-entities-only) +- [2\. Page objects](#2-page-objects) + - [2.1. Define page locators outside the constructor](#21-define-page-locators-outside-the-constructor) + - [2.2. Parametrized page URLs defining](#22-parametrized-page-urls-defining) + - [2.3. Use Components to re-use common page element groups](#23-use-components-to-re-use-common-page-element-groups) + - [2.4. Do not use dynamic values in allure step text](#24-do-not-use-dynamic-values-in-allure-step-text) + - [2.5 Use fill instead of type](#25-use-fill-instead-of-type) +- [3\. Test scenarios](#3-test-scenarios) + - [3.1. Use underscore separator for test folders naming](#31-use-underscore-separator-for-test-folders-naming) + - [3.2. Use folders for test suite organization](#32-use-folders-for-test-suite-organization) + - [3.3. Use one spec file per one test](#33-use-one-spec-file-per-one-test) + - [3.4. Before block should not contain common test data preparation](#34-before-block-should-not-contain-common-test-data-preparation) + - [3.5. Test block should only contain test steps_and_assertions](#35-test-block-should-only-contain-test-steps-and-assertions) +- [4\. Assertions](#4-assertions) + - [4.1 One assertion per one test step method](#41-one-assertion-per-one-test-step-method) + - [4.2 Use expect for assertions](#42-use-expect-for-assertions) + + +1\. Factory classes +-------------- + +#### 1.1. Use Factory classes for Entities only + +The ```FactoryItem``` class should be extended for each new Entity like User, Course, Event, StudentGroup class. +For the tables which contains additional information for the Item it's recommended to create a ```link``` method. + + +```typescript +// ✅ recommended +export class User extends FactoryItem { + + public constructor( + apiClient: SeedAPI, + options: Options, + ) { + super(apiClient); + ... + } + + async linkToCourse( + courseId: number, + status: CourseUserStatus, + ): Promise { + await this.api.seedItemIfNotExist( + FactoryItemType.CourseUsers, + { + courseId, + userId: this.id, + status, + }, + ); + } +} +``` + +2\. Page objects +-------------- + +#### 2.1. Define page locators outside the constructor + +Please define locators in the page object classes and components using the below example. +There could be different approaches, but we agreed to follow a single style within the organization. + +```typescript +// ❌ not recommended +export class SignInPage extends BasePage{ + private readonly emailField: Locator; + + constructor(page: Page) { + super(page); + this.emailField = page.getByTestId('sign-in-user-email'); + } +} + +// ✅ recommended +export class SignInPage extends BasePage{ + private readonly emailField = this.page.getByTestId('sign-in-user-email'); + + constructor(page: Page) { + super(page); + } +} +``` + + + +#### 2.2. Parametrized page URLs defining + +Use the parametrized ROUTES to define the parametrized page object URLs. + +```typescript +// ❌ not recommended +interface Options { + chatId: number; +} + +export class ChatPage extends LoggedInBasePage { + + constructor( + page: Page, + options: Options, + ) { + super(page); + + this.url = `${ROUTES.chat}\\${options.chatId}`; + } +} + +// ✅ recommended +interface Options { + chatId: number; +} + +export class ChatPage extends LoggedInBasePage { + + constructor( + page: Page, + options: Options, + ) { + super(page); + + this.url = ROUTES.chat(options.chatId).index; + } +} +``` + + +#### 2.3. Use Components to re-use common page element groups + + +Do not repeat the code for page elements. +If you defined some common elements from the header, footer, sideBar, or some popups that appear on several pages - make sure to define them in the corresponding component class. + + + +#### 2.4. Do not use dynamic values in allure step text + + +Do not use dynamic values in allure step text becuase that way new test case will be added to the Allure TestOps on each test re-run. + +```typescript +// ❌ not recommended +async typeCourseName(name: string): Promise { + await test.step(`Type course name ${name}`, async () => { + await this.courseDropwDown.type(name); + }); +} + + +// ✅ recommended +async typeCourseName(name: string): Promise { + await test.step('Type course name', async () => { + await this.courseDropwDown.type(name); + }); +} + +``` + +#### 2.5. Use fill instead of type + + +Use `fill()` instead of `type()` method, as `type()` is deprecated in [playwright](https://playwright.dev/docs/api/class-locator#locator-type). + +```typescript +// ❌ not recommended + +await this.textField.type(text); + + +// ✅ recommended + +await this.textField.fill(text); +``` + +3\. Test scenarios +-------------- + +#### 3.1. Use underscore separator for test folders naming + + +Use the underscore separator for test folder names. Never use spaces in the folders and file names. + + +```typescript +// ❌ not recommended +- tests + - e2e + - Admin Tools + - Homework-Review-Plugin + + +// ✅ recommended +- tests + - e2e + - Admin_Tools + - Homework_Review_Plugin + +``` + + +#### 3.2. Use folders for test suite organization + + +Do not add test spec files from different suites to one folder. Create separate folder for each test suite. + +```typescript +// ❌ not recommended +- tests + - e2e + - LMS_Editor + - Courses + - shouldBeCreatedWithOnlyRequiredFields.spec.ts + - shouldBeCreatedWithAllFields.spec.ts + - shouldEditOnlyRequiredFields.spec.ts + - shouldEditAllFields.spec.ts + +// ✅ recommended +- tests + - e2e + - LMS_Editor + - Courses + - New_course + - shouldBeCreatedWithOnlyRequiredFields.spec.ts + - shouldBeCreatedWithAllFields.spec.ts + - Edit_course + - shouldEditOnlyRequiredFields.spec.ts + - shouldEditAllFields.spec.ts +``` + +#### 3.3. Use one spec file per one test + + +Do not add several tests into one spec file. Each spec file should normally have one e2e test scenario. +The exception can be done for the parametrized tests. + +```typescript +// ❌ not recommended +test.describe(`New application form`, () => { + test('should allow to open the page') + test('should allow to be submitted') + test('should show the success message') + } +) + + +// ✅ recommended +test.describe(`New application form`, () => { + test('should allow to successfully submit the application by new user') + } +) + +``` + + +#### 3.4. Before block should not contain common test data preparation + +The before test block in the test spec file should generally contain only calling the fixtures and defining the shortcut constants for better test readability. +In case one need to define some additional data preparation (for example seeding some data) - one need to do this in a separate methods and fixtures which can be re-used in other tests. + + + +```typescript +// ❌ not recommended +test.beforeEach(( + { + page, + newProfessionUA, + techCheckTopicInNewCourse, + }, +) => { + const techCheckQuestion + = new TechCheckQuestion( + seedAPI, + { techCheckTopic: techCheckTopicInNewCourse }, + ); + + await techCheckQuestion.createWithEnTranslate(); + + courseName = newProfessionUA.postpaid.nameShortCode; + questionEn = techCheckQuestion.enTranslation; + + questionEditorPage = new QuestionsEditorPage(page); +}); + + +// ✅ recommended +test.beforeEach(( + { + page, + newProfessionUA, + techCheckQuestionInTopicInNewCourse, + }, +) => { + courseName = newProfessionUA.postpaid.nameShortCode; + questionEn = techCheckQuestionInTopicInNewCourse.enTranslation; + + questionEditorPage = new QuestionsEditorPage(page); +}); + +``` + +#### 3.5. Test block should only contain test steps and assertions + +Test block should contain only test-step or test-assertion methods - these are page object methods wrapped with allure steps and clearly describing user performed step or assertion. +Do not use page locators or locator action methods directly in the test spec file - add page-object method instead. + +```typescript +// ❌ not recommended +test('should allow to successfully create course', + async () => { + ... + await createCoursePage.courseField.fill(name); + ... + await createCoursePage.waitForFlashMessage('Course_succesfully_created'); + }); + + +// ✅ recommended +export class CreateCoursePage { + + async fillCourseName(name: string): Promise { + await test.step(`Fill the course name`, async () => { + await this.courseField.fill(name); + }); + } + + async assertCourseCreatedMessage(): Promise { + await test.step(`Assert course created message`, async () => { + await this.waitForFlashMessage('Course_succesfully_created'); + }); + } +} + +test('should allow to successfully create course', + async () => { + ... + await createCoursePage.fillCourseName(name); + ... + await createCoursePage.assertCourseCreatedMessage(); + }); + +``` + +4\. Assertions +-------------- + +#### 4.1. One assertion per one test step method + + +Create a separate method for each test assertion. This allows to read all the test steps at one glance and allows to automatically import clear test case steps to allure. + + +```typescript +// ❌ not recommended +export class QuestionsEditorPage { + async assertQuestionAdded(question: string): Promise { + await test.step(`Assert question added`, async () => { + await this.assertFlashMessage('editor.question_successfully_created'); + + await expect(this.questionInList.getByText(question)).toBeVisible(); + }); + } +} + +// ✅ recommended +export class QuestionsEditorPage { + + async assertQuestionCreatedSuccessMessage(): Promise { + await this.assertFlashMessage('editor.question_successfully_created'); + } + + async assertQuestionIsPresentInTheList(question: string): Promise { + await test.step(`Assert question is present in the list`, async () => { + await expect(this.questionInList.getByText(question)).toBeVisible(); + }); + } +} + +``` + +#### 4.2. Use expect for assertions + + +Always use expect for assertions. + +```typescript +// ❌ not recommended +export class QuestionsEditorPage { + + async assertQuestionIsPresentInTheList(question: string): Promise { + await test.step(`Assert question is present in the list`, async () => { + await this.questionInList.getByText(question).isVisible(); + }); + } +} + +// ✅ recommended +export class QuestionsEditorPage { + + async assertQuestionIsPresentInTheList(question: string): Promise { + await test.step(`Assert question is present in the list`, async () => { + await expect(this.questionInList.getByText(question)).toBeVisible(); + }); + } +} + +``` diff --git a/testing.md b/testing.md index d0b1604..ba7328a 100644 --- a/testing.md +++ b/testing.md @@ -11,28 +11,25 @@ - [1.6. Use multiple describe blocks if you test different things](#16-use-multiple-describe-blocks-if-you-test-different-things) - [1.7. Backend: Use test factories instead of creating instances explicitly](#17-backend-use-test-factories-instead-of-creating-instances-explicitly) - [1.8. Backend: avoid fetching values from database in tests using ORM](#18-backend-avoid-fetching-values-from-database-in-tests-using-orm) -- [2\. E2E Test scenarios](#2-e2e-test-scenarios) - - [2.1 Allure attributes](#21-allure-attributes) - - [2.2 Adding allure steps wrappers](#22-adding-allure-steps-wrappers) 1\. Test cases -------------- #### 1.1. Single test case per scenario - + When you add a test ask yourself if this test is required and why. Is there any other test checking the same functionality? If you can't find a good reason for the test to exist - don't write it. - + #### 1.2. Valid and short test case - + There should be only necessary and valid steps in a single test case. If a single test case contains too many test steps this may lose its aim. - + ```javascript // ❌ bad @@ -61,15 +58,15 @@ it(`should return a negative number when adding negative and zero`, () => { }); ``` - + #### 1.3. Fill in the 'describe' block - + Write a module name that is tested in the **describe** block for the unit and integration test cases. - + ```javascript // ❌ bad @@ -87,15 +84,15 @@ describe(`Function 'sum':`, () => { }); ``` - + #### 1.4. Test case descriptions should follow a pattern: - + `should [EXPECTED_RESULT] when [STATE]`. With filled in `describe` block each test case description should start with lowercase. - + ```javascript // ❌ bad @@ -105,11 +102,11 @@ it('Works without arguments', () => {}) it('should return 0 when called without arguments', () => {}) ``` - + #### 1.5. Every single test case should explain what `should be done`. - + ```javascript // ❌ bad @@ -125,7 +122,7 @@ it(`should return 0 if called without arguments`, () => {}); #### 1.6. Use multiple describe blocks if you test different things - + ```javascript // ❌ bad @@ -144,19 +141,19 @@ describe('FlyingRobot class', () => { }); ``` - + #### 1.7. Backend: Use test factories instead of creating instances explicitly - + Why? To be tool-agnostic. It's possible to update factories in one place instead of checking every test-case separately - + **💡 Note:** If your project doesn't have factories infrastructure, time to create it - + ```javascript // ❌ bad @@ -166,15 +163,15 @@ const user = await User.create(); const user = await userFactory.create(); ``` - + #### 1.8. Backend: avoid fetching values from database in tests using ORM - + Why? Again, to be tool-agnostic and don't rely on existing infrastructure. One day ORM may be changed which may lead to refactoring the whole tests infrastructure - + ```javascript await client.signUp({ email: 'bob@gmail.com' }); @@ -185,20 +182,3 @@ const user = await UserModel.findOne({ where: { email: 'bob@gmail.com' }}) // di // ✅ good const user = await client.getUser({ email: 'bob@gmail.com' }) // use API client to make sure user is created ``` - -2\. E2E Test scenarios ----------------------- - - - -### 2.1 Allure attributes - - - -Please see the ["3. Adding attributes to the automated tests"](https://app.clickup.com/24383048/v/dc/q83j8-12520/q83j8-5980?&block=block-f46d3fd3-0ebe-4763-b915-da8b7ba51819) instruction on which allure attributes need to be added to the E2E tests. - - - -### 2.2 Adding allure steps wrappers - -Please see the ["4.1 Creating steps"](https://app.clickup.com/24383048/v/dc/q83j8-12520/q83j8-5980?&block=block-a08386b0-b139-41ce-826a-5a96d4f3215b) instruction on how to add cypress allure step wrappers. \ No newline at end of file