diff options
Diffstat (limited to 'doc/development/testing_guide')
| -rw-r--r-- | doc/development/testing_guide/best_practices.md | 24 | ||||
| -rw-r--r-- | doc/development/testing_guide/ci.md | 1 | ||||
| -rw-r--r-- | doc/development/testing_guide/end_to_end/index.md | 29 | ||||
| -rw-r--r-- | doc/development/testing_guide/end_to_end/page_objects.md | 41 | ||||
| -rw-r--r-- | doc/development/testing_guide/end_to_end/quick_start_guide.md | 77 | ||||
| -rw-r--r-- | doc/development/testing_guide/end_to_end/style_guide.md | 61 | ||||
| -rw-r--r-- | doc/development/testing_guide/flaky_tests.md | 4 | ||||
| -rw-r--r-- | doc/development/testing_guide/frontend_testing.md | 572 | ||||
| -rw-r--r-- | doc/development/testing_guide/img/qa_on_merge_requests_cicd_architecture.png | bin | 64862 -> 0 bytes | |||
| -rw-r--r-- | doc/development/testing_guide/img/review_apps_cicd_architecture.png | bin | 136431 -> 0 bytes | |||
| -rw-r--r-- | doc/development/testing_guide/index.md | 18 | ||||
| -rw-r--r-- | doc/development/testing_guide/review_apps.md | 66 | ||||
| -rw-r--r-- | doc/development/testing_guide/testing_levels.md | 3 |
13 files changed, 743 insertions, 153 deletions
diff --git a/doc/development/testing_guide/best_practices.md b/doc/development/testing_guide/best_practices.md index 448d9fd01c4..f30a83a4c71 100644 --- a/doc/development/testing_guide/best_practices.md +++ b/doc/development/testing_guide/best_practices.md @@ -15,16 +15,6 @@ manifest themselves within our code. When designing our tests, take time to revi our test design. We can find some helpful heuristics documented in the Handbook in the [Test Design](https://about.gitlab.com/handbook/engineering/quality/guidelines/test-engineering/test-design/) section. -## Run tests against MySQL - -By default, tests are only run against PostgreSQL, but you can run them on -demand against MySQL by following one of the following conventions: - -| Convention | Valid example | -|:----------------------|:-----------------------------| -| Include `mysql` in your branch name | `enhance-mysql-support` | -| Include `[run mysql]` in your commit message | `Fix MySQL support<br><br>[run mysql]` | - ## Test speed GitLab has a massive test suite that, without [parallelization], can take hours @@ -70,6 +60,7 @@ bundle exec rspec spec/[path]/[to]/[spec].rb - On `before` and `after` hooks, prefer it scoped to `:context` over `:all` - When using `evaluate_script("$('.js-foo').testSomething()")` (or `execute_script`) which acts on a given element, use a Capyabara matcher beforehand (e.g. `find('.js-foo')`) to ensure the element actually exists. +- Use `focus: true` to isolate parts of the specs you want to run. [four-phase-test]: https://robots.thoughtbot.com/four-phase-test @@ -454,6 +445,19 @@ complexity of RSpec expectations.They should be placed under a certain type of specs only (e.g. features, requests etc.) but shouldn't be if they apply to multiple type of specs. +#### `be_like_time` + +Time returned from a database can differ in precision from time objects +in Ruby, so we need flexible tolerances when comparing in specs. We can +use `be_like_time` to compare that times are within one second of each +other. + +Example: + +```ruby +expect(metrics.merged_at).to be_like_time(time) +``` + #### `have_gitlab_http_status` Prefer `have_gitlab_http_status` over `have_http_status` because the former diff --git a/doc/development/testing_guide/ci.md b/doc/development/testing_guide/ci.md index 87d48726268..d9f66a827de 100644 --- a/doc/development/testing_guide/ci.md +++ b/doc/development/testing_guide/ci.md @@ -39,7 +39,6 @@ slowest test files and try to improve them. ## CI setup -- On CE and EE, the test suite runs both PostgreSQL and MySQL. - Rails logging to `log/test.log` is disabled by default in CI [for performance reasons][logging]. To override this setting, provide the `RAILS_ENABLE_TEST_LOG` environment variable. diff --git a/doc/development/testing_guide/end_to_end/index.md b/doc/development/testing_guide/end_to_end/index.md index 59eb3ecfd7e..3ae3ce183d9 100644 --- a/doc/development/testing_guide/end_to_end/index.md +++ b/doc/development/testing_guide/end_to_end/index.md @@ -45,11 +45,11 @@ Results are reported in the `#qa-staging` Slack channel. ### Testing code in merge requests -#### Using the `package-and-qa` job +#### Using the `package-and-qa-manual` job It is possible to run end-to-end tests for a merge request, eventually being run in a pipeline in the [`gitlab-qa`](https://gitlab.com/gitlab-org/gitlab-qa/) project, -by triggering the `package-and-qa` manual action in the `test` stage (not +by triggering the `package-and-qa-manual` manual action in the `test` stage (not available for forks). **This runs end-to-end tests against a custom Omnibus package built from your @@ -65,28 +65,23 @@ Below you can read more about how to use it and how does it work. Currently, we are using _multi-project pipeline_-like approach to run QA pipelines. - - -<details> -<summary>Show mermaid source</summary> -<pre> +```mermaid graph LR A1 -.->|1. Triggers an omnibus-gitlab pipeline and wait for it to be done| A2 - B2[<b>`Trigger-qa` stage</b><br />`Trigger:qa-test` job] -.->|2. Triggers a gitlab-qa pipeline and wait for it to be done| A3 + B2[`Trigger-qa` stage<br>`Trigger:qa-test` job] -.->|2. Triggers a gitlab-qa pipeline and wait for it to be done| A3 -subgraph gitlab-ce/ee pipeline - A1[<b>`test` stage</b><br />`package-and-qa` job] +subgraph "gitlab-ce/ee pipeline" + A1[`test` stage<br>`package-and-qa-manual` job] end -subgraph omnibus-gitlab pipeline - A2[<b>`Trigger-docker` stage</b><br />`Trigger:gitlab-docker` job] -->|once done| B2 +subgraph "omnibus-gitlab pipeline" + A2[`Trigger-docker` stage<br>`Trigger:gitlab-docker` job] -->|once done| B2 end -subgraph gitlab-qa pipeline - A3>QA jobs run] -.->|3. Reports back the pipeline result to the `package-and-qa` job<br />and post the result on the original commit tested| A1 +subgraph "gitlab-qa pipeline" + A3>QA jobs run] -.->|3. Reports back the pipeline result to the `package-and-qa-manual` job<br>and post the result on the original commit tested| A1 end -</pre> -</details> +``` 1. Developer triggers a manual action, that can be found in CE / EE merge requests. This starts a chain of pipelines in multiple projects. @@ -148,7 +143,7 @@ Once you decided where to put [test environment orchestration scenarios] and the [GitLab QA orchestrator README][gitlab-qa-readme], and [the already existing instance-level scenarios][instance-level scenarios]. -Continued reading: +Continued reading: - [Quick Start Guide](quick_start_guide.md) - [Style Guide](style_guide.md) diff --git a/doc/development/testing_guide/end_to_end/page_objects.md b/doc/development/testing_guide/end_to_end/page_objects.md index 05cb03eb4bd..850ea6b60ac 100644 --- a/doc/development/testing_guide/end_to_end/page_objects.md +++ b/doc/development/testing_guide/end_to_end/page_objects.md @@ -27,7 +27,7 @@ When someone later changes `t.text_field :login` in the view associated with this page to `t.text_field :username` it will generate a different field identifier, what would effectively break all tests. -Because we are using `Page::Main::Login.act { sign_in_using_credentials }` +Because we are using `Page::Main::Login.perform(&:sign_in_using_credentials)` everywhere, when we want to sign into GitLab, the page object is the single source of truth, and we will need to update `fill_in :user_login` to `fill_in :user_username` only in a one place. @@ -40,7 +40,7 @@ the time it would take to build packages and test everything. That is why when someone changes `t.text_field :login` to `t.text_field :username` in the _new session_ view we won't know about this change until our GitLab QA nightly pipeline fails, or until someone triggers -`package-and-qa` action in their merge request. +`package-and-qa-manual` action in their merge request. Obviously such a change would break all tests. We call this problem a _fragile tests problem_. @@ -92,20 +92,25 @@ end The `view` DSL method will correspond to the rails View, partial, or vue component that renders the elements. The `element` DSL method in turn declares an element for which a corresponding -`qa-element-name-dasherized` CSS class will need to be added to the view file. +`data-qa-selector=element_name_snaked` data attribute will need to be added to the view file. You can also define a value (String or Regexp) to match to the actual view code but **this is deprecated** in favor of the above method for two reasons: - Consistency: there is only one way to define an element -- Separation of concerns: QA uses dedicated CSS classes instead of reusing code +- Separation of concerns: QA uses dedicated `data-qa-*` attributes instead of reusing code or classes used by other components (e.g. `js-*` classes etc.) ```ruby view 'app/views/my/view.html.haml' do - # Implicitly require `.qa-logout-button` CSS class to be present in the view + + ### Good ### + + # Implicitly require the CSS selector `[data-qa-selector="logout_button"]` to be present in the view element :logout_button + ### Bad ### + ## This is deprecated and forbidden by the `QA/ElementWithPattern` RuboCop cop. # Require `f.submit "Sign in"` to be present in `my/view.html.haml element :my_button, 'f.submit "Sign in"' # rubocop:disable QA/ElementWithPattern @@ -129,24 +134,38 @@ view 'app/views/my/view.html.haml' do end ``` -To add these elements to the view, you must change the rails View, partial, or vue component by adding a `qa-element-descriptor` class +To add these elements to the view, you must change the rails View, partial, or vue component by adding a `data-qa-selector` attribute for each element defined. -In our case, `qa-login-field`, `qa-password-field` and `qa-sign-in-button` +In our case, `data-qa-selector="login_field"`, `data-qa-selector="password_field"` and `data-qa-selector="sign_in_button"` **app/views/my/view.html.haml** ```haml -= f.text_field :login, class: "form-control top qa-login-field", autofocus: "autofocus", autocapitalize: "off", autocorrect: "off", required: true, title: "This field is required." -= f.password_field :password, class: "form-control bottom qa-password-field", required: true, title: "This field is required." -= f.submit "Sign in", class: "btn btn-success qa-sign-in-button" += f.text_field :login, class: "form-control top", autofocus: "autofocus", autocapitalize: "off", autocorrect: "off", required: true, title: "This field is required.", data: { qa_selector: 'login_field' } += f.password_field :password, class: "form-control bottom", required: true, title: "This field is required.", data: { qa_selector: 'password_field' } += f.submit "Sign in", class: "btn btn-success", data: { qa_selector: 'sign_in_button' } ``` Things to note: -- The CSS class must be `kebab-cased` (separated with hyphens "`-`") +- The name of the element and the qa_selector must match and be snake_cased - If the element appears on the page unconditionally, add `required: true` to the element. See [Dynamic element validation](dynamic_element_validation.md) +- You may see `.qa-selector` classes in existing Page Objects. We should prefer the [`data-qa-selector`](#data-qa-selector-vs-qa-selector) + method of definition over the `.qa-selector` CSS class + +### `data-qa-selector` vs `.qa-selector` + +> Introduced in GitLab 12.1 + +There are two supported methods of defining elements within a view. + +1. `data-qa-selector` attribute +1. `.qa-selector` class + +Any existing `.qa-selector` class should be considered deprecated +and we should prefer the `data-qa-selector` method of definition. ## Running the test locally diff --git a/doc/development/testing_guide/end_to_end/quick_start_guide.md b/doc/development/testing_guide/end_to_end/quick_start_guide.md index 064fb0e31dd..e1df8be8b6f 100644 --- a/doc/development/testing_guide/end_to_end/quick_start_guide.md +++ b/doc/development/testing_guide/end_to_end/quick_start_guide.md @@ -101,7 +101,7 @@ it 'replaces an existing label if it has the same key' do page.find('#content-body').click page.refresh - labels_block = page.find('.qa-labels-block') + labels_block = page.find(%q([data-qa-selector="labels_block"])) expect(labels_block).to have_content('animal::dolphin') expect(labels_block).not_to have_content('animal::fox') @@ -110,15 +110,15 @@ end ``` > Notice that the test itself is simple. The most challenging part is the creation of the application state, which will be covered later. - +> > The exemplified test case's MVC is not enough for the change to be merged, but it helps to build up the test logic. The reason is that we do not want to use locators directly in the tests, and tests **must** use [Page Objects] before they can be merged. This way we better separate the responsibilities, where the Page Objects encapsulate elements and methods that allow us to interact with pages, while the spec files describe the test cases in more business-related language. Below are the steps that the test covers: 1. The test finds the 'Edit' link for the labels and clicks on it. -2. Then it fills in the 'Assign labels' input field with the value 'animal::dolphin' and press enters. -3. Then it clicks in the content body to apply the label and refreshes the page. -4. Finally, the expectations check that the previous scoped label was removed and that the new one was added. +1. Then it fills in the 'Assign labels' input field with the value 'animal::dolphin' and press enters. +1. Then it clicks in the content body to apply the label and refreshes the page. +1. Finally, the expectations check that the previous scoped label was removed and that the new one was added. Let's now see how the second test case would look. @@ -130,7 +130,7 @@ it 'keeps both scoped labels when adding a label with a different key' do page.find('#content-body').click page.refresh - labels_block = page.find('.qa-labels-block') + labels_block = page.find(%q([data-qa-selector="labels_block"])) expect(labels_block).to have_content('animal::fox') expect(labels_block).to have_content('plant::orchid') @@ -139,14 +139,14 @@ it 'keeps both scoped labels when adding a label with a different key' do end ``` -> Note that elements are always located using CSS selectors, and a good practice is to add test-specific selectors (this is called adding testability to the application and we will talk more about it later.) For example, the `labels_block` element uses the selector `.qa-labels-block`, which was added specifically for testing purposes. +> Note that elements are always located using CSS selectors, and a good practice is to add test-specific selectors (this is called "testability"). For example, the `labels_block` element uses the CSS selector [`data-qa-selector="labels_block"`](page_objects.md#data-qa-selector-vs-qa-selector), which was added specifically for testing purposes. Below are the steps that the test covers: 1. The test finds the 'Edit' link for the labels and clicks on it. -2. Then it fills in the 'Assign labels' input field with the value 'plant::orchid' and press enters. -3. Then it clicks in the content body to apply the label and refreshes the page. -4. Finally, the expectations check that both scoped labels are present. +1. Then it fills in the 'Assign labels' input field with the value 'plant::orchid' and press enters. +1. Then it clicks in the content body to apply the label and refreshes the page. +1. Finally, the expectations check that both scoped labels are present. > Similar to the previous test, this one is also very straightforward, but there is some code duplication. Let's address it. @@ -168,7 +168,7 @@ end it 'replaces an existing label if it has the same key' do select_label_and_refresh @new_label_same_scope - labels_block = page.find('.qa-labels-block') + labels_block = page.find(%q([data-qa-selector="labels_block"])) expect(labels_block).to have_content(@new_label_same_scope) expect(labels_block).not_to have_content(@initial_label) @@ -179,7 +179,7 @@ end it 'keeps both scoped label when adding a label with a different key' do select_label_and_refresh @new_label_different_scope - labels_block = page.find('.qa-labels-block') + labels_block = page.find(%q([data-qa-selector="labels_block"])) expect(labels_blocks).to have_content(@new_label_different_scope) expect(labels_blocks).to have_content(@initial_label) @@ -211,7 +211,7 @@ A pre-condition for the entire test suite is defined in the `before :context` bl > For our test suite, due to the need of the tests being completely independent of each other, we won't use the `before :context` block. The `before :context` block would make the tests dependent on each other because the first test changes the label of the issue, and the second one depends on the `'animal::fox'` label being set. -> **Tip:** In case of a test suite with only one `it` block it's ok to use only the `before` block (see below) with all the test's pre-conditions. +TIP: **Tip:** In case of a test suite with only one `it` block it's ok to use only the `before` block (see below) with all the test's pre-conditions. #### `before` @@ -222,7 +222,7 @@ As the pre-conditions for our test suite, the things that needs to happen before - A project being created with an issue and labels already set; - The issue page being opened with only one scoped label applied to it. -> When running end-to-end tests as part of the GitLab's continuous integration process [a license is already set as an environment variable](https://gitlab.com/gitlab-org/gitlab-ee/blob/1a60d926740db10e3b5724713285780a4f470531/qa/qa/ee/strategy.rb#L20). For running tests locally you can set up such license by following the document [what tests can be run?](https://gitlab.com/gitlab-org/gitlab-qa/blob/master/docs/what_tests_can_be_run.md#supported-remote-grid-environment-variables), based on the [supported GitLab environment variables](https://gitlab.com/gitlab-org/gitlab-qa/blob/master/docs/what_tests_can_be_run.md#supported-gitlab-environment-variables). +> When running end-to-end tests as part of the GitLab's continuous integration process [a license is already set as an environment variable](https://gitlab.com/gitlab-org/gitlab-ee/blob/1a60d926740db10e3b5724713285780a4f470531/qa/qa/ee/strategy.rb#L20). For running tests locally you can set up such license by following the document [what tests can be run?](https://gitlab.com/gitlab-org/gitlab-qa/blob/master/docs/what_tests_can_be_run.md), based on the [supported GitLab environment variables](https://gitlab.com/gitlab-org/gitlab-qa/blob/master/docs/what_tests_can_be_run.md#supported-gitlab-environment-variables). #### Implementation @@ -274,11 +274,11 @@ end In the `before` block we create all the application state needed for the tests to run. We do that by using the `Runtime::Browser.visit` method to go to the login page, by performing a `sign_in_using_credentials` from the `Login` Page Object, by fabricating resources via APIs (`issue`, and `Resource::Label`), and by using the `issue.visit!` to visit the issue page. > A project is created in the background by creating the `issue` resource. - +> > When creating the [Resources], notice that when calling the `fabricate_via_api` method, we pass some attribute:values, like `title`, and `labels` for the `issue` resource; and `project` and `title` for the `label` resource. - +> > What's important to understand here is that by creating the application state mostly using the public APIs we save a lot of time in the test suite setup stage. - +> > Soon we will cover the use of the already existing resources' methods and the creation of your own `fabricate_via_api` methods for resources where this is still not available, but first, let's optimize our implementation. ### 6. Optimization @@ -290,7 +290,7 @@ As already mentioned in the [best practices](best_practices.md) document, end-to Some improvements that we could make in our test suite to optimize its time to run are: 1. Having a single test case (an `it` block) that exercises both scenarios to avoid "wasting" time in the tests' pre-conditions, instead of having two different test cases. -2. Making the selection of labels more performant by allowing for the selection of more than one label in the same reusable method. +1. Making the selection of labels more performant by allowing for the selection of more than one label in the same reusable method. Let's look at a suggestion that addresses the above points, one by one: @@ -305,7 +305,7 @@ module QA it 'correctly applies scoped labels depending on if they are from the same or a different scope' do select_labels_and_refresh [@new_label_same_scope, @new_label_different_scope] - labels_block = page.all('.qa-labels-block') + labels_block = page.all(%q([data-qa-selector="labels_block"])) expect(labels_block).to have_content(@new_label_same_scope) expect(labels_block).to have_content(@new_label_different_scope) @@ -332,8 +332,8 @@ To address point 1, we changed the test implementation from two `it` blocks into > Notice that the implementation of the new and unique `it` block had to change a little bit. Below we describe in details what it does. 1. It selects two scoped labels simultaneously, one from the same scope of the one already applied in the issue during the setup phase (in the `before` block), and another one from a different scope. -2. It asserts that the correct labels are visible in the `labels_block`, and that the labels were correctly added and removed; -3. Finally, the `select_label_and_refresh` method is changed to `select_labels_and_refresh`, which accepts an array of labels instead of a single label, and it iterates on them for faster label selection (this is what is used in step 1 explained above.) +1. It asserts that the correct labels are visible in the `labels_block`, and that the labels were correctly added and removed; +1. Finally, the `select_label_and_refresh` method is changed to `select_labels_and_refresh`, which accepts an array of labels instead of a single label, and it iterates on them for faster label selection (this is what is used in step 1 explained above.) ### 7. Resources @@ -362,7 +362,7 @@ First, in the [issue resource](https://gitlab.com/gitlab-org/gitlab-ee/blob/d358 Add the following `attribute :id` and `attribute :labels` right above the [`attribute :title`](https://gitlab.com/gitlab-org/gitlab-ee/blob/d3584e80b4236acdf393d815d604801573af72cc/qa/qa/resource/issue.rb#L15). > This line is needed to allow for the issue fabrication, and for labels to be automatically added to the issue when fabricating it via API. - +> > We add the attributes above the existing attribute to keep them alphabetically organized. Then, let's initialize an instance variable for labels to allow an empty array as default value when such information is not passed during the resource fabrication, since this optional. [Between the attributes and the `fabricate!` method](https://gitlab.com/gitlab-org/gitlab-ee/blob/1a1f1408728f19b2aa15887cd20bddab7e70c8bd/qa/qa/resource/issue.rb#L18), add the following: @@ -437,7 +437,7 @@ By defining the `resource_web_url(resource)` method, we override the one from th By defining the `api_get_path` method, we **would** allow for the [`ApiFabricator`](https://gitlab.com/gitlab-org/gitlab-ee/blob/master/qa/qa/resource/api_fabricator.rb) module to know which path to use to get a single label, but since there's no path available for that in the publich API, we raise a `NotImplementedError` instead. -By defining the `api_post_path` method, we allow for the [`ApiFabricator `](https://gitlab.com/gitlab-org/gitlab-ee/blob/master/qa/qa/resource/api_fabricator.rb) module to know which path to use to create a new label in a specific project. +By defining the `api_post_path` method, we allow for the [`ApiFabricator`](https://gitlab.com/gitlab-org/gitlab-ee/blob/master/qa/qa/resource/api_fabricator.rb) module to know which path to use to create a new label in a specific project. By defining the `api_post_body` method, we we allow for the [`ApiFabricator.api_post`](https://gitlab.com/gitlab-org/gitlab-ee/blob/a9177ca1812bac57e2b2fa4560e1d5dd8ffac38b/qa/qa/resource/api_fabricator.rb#L68) method to know which data to send when making the `POST` request. @@ -542,9 +542,9 @@ end Notice that we have not only moved the `select_labels_and_refresh` method, but we have also changed its implementation to: 1. Click the `:edit_link_labels` element previously defined, instead of using `find('.block.labels .edit-link').click` -2. Use `within_element(:dropdown_menu_labels, text: label)`, and inside of it, we call `send_keys_to_element(:dropdown_input_field, [label, :enter])`, which is a method that we will implement in the `QA::Page::Base` class to replace `find('.dropdown-menu-labels .dropdown-input-field').send_keys [label, :enter]` -3. Use `click_body` after iterating on each label, instead of using `find('#content-body').click` -4. Iterate on every label again, and then we use `has_element?(:labels_block, text: label)` after clicking the page body (which applies the labels), and before refreshing the page, to avoid test flakiness due to refreshing too fast. +1. Use `within_element(:dropdown_menu_labels, text: label)`, and inside of it, we call `send_keys_to_element(:dropdown_input_field, [label, :enter])`, which is a method that we will implement in the `QA::Page::Base` class to replace `find('.dropdown-menu-labels .dropdown-input-field').send_keys [label, :enter]` +1. Use `click_body` after iterating on each label, instead of using `find('#content-body').click` +1. Iterate on every label again, and then we use `has_element?(:labels_block, text: label)` after clicking the page body (which applies the labels), and before refreshing the page, to avoid test flakiness due to refreshing too fast. ##### Details of `text_of_labels_block` @@ -552,37 +552,36 @@ The `text_of_labels_block` method is a simple method that returns the `:labels_b #### Updates in the view (*.html.haml) and `dropdowns_helper.rb` files -Now let's change the view and the `dropdowns_helper` files to add the selectors that relate to the Page Object. +Now let's change the view and the `dropdowns_helper` files to add the selectors that relate to the [Page Objects]. -In the [app/views/shared/issuable/_sidebar.html.haml](https://gitlab.com/gitlab-org/gitlab-ee/blob/master/app/views/shared/issuable/_sidebar.html.haml) file, on [line 105 ](https://gitlab.com/gitlab-org/gitlab-ee/blob/84043fa72ca7f83ae9cde48ad670e6d5d16501a3/app/views/shared/issuable/_sidebar.html.haml#L105), add an extra class `qa-edit-link-labels`. +In [`app/views/shared/issuable/_sidebar.html.haml:105`](https://gitlab.com/gitlab-org/gitlab-ee/blob/7ca12defc7a965987b162a6ebef302f95dc8867f/app/views/shared/issuable/_sidebar.html.haml#L105), add a `data: { qa_selector: 'edit_link_labels' }` data attribute. The code should look like this: ```haml -= link_to _('Edit'), '#', class: 'js-sidebar-dropdown-toggle edit-link float-right qa-edit-link-labels' += link_to _('Edit'), '#', class: 'js-sidebar-dropdown-toggle edit-link float-right', data: { qa_selector: 'edit_link_labels' } ``` -In the same file, on [line 121](https://gitlab.com/gitlab-org/gitlab-ee/blob/84043fa72ca7f83ae9cde48ad670e6d5d16501a3/app/views/shared/issuable/_sidebar.html.haml#L121), add an extra class `.qa-dropdown-menu-labels`. +In the same file, on [line 121](https://gitlab.com/gitlab-org/gitlab-ee/blob/7ca12defc7a965987b162a6ebef302f95dc8867f/app/views/shared/issuable/_sidebar.html.haml#L121), add a `data: { qa_selector: 'dropdown_menu_labels' }` data attribute. The code should look like this: ```haml -.dropdown-menu.dropdown-select.dropdown-menu-paging.dropdown-menu-labels.dropdown-menu-selectable.qa-dropdown-menu-labels +.dropdown-menu.dropdown-select.dropdown-menu-paging.dropdown-menu-labels.dropdown-menu-selectable.dropdown-extended-height{ data: { qa_selector: 'dropdown_menu_labels' } } ``` -In the [`dropdowns_helper.rb`](https://gitlab.com/gitlab-org/gitlab-ee/blob/master/app/helpers/dropdowns_helper.rb) file, on [line 94](https://gitlab.com/gitlab-org/gitlab-ee/blob/99e51a374f2c20bee0989cac802e4b5621f72714/app/helpers/dropdowns_helper.rb#L94), add an extra class `qa-dropdown-input-field`. +In [`app/helpers/dropdowns_helper.rb:94`](https://gitlab.com/gitlab-org/gitlab-ee/blob/7ca12defc7a965987b162a6ebef302f95dc8867f/app/helpers/dropdowns_helper.rb#L94), add a `data: { qa_selector: 'dropdown_input_field' }` data attribute. The code should look like this: ```ruby -filter_output = search_field_tag search_id, nil, class: "dropdown-input-field qa-dropdown-input-field", placeholder: placeholder, autocomplete: 'off' +filter_output = search_field_tag search_id, nil, class: "dropdown-input-field", placeholder: placeholder, autocomplete: 'off', data: { qa_selector: 'dropdown_input_field' } ``` -> Classes starting with `qa-` are used for testing purposes only, and by defining such classes in the elements we add **testability** in the application. - -> When defining a class like `qa-labels-block`, it is transformed into `:labels_block` for usage in the Page Objects. So, `qa-edit-link-labels` is transformed into `:edit_link_labels`, `qa-dropdown-menu-labels` is transformed into `:dropdown_menu_labels`, and `qa-dropdown-input-field` is transformed into `:dropdown_input_field`. Also, we use a [sanity test](https://gitlab.com/gitlab-org/gitlab-ce/tree/master/qa/qa/page#how-did-we-solve-fragile-tests-problem) to check that defined elements have their respective `qa-` selectors in the specified views. - -> We did not define the `qa-labels-block` class in the `app/views/shared/issuable/_sidebar.html.haml` file because it was already there to be used. +> `data-qa-*` data attributes and CSS classes starting with `qa-` are used solely for the purpose of QA and testing. +> By defining these, we add **testability** to the application. +> +> When defining a data attribute like: `qa_selector: 'labels_block'`, it should match the element definition: `element :labels_block`. We use a [sanity test](https://gitlab.com/gitlab-org/gitlab-ce/tree/master/qa/qa/page#how-did-we-solve-fragile-tests-problem) to check that defined elements have their respective selectors in the specified views. #### Updates in the `QA::Page::Base` class @@ -600,8 +599,6 @@ This method receives an element (`name`) and the `keys` that it will send to tha As you might remember, in the Issue Page Object we call this method like this: `send_keys_to_element(:dropdown_input_field, [label, :enter])`. -___ - With that, you should be able to start writing end-to-end tests yourself. *Congratulations!* [Page Objects]: page_objects.md diff --git a/doc/development/testing_guide/end_to_end/style_guide.md b/doc/development/testing_guide/end_to_end/style_guide.md index 0272e1810f2..97560e616a1 100644 --- a/doc/development/testing_guide/end_to_end/style_guide.md +++ b/doc/development/testing_guide/end_to_end/style_guide.md @@ -45,7 +45,7 @@ Notice that in the above example, before clicking the `:operations_environments_ > We can create these methods as helpers to abstract multi-step navigation. -### Element naming convention +## Element naming convention When adding new elements to a page, it's important that we have a uniform element naming convention. @@ -63,17 +63,17 @@ We follow a simple formula roughly based on hungarian notation. - `_checkbox` - `_radio` - `_content` - + *Note: This list is a work in progress. This list will eventually be the end-all enumeration of all available types. I.e., any element that does not end with something in this list is bad form.* - -#### Examples + +### Examples **Good** ```ruby view '...' do - element :edit_button + element :edit_button element :notes_tab element :squash_checkbox element :username_field @@ -84,16 +84,61 @@ end **Bad** ```ruby -view '...' do +view '...' do # `_confirmation` should be `_field`. what sort of confirmation? a checkbox confirmation? no real way to disambiguate. # an appropriate replacement would be `element :password_confirmation_field` element :password_confirmation - # `clone_options` is too vague. If it's a dropdown menu, it should be `clone_dropdown`. + # `clone_options` is too vague. If it's a dropdown menu, it should be `clone_dropdown`. # If it's a checkbox, it should be `clone_checkbox` element :clone_options - + # how is this url being displayed? is it a textbox? a simple span? + # If it is content on the page, it should be `ssh_clone_url_content` element :ssh_clone_url end ``` + +## Block argument naming + +To have a standard on how we call pages when using the `.perform` method, we use the name of page object being called, all lowercased, and separated by underscore, if needed (see good and bad examples below.) This also applies to resources. We chose not to simply use `page` because that would shadow the Capybara DSL, potentially leading to confusion and bugs. + +### Examples + +**Good** + +```ruby +# qa/specs/features/browser_ui/1_manage/project/add_project_member_spec.rb + +Page::Project::Settings::Members.perform do |members| + members.do_something +end +``` + +```ruby +# qa/specs/features/ee/browser_ui/3_create/merge_request/add_batch_comments_in_merge_request_spec.rb + +Resource::MergeRequest.fabricate! do |merge_request| + merge_request.do_something_else +end +``` + +**Bad** + +```ruby +# qa/specs/features/browser_ui/1_manage/project/add_project_member_spec.rb + +Page::Project::Settings::Members.perform do |project_settings_members_page| + project_settings_members_page.do_something +end +``` + +```ruby +# qa/specs/features/ee/browser_ui/3_create/merge_request/add_batch_comments_in_merge_request_spec.rb + +Resource::MergeRequest.fabricate! do |merge_request_page| + merge_request_page.do_something_else +end +``` + +> Besides the advantage of having a standard in place, by following this standard we also write shorter lines of code.
\ No newline at end of file diff --git a/doc/development/testing_guide/flaky_tests.md b/doc/development/testing_guide/flaky_tests.md index 931cbc51cae..eb0bf6fc563 100644 --- a/doc/development/testing_guide/flaky_tests.md +++ b/doc/development/testing_guide/flaky_tests.md @@ -35,8 +35,8 @@ Once a test is in quarantine, there are 3 choices: Quarantined tests are run on the CI in dedicated jobs that are allowed to fail: -- `rspec-pg-quarantine` and `rspec-mysql-quarantine` (CE & EE) -- `rspec-pg-quarantine-ee` and `rspec-mysql-quarantine-ee` (EE only) +- `rspec-pg-quarantine` (CE & EE) +- `rspec-pg-quarantine-ee` (EE only) ## Automatic retries and flaky tests detection diff --git a/doc/development/testing_guide/frontend_testing.md b/doc/development/testing_guide/frontend_testing.md index 98df0b5ea7c..7dc89a3fcdb 100644 --- a/doc/development/testing_guide/frontend_testing.md +++ b/doc/development/testing_guide/frontend_testing.md @@ -79,6 +79,37 @@ describe('Component', () => { Remember that the performance of each test depends on the environment. +### Manual module mocks + +Jest supports [manual module mocks](https://jestjs.io/docs/en/manual-mocks) by placing a mock in a `__mocks__/` directory next to the source module. **Don't do this.** We want to keep all of our test-related code in one place (the `spec/` folder), and the logic that Jest uses to apply mocks from `__mocks__/` is rather inconsistent. + +Instead, our test runner detects manual mocks from `spec/frontend/mocks/`. Any mock placed here is automatically picked up and injected whenever you import its source module. + +- Files in `spec/frontend/mocks/ce` will mock the corresponding CE module from `app/assets/javascripts`, mirroring the source module's path. + - Example: `spec/frontend/mocks/ce/lib/utils/axios_utils` will mock the module `~/lib/utils/axios_utils`. +- Files in `spec/frontend/mocks/node` will mock NPM packages of the same name or path. +- We don't support mocking EE modules yet. + +If a mock is found for which a source module doesn't exist, the test suite will fail. 'Virtual' mocks, or mocks that don't have a 1-to-1 association with a source module, are not supported yet. + +#### Writing a mock + +Create a JS module in the appropriate place in `spec/frontend/mocks/`. That's it. It will automatically mock its source package in all tests. + +Make sure that your mock's export has the same format as the mocked module. So, if you're mocking a CommonJS module, you'll need to use `module.exports` instead of the ES6 `export`. + +It might be useful for a mock to expose a property that indicates if the mock was loaded. This way, tests can assert the presence of a mock without calling any logic and causing side-effects. The `~/lib/utils/axios_utils` module mock has such a property, `isMock`, that is `true` in the mock and undefined in the original class. Jest's mock functions also have a `mock` property that you can test. + +#### Bypassing mocks + +If you ever need to import the original module in your tests, use [`jest.requireActual()`](https://jestjs.io/docs/en/jest-object#jestrequireactualmodulename) (or `jest.requireActual().default` for the default export). The `jest.mock()` and `jest.unmock()` won't have an effect on modules that have a manual mock, because mocks are imported and cached before any tests are run. + +#### Keep mocks light + +Global mocks introduce magic and can affect how modules are imported in your tests. Try to keep them as light as possible and dependency-free. A global mock should be useful for any unit test. For example, the `axios_utils` and `jquery` module mocks throw an error when an HTTP request is attempted, since this is useful behaviour in >99% of tests. + +When in doubt, construct mocks in your test file using [`jest.mock()`](https://jestjs.io/docs/en/jest-object#jestmockmodulename-factory-options), [`jest.spyOn()`](https://jestjs.io/docs/en/jest-object#jestspyonobject-methodname), etc. + ## Karma test suite GitLab uses the [Karma][karma] test runner with [Jasmine] as its test @@ -201,7 +232,7 @@ module. GitLab has a custom `spyOnDependency` method which utilizes [babel-plugin-rewire](https://github.com/speedskater/babel-plugin-rewire) to achieve this. It can be used like so: -```js +```javascript // my_module.js import { visitUrl } from '~/lib/utils/url_utility'; @@ -210,7 +241,7 @@ export default function doSomething() { } ``` -```js +```javascript // my_module_spec.js import doSomething from '~/my_module'; @@ -434,7 +465,7 @@ See this [section][vue-test]. For running the frontend tests, you need the following commands: -- `rake karma:fixtures` (re-)generates [fixtures](#frontend-test-fixtures). +- `rake frontend:fixtures` (re-)generates [fixtures](#frontend-test-fixtures). - `yarn test` executes the tests. As long as the fixtures don't change, `yarn test` is sufficient (and saves you some time). @@ -487,8 +518,8 @@ Information on setting up and running RSpec integration tests with Code that is added to HAML templates (in `app/views/`) or makes Ajax requests to the backend has tests that require HTML or JSON from the backend. Fixtures for these tests are located at: -- `spec/javascripts/fixtures/`, for running tests in CE. -- `ee/spec/javascripts/fixtures/`, for running tests in EE. +- `spec/frontend/fixtures/`, for running tests in CE. +- `ee/spec/frontend/fixtures/`, for running tests in EE. Fixture files in: @@ -499,29 +530,29 @@ The following are examples of tests that work for both Karma and Jest: ```javascript it('makes a request', () => { - const responseBody = getJSONFixture('some/fixture.json'); // loads spec/javascripts/fixtures/some/fixture.json + const responseBody = getJSONFixture('some/fixture.json'); // loads spec/frontend/fixtures/some/fixture.json axiosMock.onGet(endpoint).reply(200, responseBody); - + myButton.click(); - + // ... }); it('uses some HTML element', () => { - loadFixtures('some/page.html'); // loads spec/javascripts/fixtures/some/page.html and adds it to the DOM - + loadFixtures('some/page.html'); // loads spec/frontend/fixtures/some/page.html and adds it to the DOM + const element = document.getElementById('#my-id'); - + // ... }); ``` -HTML and JSON fixtures are generated from backend views and controllers using RSpec (see `spec/javascripts/fixtures/*.rb`). +HTML and JSON fixtures are generated from backend views and controllers using RSpec (see `spec/frontend/fixtures/*.rb`). For each fixture, the content of the `response` variable is stored in the output file. This variable gets automagically set if the test is marked as `type: :request` or `type: :controller`. -Fixtures are regenerated using the `bin/rake karma:fixtures` command but you can also generate them individually, -for example `bin/rspec spec/javascripts/fixtures/merge_requests.rb`. +Fixtures are regenerated using the `bin/rake frontend:fixtures` command but you can also generate them individually, +for example `bin/rspec spec/frontend/fixtures/merge_requests.rb`. When creating a new fixture, it often makes sense to take a look at the corresponding tests for the endpoint in `(ee/)spec/controllers/` or `(ee/)spec/requests/`. ## Gotchas @@ -557,11 +588,522 @@ end [jasmine-focus]: https://jasmine.github.io/2.5/focused_specs.html [karma]: http://karma-runner.github.io/ -[vue-test]: https://docs.gitlab.com/ce/development/fe_guide/vue.html#testing-vue-components +[vue-test]: ../fe_guide/vue.md#testing-vue-components [rspec]: https://github.com/rspec/rspec-rails#feature-specs [capybara]: https://github.com/teamcapybara/capybara [jasmine]: https://jasmine.github.io/ +## Overview of Frontend Testing Levels + +Tests relevant for frontend development can be found at the following places: + +- `spec/javascripts/` which are run by Karma (command: `yarn karma`) and contain + - [frontend unit tests](#frontend-unit-tests) + - [frontend component tests](#frontend-component-tests) + - [frontend integration tests](#frontend-integration-tests) +- `spec/frontend/` which are run by Jest (command: `yarn jest`) and contain + - [frontend unit tests](#frontend-unit-tests) + - [frontend component tests](#frontend-component-tests) + - [frontend integration tests](#frontend-integration-tests) +- `spec/features/` which are run by RSpec and contain + - [feature tests](#feature-tests) + +All tests in `spec/javascripts/` will eventually be migrated to `spec/frontend/` (see also [#52483](https://gitlab.com/gitlab-org/gitlab-ce/issues/52483)). + +In addition, there used to be feature tests in `features/`, run by Spinach. +These were removed from the codebase in May 2018 ([#23036](https://gitlab.com/gitlab-org/gitlab-ce/issues/23036)). + +See also [Notes on testing Vue components](../fe_guide/vue.html#testing-vue-components). + +### Frontend unit tests + +Unit tests are on the lowest abstraction level and typically test functionality that is not directly perceivable by a user. + +```mermaid +graph RL + plain[Plain JavaScript]; + Vue[Vue Components]; + feature-flags[Feature Flags]; + license-checks[License Checks]; + + plain---Vuex; + plain---GraphQL; + Vue---plain; + Vue---Vuex; + Vue---GraphQL; + browser---plain; + browser---Vue; + plain---backend; + Vuex---backend; + GraphQL---backend; + Vue---backend; + backend---database; + backend---feature-flags; + backend---license-checks; + + class plain tested; + class Vuex tested; + + classDef node color:#909090,fill:#f0f0f0,stroke-width:2px,stroke:#909090 + classDef label stroke-width:0; + classDef tested color:#000000,fill:#a0c0ff,stroke:#6666ff,stroke-width:2px,stroke-dasharray: 5, 5; + + subgraph " " + tested; + mocked; + class tested tested; + end +``` + +#### When to use unit tests + +<details> + <summary>exported functions and classes</summary> + Anything that is exported can be reused at various places in a way you have no control over. + Therefore it is necessary to document the expected behavior of the public interface with tests. +</details> + +<details> + <summary>Vuex actions</summary> + Any Vuex action needs to work in a consistent way independent of the component it is triggered from. +</details> + +<details> + <summary>Vuex mutations</summary> + For complex Vuex mutations it helps to identify the source of a problem by separating the tests from other parts of the Vuex store. +</details> + +#### When *not* to use unit tests + +<details> + <summary>non-exported functions or classes</summary> + Anything that is not exported from a module can be considered private or an implementation detail and doesn't need to be tested. +</details> + +<details> + <summary>constants</summary> + Testing the value of a constant would mean to copy it. + This results in extra effort without additional confidence that the value is correct. +</details> + +<details> + <summary>Vue components</summary> + Computed properties, methods, and lifecycle hooks can be considered an implementation detail of components and don't need to be tested. + They are implicitly covered by component tests. + The <a href="https://vue-test-utils.vuejs.org/guides/#getting-started">official Vue guidelines</a> suggest the same. +</details> + +#### What to mock in unit tests + +<details> + <summary>state of the class under test</summary> + Modifying the state of the class under test directly rather than using methods of the class avoids side-effects in test setup. +</details> + +<details> + <summary>other exported classes</summary> + Every class needs to be tested in isolation to prevent test scenarios from growing exponentially. +</details> + +<details> + <summary>single DOM elements if passed as parameters</summary> + For tests that only operate on single DOM elements rather than a whole page, creating these elements is cheaper than loading a whole HTML fixture. +</details> + +<details> + <summary>all server requests</summary> + When running frontend unit tests, the backend may not be reachable. + Therefore all outgoing requests need to be mocked. +</details> + +<details> + <summary>asynchronous background operations</summary> + Background operations cannot be stopped or waited on, so they will continue running in the following tests and cause side effects. +</details> + +#### What *not* to mock in unit tests + +<details> + <summary>non-exported functions or classes</summary> + Everything that is not exported can be considered private to the module and will be implicitly tested via the exported classes / functions. +</details> + +<details> + <summary>methods of the class under test</summary> + By mocking methods of the class under test, the mocks will be tested and not the real methods. +</details> + +<details> + <summary>utility functions (pure functions, or those that only modify parameters)</summary> + If a function has no side effects because it has no state, it is safe to not mock it in tests. +</details> + +<details> + <summary>full HTML pages</summary> + Loading the HTML of a full page slows down tests, so it should be avoided in unit tests. +</details> + +### Frontend component tests + +Component tests cover the state of a single component that is perceivable by a user depending on external signals such as user input, events fired from other components, or application state. + +```mermaid +graph RL + plain[Plain JavaScript]; + Vue[Vue Components]; + feature-flags[Feature Flags]; + license-checks[License Checks]; + + plain---Vuex; + plain---GraphQL; + Vue---plain; + Vue---Vuex; + Vue---GraphQL; + browser---plain; + browser---Vue; + plain---backend; + Vuex---backend; + GraphQL---backend; + Vue---backend; + backend---database; + backend---feature-flags; + backend---license-checks; + + class Vue tested; + + classDef node color:#909090,fill:#f0f0f0,stroke-width:2px,stroke:#909090 + classDef label stroke-width:0; + classDef tested color:#000000,fill:#a0c0ff,stroke:#6666ff,stroke-width:2px,stroke-dasharray: 5, 5; + + subgraph " " + tested; + mocked; + class tested tested; + end +``` + +#### When to use component tests + +- Vue components + +#### When *not* to use component tests + +<details> + <summary>Vue applications</summary> + Vue applications may contain many components. + Testing them on a component level requires too much effort. + Therefore they are tested on frontend integration level. +</details> + +<details> + <summary>HAML templates</summary> + HAML templates contain only Markup and no frontend-side logic. + Therefore they are not complete components. +</details> + +#### What to mock in component tests + +<details> + <summary>DOM</summary> + Operating on the real DOM is significantly slower than on the virtual DOM. +</details> + +<details> + <summary>properties and state of the component under test</summary> + Similarly to testing classes, modifying the properties directly (rather than relying on methods of the component) avoids side-effects. +</details> + +<details> + <summary>Vuex store</summary> + To avoid side effects and keep component tests simple, Vuex stores are replaced with mocks. +</details> + +<details> + <summary>all server requests</summary> + Similar to unit tests, when running component tests, the backend may not be reachable. + Therefore all outgoing requests need to be mocked. +</details> + +<details> + <summary>asynchronous background operations</summary> + Similar to unit tests, background operations cannot be stopped or waited on, so they will continue running in the following tests and cause side effects. +</details> + +<details> + <summary>child components</summary> + Every component is tested individually, so child components are mocked. + See also <a href="https://vue-test-utils.vuejs.org/api/#shallowmount">shallowMount()</a> +</details> + +#### What *not* to mock in component tests + +<details> + <summary>methods or computed properties of the component under test</summary> + By mocking part of the component under test, the mocks will be tested and not the real component. +</details> + +<details> + <summary>functions and classes independent from Vue</summary> + All plain JavaScript code is already covered by unit tests and needs not to be mocked in component tests. +</details> + +### Frontend integration tests + +Integration tests cover the interaction between all components on a single page. +Their abstraction level is comparable to how a user would interact with the UI. + +```mermaid +graph RL + plain[Plain JavaScript]; + Vue[Vue Components]; + feature-flags[Feature Flags]; + license-checks[License Checks]; + + plain---Vuex; + plain---GraphQL; + Vue---plain; + Vue---Vuex; + Vue---GraphQL; + browser---plain; + browser---Vue; + plain---backend; + Vuex---backend; + GraphQL---backend; + Vue---backend; + backend---database; + backend---feature-flags; + backend---license-checks; + + class plain tested; + class Vue tested; + class Vuex tested; + class GraphQL tested; + class browser tested; + linkStyle 0,1,2,3,4,5,6 stroke:#6666ff,stroke-width:2px,stroke-dasharray: 5, 5; + + classDef node color:#909090,fill:#f0f0f0,stroke-width:2px,stroke:#909090 + classDef label stroke-width:0; + classDef tested color:#000000,fill:#a0c0ff,stroke:#6666ff,stroke-width:2px,stroke-dasharray: 5, 5; + + subgraph " " + tested; + mocked; + class tested tested; + end +``` + +#### When to use integration tests + +<details> + <summary>page bundles (<code>index.js</code> files in <code>app/assets/javascripts/pages/</code>)</summary> + Testing the page bundles ensures the corresponding frontend components integrate well. +</details> + +<details> + <summary>Vue applications outside of page bundles</summary> + Testing Vue applications as a whole ensures the corresponding frontend components integrate well. +</details> + +#### What to mock in integration tests + +<details> + <summary>HAML views (use fixtures instead)</summary> + Rendering HAML views requires a Rails environment including a running database which we cannot rely on in frontend tests. +</details> + +<details> + <summary>all server requests</summary> + Similar to unit and component tests, when running component tests, the backend may not be reachable. + Therefore all outgoing requests need to be mocked. +</details> + +<details> + <summary>asynchronous background operations that are not perceivable on the page</summary> + Background operations that affect the page need to be tested on this level. + All other background operations cannot be stopped or waited on, so they will continue running in the following tests and cause side effects. +</details> + +#### What *not* to mock in integration tests + +<details> + <summary>DOM</summary> + Testing on the real DOM ensures our components work in the environment they are meant for. + Part of this will be delegated to <a href="https://gitlab.com/gitlab-org/quality/team-tasks/issues/45">cross-browser testing</a>. +</details> + +<details> + <summary>properties or state of components</summary> + On this level, all tests can only perform actions a user would do. + For example to change the state of a component, a click event would be fired. +</details> + +<details> + <summary>Vuex stores</summary> + When testing the frontend code of a page as a whole, the interaction between Vue components and Vuex stores is covered as well. +</details> + +### Feature tests + +In contrast to [frontend integration tests](#frontend-integration-tests), feature tests make requests against the real backend instead of using fixtures. +This also implies that database queries are executed which makes this category significantly slower. + +See also the [RSpec testing guidelines](../testing_guide/best_practices.md#rspec). + +```mermaid +graph RL + plain[Plain JavaScript]; + Vue[Vue Components]; + feature-flags[Feature Flags]; + license-checks[License Checks]; + + plain---Vuex; + plain---GraphQL; + Vue---plain; + Vue---Vuex; + Vue---GraphQL; + browser---plain; + browser---Vue; + plain---backend; + Vuex---backend; + GraphQL---backend; + Vue---backend; + backend---database; + backend---feature-flags; + backend---license-checks; + + class backend tested; + class plain tested; + class Vue tested; + class Vuex tested; + class GraphQL tested; + class browser tested; + linkStyle 0,1,2,3,4,5,6,7,8,9,10 stroke:#6666ff,stroke-width:2px,stroke-dasharray: 5, 5; + + classDef node color:#909090,fill:#f0f0f0,stroke-width:2px,stroke:#909090 + classDef label stroke-width:0; + classDef tested color:#000000,fill:#a0c0ff,stroke:#6666ff,stroke-width:2px,stroke-dasharray: 5, 5; + + subgraph " " + tested; + mocked; + class tested tested; + end +``` + +#### When to use feature tests + +- Use cases that require a backend and cannot be tested using fixtures. +- Behavior that is not part of a page bundle but defined globally. + +#### Relevant notes + +A `:js` flag is added to the test to make sure the full environment is loaded. + +```ruby +scenario 'successfully', :js do + sign_in(create(:admin)) +end +``` + +The steps of each test are written using capybara methods ([documentation](https://www.rubydoc.info/gems/capybara)). + +Bear in mind <abbr title="XMLHttpRequest">XHR</abbr> calls might require you to use `wait_for_requests` in between steps, like so: + +```ruby +find('.form-control').native.send_keys(:enter) + +wait_for_requests + +expect(page).not_to have_selector('.card') +``` + +## Test helpers + +### Vuex Helper: `testAction` + +We have a helper available to make testing actions easier, as per [official documentation](https://vuex.vuejs.org/guide/testing.html): + +```javascript +testAction( + actions.actionName, // action + { }, // params to be passed to action + state, // state + [ + { type: types.MUTATION}, + { type: types.MUTATION_1, payload: {}}, + ], // mutations committed + [ + { type: 'actionName', payload: {}}, + { type: 'actionName1', payload: {}}, + ] // actions dispatched + done, +); +``` + +Check an example in [spec/javascripts/ide/stores/actions_spec.jsspec/javascripts/ide/stores/actions_spec.js](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/spec/javascripts/ide/stores/actions_spec.js). + +### Vue Helper: `mountComponent` + +To make mounting a Vue component easier and more readable, we have a few helpers available in `spec/helpers/vue_mount_component_helper`: + +- `createComponentWithStore` +- `mountComponentWithStore` + +Examples of usage: + +```javascript +beforeEach(() => { + vm = createComponentWithStore(Component, store); + + vm.$store.state.currentBranchId = 'master'; + + vm.$mount(); +}); +``` + +```javascript +beforeEach(() => { + vm = mountComponentWithStore(Component, { + el: '#dummy-element', + store, + props: { badge }, + }); +}); +``` + +Don't forget to clean up: + +```javascript +afterEach(() => { + vm.$destroy(); +}); +``` + +## Testing with older browsers + +Some regressions only affect a specific browser version. We can install and test in particular browsers with either Firefox or Browserstack using the following steps: + +### Browserstack + +[Browserstack](https://www.browserstack.com/) allows you to test more than 1200 mobile devices and browsers. +You can use it directly through the [live app](https://www.browserstack.com/live) or you can install the [chrome extension](https://chrome.google.com/webstore/detail/browserstack/nkihdmlheodkdfojglpcjjmioefjahjb) for easy access. +You can find the credentials on 1Password, under `frontendteam@gitlab.com`. + +### Firefox + +#### macOS + +You can download any older version of Firefox from the releases FTP server, <https://ftp.mozilla.org/pub/firefox/releases/>: + +1. From the website, select a version, in this case `50.0.1`. +1. Go to the mac folder. +1. Select your preferred language, you will find the dmg package inside, download it. +1. Drag and drop the application to any other folder but the `Applications` folder. +1. Rename the application to something like `Firefox_Old`. +1. Move the application to the `Applications` folder. +1. Open up a terminal and run `/Applications/Firefox_Old.app/Contents/MacOS/firefox-bin -profilemanager` to create a new profile specific to that Firefox version. +1. Once the profile has been created, quit the app, and run it again like normal. You now have a working older Firefox version. + --- [Return to Testing documentation](index.md) diff --git a/doc/development/testing_guide/img/qa_on_merge_requests_cicd_architecture.png b/doc/development/testing_guide/img/qa_on_merge_requests_cicd_architecture.png Binary files differdeleted file mode 100644 index 5b93a05db96..00000000000 --- a/doc/development/testing_guide/img/qa_on_merge_requests_cicd_architecture.png +++ /dev/null diff --git a/doc/development/testing_guide/img/review_apps_cicd_architecture.png b/doc/development/testing_guide/img/review_apps_cicd_architecture.png Binary files differdeleted file mode 100644 index 1ee28d3db91..00000000000 --- a/doc/development/testing_guide/img/review_apps_cicd_architecture.png +++ /dev/null diff --git a/doc/development/testing_guide/index.md b/doc/development/testing_guide/index.md index aadbea1a540..96e8c30a679 100644 --- a/doc/development/testing_guide/index.md +++ b/doc/development/testing_guide/index.md @@ -22,62 +22,44 @@ automated testing means, and what are its principles: - [Five Factor Testing](https://www.devmynd.com/blog/five-factor-testing): Why do we need tests? - [Principles of Automated Testing](http://www.lihaoyi.com/post/PrinciplesofAutomatedTesting.html): Levels of testing. Prioritize tests. Cost of tests. ---- - ## [Testing levels](testing_levels.md) Learn about the different testing levels, and how to decide at what level your changes should be tested. ---- - ## [Testing best practices](best_practices.md) Everything you should know about how to write good tests: Test Design, RSpec, FactoryBot, system tests, parameterized tests etc. ---- - ## [Frontend testing standards and style guidelines](frontend_testing.md) Everything you should know about how to write good Frontend tests: Karma, testing promises, stubbing etc. ---- - ## [Flaky tests](flaky_tests.md) What are flaky tests, the different kind of flaky tests we encountered, and what we do about them. ---- - ## [GitLab tests in the Continuous Integration (CI) context](ci.md) How GitLab test suite is run in the CI context: setup, caches, artifacts, parallelization, monitoring. ---- - ## [Review apps](review_apps.md) How review apps are set up for GitLab CE/EE and how to use them. ---- - ## [Testing Rake tasks](testing_rake_tasks.md) Everything you should know about how to test Rake tasks. ---- - ## [End-to-end tests](end_to_end/index.md) Everything you should know about how to run end-to-end tests using [GitLab QA][gitlab-qa] testing framework. ---- - [Return to Development documentation](../README.md) [RSpec]: https://github.com/rspec/rspec-rails#feature-specs diff --git a/doc/development/testing_guide/review_apps.md b/doc/development/testing_guide/review_apps.md index ae40d628717..11449712a04 100644 --- a/doc/development/testing_guide/review_apps.md +++ b/doc/development/testing_guide/review_apps.md @@ -8,38 +8,33 @@ Review Apps are automatically deployed by each pipeline, both in ### CI/CD architecture diagram - - -<details> -<summary>Show mermaid source</summary> -<pre> +```mermaid graph TD build-qa-image -.->|once the `prepare` stage is done| gitlab:assets:compile review-build-cng -->|triggers a CNG-mirror pipeline and wait for it to be done| CNG-mirror review-build-cng -.->|once the `test` stage is done| review-deploy review-deploy -.->|once the `review` stage is done| review-qa-smoke -subgraph 1. gitlab-ce/ee `prepare` stage +subgraph "1. gitlab-ce/ee `prepare` stage" build-qa-image end -subgraph 2. gitlab-ce/ee `test` stage +subgraph "2. gitlab-ce/ee `test` stage" gitlab:assets:compile -->|plays dependent job once done| review-build-cng end -subgraph 3. gitlab-ce/ee `review` stage - review-deploy["review-deploy<br /><br />Helm deploys the Review App using the Cloud<br/>Native images built by the CNG-mirror pipeline.<br /><br />Cloud Native images are deployed to the `review-apps-ce` or `review-apps-ee`<br />Kubernetes (GKE) cluster, in the GCP `gitlab-review-apps` project."] +subgraph "3. gitlab-ce/ee `review` stage" + review-deploy["review-deploy<br><br>Helm deploys the Review App using the Cloud<br/>Native images built by the CNG-mirror pipeline.<br><br>Cloud Native images are deployed to the `review-apps-ce` or `review-apps-ee`<br>Kubernetes (GKE) cluster, in the GCP `gitlab-review-apps` project."] end -subgraph 4. gitlab-ce/ee `qa` stage - review-qa-smoke[review-qa-smoke<br /><br />gitlab-qa runs the smoke suite against the Review App.] +subgraph "4. gitlab-ce/ee `qa` stage" + review-qa-smoke[review-qa-smoke<br><br>gitlab-qa runs the smoke suite against the Review App.] end -subgraph CNG-mirror pipeline +subgraph "CNG-mirror pipeline" CNG-mirror>Cloud Native images are built]; end -</pre> -</details> +``` ### Detailed explanation @@ -115,6 +110,28 @@ On every [pipeline][gitlab-pipeline] in the `qa` stage, the browser performance testing using a [Sitespeed.io Container](../../user/project/merge_requests/browser_performance_testing.md). +## Cluster configuration + +### Node pools + +Both `review-apps-ce` and `review-apps-ee` clusters are currently set up with +two node pools: + +- a node pool of non-preemptible `n1-standard-2` (2 vCPU, 7.5 GB memory) nodes + dedicated to the `tiller` deployment (see below) with a single node. +- a node pool of preemptible `n1-standard-2` (2 vCPU, 7.5 GB memory) nodes, + with a minimum of 1 node and a maximum of 250 nodes. + +### Helm/Tiller + +The `tiller` deployment (the Helm server) is deployed to a dedicated node pool +that has the `app=helm` label and a specific +[taint](https://kubernetes.io/docs/concepts/configuration/taint-and-toleration/) +to prevent other pods from being scheduled on this node pool. + +This is to ensure Tiller isn't affected by "noisy" neighbors that could put +their node under pressure. + ## How to: ### Log into my Review App @@ -137,8 +154,8 @@ secure note named **gitlab-{ce,ee} Review App's root password**. ### Run a Rails console -1. [Filter Workloads by your Review App slug](https://console.cloud.google.com/kubernetes/workload?project=gitlab-review-apps) - , e.g. `review-qa-raise-e-12chm0`. +1. [Filter Workloads by your Review App slug](https://console.cloud.google.com/kubernetes/workload?project=gitlab-review-apps), + e.g. `review-qa-raise-e-12chm0`. 1. Find and open the `task-runner` Deployment, e.g. `review-qa-raise-e-12chm0-task-runner`. 1. Click on the Pod in the "Managed pods" section, e.g. `review-qa-raise-e-12chm0-task-runner-d5455cc8-2lsvz`. 1. Click on the `KUBECTL` dropdown, then `Exec` -> `task-runner`. @@ -196,7 +213,7 @@ For the record, the debugging steps to find out this issue were: 1. `kubectl describe pod <pod name>` & confirm exact error message 1. Web search for exact error message, following rabbit hole to [a relevant kubernetes bug report](https://github.com/kubernetes/kubernetes/issues/57345) 1. Access the node over SSH via the GCP console (**Computer Engine > VM - instances** then click the "SSH" button for the node where the `dns-gitlab-review-app-external-dns` pod runs) + instances** then click the "SSH" button for the node where the `dns-gitlab-review-app-external-dns` pod runs) 1. In the node: `systemctl --version` => systemd 232 1. Gather some more information: - `mount | grep kube | wc -l` => e.g. 290 @@ -211,7 +228,7 @@ For the record, the debugging steps to find out this issue were: To resolve the problem, we needed to (forcibly) drain some nodes: 1. Try a normal drain on the node where the `dns-gitlab-review-app-external-dns` - pod runs so that Kubernetes automatically move it to another node: `kubectl drain NODE_NAME` + pod runs so that Kubernetes automatically move it to another node: `kubectl drain NODE_NAME` 1. If that doesn't work, you can also perform a forcible "drain" the node by removing all pods: `kubectl delete pods --field-selector=spec.nodeName=NODE_NAME` 1. In the node: - Perform `systemctl daemon-reload` to remove the dead/inactive units @@ -238,17 +255,8 @@ that a machine will hit the "too many mount points" problem in the future. thousands of unused Docker images.** > We have to start somewhere and improve later. Also, we're using the - CNG-mirror project to store these Docker images so that we can just wipe out - the registry at some point, and use a new fresh, empty one. - -**How big are the Kubernetes clusters (`review-apps-ce` and `review-apps-ee`)?** - - > The clusters are currently set up with a single pool of preemptible nodes, - with a minimum of 1 node and a maximum of 500 nodes. - -**What are the machine running on the cluster?** - - > We're currently using `n1-standard-1` (1 vCPU, 3.75 GB memory) machines. + > CNG-mirror project to store these Docker images so that we can just wipe out + > the registry at some point, and use a new fresh, empty one. **How do we secure this from abuse? Apps are open to the world so we need to find a way to limit it to only us.** diff --git a/doc/development/testing_guide/testing_levels.md b/doc/development/testing_guide/testing_levels.md index e1ce4d3b7d1..0090c84cbf0 100644 --- a/doc/development/testing_guide/testing_levels.md +++ b/doc/development/testing_guide/testing_levels.md @@ -63,10 +63,9 @@ They're useful to test permissions, redirections, what view is rendered etc. | Code path | Tests path | Testing engine | Notes | | --------- | ---------- | -------------- | ----- | -| `app/controllers/` | `spec/controllers/` | RSpec | | +| `app/controllers/` | `spec/controllers/` | RSpec | For N+1 tests, use [request specs](../query_recorder.md#use-request-specs-instead-of-controller-specs) | | `app/mailers/` | `spec/mailers/` | RSpec | | | `lib/api/` | `spec/requests/api/` | RSpec | | -| `lib/ci/api/` | `spec/requests/ci/api/` | RSpec | | | `app/assets/javascripts/` | `spec/javascripts/`, `spec/frontend/` | Karma & Jest | More details in the [Frontend Testing guide](frontend_testing.md) section. | ### About controller tests |
