Apply some other changes based on doc/code reivewdocs/third-iteration-on-writing-end-to-end-tests-doc

1 files changed, 100 insertions, 82 deletions

diff --git a/qa/docs/writing_tests_from_scratch.md b/qa/docs/writing_tests_from_scratch.md
index 0338eba77dc..6dcf3da84a1 100644
--- a/qa/docs/writing_tests_from_scratch.md
+++ b/qa/docs/writing_tests_from_scratch.md
@@ -87,7 +87,7 @@ end
 
 For the [MVC](https://about.gitlab.com/handbook/values/#minimum-viable-change-mvc) of our test cases, let's say that we already have the application in the state needed for the tests, and then let's focus on the logic of the test cases only.
 
-To evolve the test cases drafted on step 2, let's imagine that the user is already logged into a GitLab EE instance, they already have at least a Premium license in use, there is already a project created, there is already an issue opened in the project, the issue already has a scoped label (e.g. `foo::bar`), there are other scoped labels (for the same scope and for a different scope (e.g. `foo::baz` and `bar::bah`), and finally, the user is already on the issue's page. Let's also suppose that for every test case the application is in a clean state, meaning that one test case won't affect another.
+To evolve the test cases drafted on step 2, let's imagine that the user is already logged into a GitLab EE instance, they already have at least a Premium license in use, there is already a project created, there is already an issue opened in the project, the issue already has a scoped label (e.g. `animal::fox`), there are other scoped labels (for the same scope and for a different scope (e.g. `animal::dolphin` and `plant::orchid`), and finally, the user is already on the issue's page. Let's also suppose that for every test case the application is in a clean state, meaning that one test case won't affect another.
 
 > Note: there are different approaches to creating an application state for end-to-end tests. Some of them are very time consuming and subject to failures, such as when using the GUI for all the pre-conditions of the tests. On the other hand, other approaches are more efficient, such as using the public APIs. The latter is more efficient since it doesn't depend on the GUI. We won't focus on this part yet, but it's good to keep it in mind.
 
@@ -97,15 +97,15 @@ Let's now focus on the first test case.
 it 'replaces an existing label if it has the same key' do
   # This implementation is only for tutorial purposes. We normally encapsulate elements in Page Objects (which we cover on section 8).
   page.find('.block.labels .edit-link').click
-  page.find('.dropdown-menu-labels .dropdown-input-field').send_keys ['foo::baz', :enter]
+  page.find('.dropdown-menu-labels .dropdown-input-field').send_keys ['animal::dolphin', :enter]
   page.find('#content-body').click
   page.refresh
 
   labels_block = page.find('.qa-labels-block')
 
-  expect(labels_block).to have_content('foo::baz')
-  expect(labels_block).not_to have_content('foo::bar')
-  expect(page).to have_content('added foo::baz label and removed foo::bar')
+  expect(labels_block).to have_content('animal::dolphin')
+  expect(labels_block).not_to have_content('animal::fox')
+  expect(page).to have_content('added animal::dolphin label and removed animal::fox')
 end
 ```
 
@@ -116,7 +116,7 @@ end
 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 'foo::baz' and press enters.
+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.
 
@@ -126,25 +126,25 @@ Let's now see what the second test case would look like.
 it 'keeps both scoped labels when adding a label with a different key' do
   # This implementation is only for tutorial purposes. We normally encapsulate elements in Page Objects (which we cover on section 8).
   page.find('.block.labels .edit-link').click
-  page.find('.dropdown-menu-labels .dropdown-input-field').send_keys ['bar::bah', :enter]
+  page.find('.dropdown-menu-labels .dropdown-input-field').send_keys ['plant::orchid', :enter]
   page.find('#content-body').click
   page.refresh
 
   labels_block = page.find('.qa-labels-block')
 
-  expect(labels_block).to have_content('bar::bah')
-  expect(labels_block).to have_content('foo::bar')
-  expect(page).to have_content('added bar::bah')
-  expect(page).to have_content('added foo::bar')
+  expect(labels_block).to have_content('animal::fox')
+  expect(labels_block).to have_content('plant::orchid')
+  expect(page).to have_content('added animal::fox')
+  expect(page).to have_content('added plant::orchid')
 end
 ```
 
-> Note that elements are always located using CSS selectors, and a good practice is to add test specific attribute:value for elements (this is called adding testability to the application and we will talk more about it later.) In this case, a good selector is the one used for the `labels_block`, which is `.qa-labels-block`. This is a good one since it uses a class specifically used for testing purposes. An alternative approach could be having a test specific attribute, like `data-test="some-value"`.
+> 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.
 
 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 'bar::bah' and press enters.
+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.
 
@@ -158,35 +158,33 @@ If we refactor the tests created on step 3 we could come up with something like
 before do
   ...
 
-  @foo_bar_scoped_label = 'foo::bar'
-
-  ...
-
-  @labels = ['foo::baz', 'bar::bah']
+  @initial_label = 'animal::fox'
+  @new_label_same_scope = 'animal::dolphin'
+  @new_label_different_scope = 'plant::orchid'
 
   ...
 end
 
 it 'replaces an existing label if it has the same key' do
-  select_label_and_refresh @labels[0]
+  select_label_and_refresh @new_label_same_scope
 
   labels_block = page.find('.qa-labels-block')
 
-  expect(labels_block).to have_content(@labels[0])
-  expect(labels_block).not_to have_content(@foo_bar_scoped_label)
-  expect(page).to have_content("added #{@labels[0]}")
-  expect(page).to have_content("and removed #{@foo_bar_scoped_label}")
+  expect(labels_block).to have_content(@new_label_same_scope)
+  expect(labels_block).not_to have_content(@initial_label)
+  expect(page).to have_content("added #{@new_label_same_scope}")
+  expect(page).to have_content("and removed #{@initial_label}")
 end
 
 it 'keeps both scoped label when adding a label with a different key' do
-  select_label_and_refresh @labels[1]
+  select_label_and_refresh @new_label_different_scope
 
   labels_block = page.find('.qa-labels-block')
 
-  expect(labels_blocks).to have_content(@labels[1])
-  expect(labels_blocks).to have_content(@foo_bar_scoped_label)
-  expect(page).to have_content("added #{@labels[1]}")
-  expect(page).to have_content("added #{@foo_bar_scoped_label}")
+  expect(labels_blocks).to have_content(@new_label_different_scope)
+  expect(labels_blocks).to have_content(@initial_label)
+  expect(page).to have_content("added #{@new_label_different_scope}")
+  expect(page).to have_content("added #{@initial_label}")
 end
 
 def select_label_and_refresh(label)
@@ -197,7 +195,7 @@ def select_label_and_refresh(label)
 end
 ```
 
-First, we remove the duplication of strings by defining the global variables `@foo_bar_scoped_label` and `@labels` in the `before` block, and by using them in the expectations.
+First, we remove the duplication of strings by defining the global variables `@initial_label`, `@new_label_same_scope` and `@new_label_different_scope` in the `before` block, and by using them in the expectations.
 
 Then, by creating a reusable `select_label_and_refresh` method, we remove the code duplication of this action, and later we can move this method to a Page Object class that will be created for easier maintenance purposes.
 
@@ -211,7 +209,7 @@ In this section, we will address the previously mentioned subject of creating th
 
 A pre-condition for the entire test suite is defined in the `before :context` block.
 
-> 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 `'foo::bar'` label being set.
+> 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.
 
@@ -235,28 +233,26 @@ module QA
   context 'Plan' do
     describe 'Editing scoped labels on issues' do
       before do
-        project = Resource::Project.fabricate_via_api! do |resource|
-          resource.name = 'scoped-labels-project'
-        end
+        Runtime::Browser.visit(:gitlab, Page::Main::Login)
+        Page::Main::Login.perform(&:sign_in_using_credentials)
 
-        @foo_bar_scoped_label = 'foo::bar'
-        @issue = Resource::Issue.fabricate_via_api! do |issue|
-          issue.project = project
+        @initial_label = 'animal::fox'
+        @new_label_same_scope = 'animal::dolphin'
+        @new_label_different_scope = 'plant::orchid'
+
+        issue = Resource::Issue.fabricate_via_api! do |issue|
           issue.title = 'Issue to test the scoped labels'
-          issue.labels = @foo_bar_scoped_label
+          issue.labels = @initial_label
         end
 
-        @labels = ['foo::baz', 'bar::bah']
-        @labels.each do |label|
+        [@new_label_same_scope, @new_label_different_scope].each do |label|
           Resource::Label.fabricate_via_api! do |l|
-            l.project = project.id
+            l.project = issue.project.id
             l.title = label
           end
         end
 
-        Runtime::Browser.visit(:gitlab, Page::Main::Login)
-        Page::Main::Login.perform(&:sign_in_using_credentials)
-        @issue.visit!
+        issue.visit!
       end
 
       it 'replaces an existing label if it has the same key' do
@@ -275,9 +271,11 @@ module QA
 end
 ```
 
-In the `before` block we create all the application state needed for the tests to run. We do that by fabricating resources via APIs (`project`, `@issue`, and `@labels`), 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, and by using the `Page::Project::Issue::Show.perform { @issue.visit! }` to visit the issue page.
+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](./qa/qa/resource/README.md), notice that when calling the `fabricate_via_api` method, we pass some attribute:values, like `name` for the `project` resource; `project`, `title`, and `labels` for the `issue` resource; and `project` and `title` for the `label` resource.
+> When [creating the resources](./qa/qa/resource/README.md), 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.
 
@@ -305,15 +303,15 @@ module QA
         end
 
         it 'correctly applies scoped labels depending on if they are from the same or a different scope' do
-          select_labels_and_refresh @labels
+          select_labels_and_refresh [@new_label_same_scope, @new_label_different_scope]
 
           labels_block = page.all('.qa-labels-block')
 
-          expect(labels_block).to have_content(@labels[0])
-          expect(labels_block).to have_content(@labels[1])
-          expect(labels_block).not_to have_content(@foo_bar_scoped_label)
-          expect(page).to have_content("added #{@foo_bar_scoped_label}")
-          expect(page).to have_content("added #{@labels[1]} #{@labels[0]} labels and removed #{@foo_bar_scoped_label}")
+          expect(labels_block).to have_content(@new_label_same_scope)
+          expect(labels_block).to have_content(@new_label_different_scope)
+          expect(labels_block).not_to have_content(@initial_label)
+          expect(page).to have_content("added #{@initial_label}")
+          expect(page).to have_content("added #{@new_label_same_scope} #{@new_label_different_scope} labels and removed #{@initial_label}")
         end
 
         def select_labels_and_refresh(labels)
@@ -339,6 +337,8 @@ To address point 1, we changed the test implementation from two `it` blocks into
 
 ### 7. Resources
 
+**Note:** When writing this document, some code that is now merged to master was not implemented yet, but we left them here for the readers to understand the whole process of end-to-end test creation.
+
 You can think of [resources](qa/qa/resource/README.md) as anything that can be created on GitLab CE or EE, either through the GUI, the API, or the CLI.
 
 With that in mind, resources can be a project, an epic, an issue, a label, a commit, etc.
@@ -347,22 +347,12 @@ As you saw in the tests' pre-conditions and the optimization sections, we're alr
 
 > We could be using the `fabricate!` method instead, which would use the `fabricate_via_api!` method if it exists, and fallback to GUI fabrication otherwise, but we recommend being explicit to make it clear what the test does. Also, we always recommend fabricating resources via API since this makes tests faster and more reliable.
 
-For our test suite example, the [project resource](https://gitlab.com/gitlab-org/gitlab-ee/blob/d3584e80b4236acdf393d815d604801573af72cc/qa/qa/resource/project.rb#L55) already had a `fabricate_via_api!` method available, while other resources don't have it (e.g., the issue and label resources), so we will have to create them. Also, we will have to make a small change in the project resource to expose its `id` attribute so that we can refer to it when fabricating issues and labels.
+For our test suite example, the resource that we need to create don't have the necessary code for the `fabricate_via_api!` method to correctly work (e.g., the issue and label resources), so we will have to create them.
 
 #### Implementation
 
 In the following we describe the changes needed in each of the resource files mentioned above.
 
-**Project resource**
-
-Let's start with the smallest change.
-
-In the [project resource](https://gitlab.com/gitlab-org/gitlab-ee/blob/master/qa/qa/resource/project.rb), let's expose its `id` attribute.
-
-Add the following `attribute :id` right below the [`attribute :description`](https://gitlab.com/gitlab-org/gitlab-ee/blob/d3584e80b4236acdf393d815d604801573af72cc/qa/qa/resource/project.rb#L11).
-
-> This line is needed to allow for issues and labels to be automatically added to a project when fabricating them via API.
-
 **Issue resource**
 
 Now, let's make it possible to create an issue resource through the API.
@@ -461,6 +451,8 @@ To address this issue, we will move the implementation to Page Objects, and the
 
 As in a test-driven development approach, let's start changing the test file even before the Page Object implementation is in place.
 
+Replace the code in the test file by the following:
+
 ```ruby
 module QA
   context 'Plan' do
@@ -471,13 +463,13 @@ module QA
 
       it 'correctly applies scoped labels depending on if they are from the same or a different scope' do
         Page::Project::Issue::Show.perform do |issue_page|
-          issue_page.select_labels_and_refresh @labels
+          issue_page.select_labels_and_refresh [@new_label_same_scope, @new_label_different_scope]
 
-          expect(page).to have_content("added #{@foo_bar_scoped_label}")
-          expect(page).to have_content("added #{@labels[1]} #{@labels[0]} labels and removed #{@foo_bar_scoped_label}")
-          expect(issue_page.text_of_labels_block).to have_content(@labels[0])
-          expect(issue_page.text_of_labels_block).to have_content(@labels[1])
-          expect(issue_page.text_of_labels_block).not_to have_content(@foo_bar_scoped_label)
+          expect(page).to have_content("added #{@initial_label}")
+          expect(page).to have_content("added #{@new_label_same_scope} #{@new_label_different_scope} labels and removed #{@initial_label}")
+          expect(issue_page.text_of_labels_block).to have_content(@new_label_same_scope)
+          expect(issue_page.text_of_labels_block).to have_content(@new_label_different_scope)
+          expect(issue_page.text_of_labels_block).not_to have_content(@initial_label)
         end
       end
     end
@@ -487,7 +479,7 @@ end
 
 Notice that `select_labels_and_refresh` is now a method from the issue Page Object (which is not yet implemented), and that we verify the labels' text by using `text_of_labels_block`, instead of via the `labels_block` element. The `text_of_labels_block` method will also be implemented in the issue Page Object.
 
-Let's now update the issue Page Object.
+Let's now update the Issue Page Object.
 
 #### Updates in the Issue Page Object
 
@@ -503,11 +495,15 @@ view 'app/views/shared/issuable/_sidebar.html.haml' do
   element :edit_link_labels
   element :dropdown_menu_labels
 end
+
+view 'app/helpers/dropdowns_helper.rb' do
+  element :dropdown_input_field
+end
 ```
 
-Similarly to what we did before, let's first change the Page Object even without the elements being defined in the view file (`_sidebar.html.haml`), and later we will update it by adding the CSS selectors.
+Similarly to what we did before, let's first change the Page Object even without the elements being defined in the view (`_sidebar.html.haml`) and the `dropdowns_helper.rb` files, and later we will update them by adding the appropriate CSS selectors.
 
-Let's implement the methods `select_labels_and_refresh` and `text_of_labels_block`.
+Now, let's implement the methods `select_labels_and_refresh` and `text_of_labels_block`.
 
 Somewhere between the definition of the views and the private methods, add the following snippet of code:
 
@@ -515,12 +511,11 @@ Somewhere between the definition of the views and the private methods, add the f
 def select_labels_and_refresh(labels)
   click_element(:edit_link_labels)
   labels.each do |label|
-    wait(reload: false) do
-      has_element?(:dropdown_menu_labels, text: label)
-      find('.dropdown-menu-labels .dropdown-input-field').send_keys [label, :enter]
+    within_element(:dropdown_menu_labels, text: label) do
+      send_keys_to_element(:dropdown_input_field, [label, :enter])
     end
   end
-  find('#content-body').click
+  click_body
   labels.each do |label|
     has_element?(:labels_block, text: label)
   end
@@ -535,17 +530,18 @@ end
 ##### Details of `select_labels_and_refresh`
 
 Notice that we have not only moved the `select_labels_and_refresh` method, but we have also changed its implementation to:
-1. Click in the `:edit_link_labels` element previously defined, instead of using `find('.block.labels .edit-link').click`
-2. Use the `wait` block, which inside of it, before interacting with the input field to type the label and press enter, we call the `has_element?` method, passing the previously defined `:dropdown_menu_labels` element, and the `text` attribute with the `label` from the current iteration as the value. This way we avoid test flakiness since we ensure that we will only interact with the element when it's ready to be interacted with.
-3. Use the `has_element?(:labels_block, text: label)` after clicking in the content body (which applies the labels), and before refreshing the page, to avoid test flakiness due to refreshing too fast.
+1. Click in 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 the `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 the `has_element?(:labels_block, text: label)` after clicking in 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`
 
 The `text_of_labels_block` method is a simple method that returns the `:labels_block` element (`find_element(:labels_block)`).
 
-#### Updates in the view file
+#### Updates in the view (*.html.haml) and `dropdowns_helper.rb` files
 
-The last thing that we have to do is to update a view file to add the selectors that relate with the Page Object.
+Now let's change the view and the `dropdowns_helper` files to add the selectors that relate with the Page Object.
 
 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`.
 
@@ -553,12 +549,34 @@ The code should look like this: `= link_to _('Edit'), '#', class: 'js-sidebar-dr
 
 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`.
 
-The code should look like this: `.dropdown-menu.dropdown-select.dropdown-menu-paging.dropdown-menu-labels.qa-dropdown-menu-labels.dropdown-menu-selectable`
+The code should look like this: `.dropdown-menu.dropdown-select.dropdown-menu-paging.dropdown-menu-labels.qa-dropdown-menu-labels.dropdown-menu-selectable`.
+
+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`.
+
+The code should look like this: `filter_output = search_field_tag search_id, nil, class: "dropdown-input-field qa-dropdown-input-field", placeholder: placeholder, autocomplete: 'off'`.
 
 > 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` in a view, it is transformed into `:labels_block` for usage in the Page Objects. So, `qa-edit-link-labels` is tranformed into `:edit_link_labels`, and `qa-dropdown-menu-labels` is transformed into `:dropdown_menu_labels`. 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.
+> 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 tranformed 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.
 
+#### Updates in the `QA::Page::Base` class
+
+The last thing that we have to do is to update `QA::Page::Base` class to add the `send_keys_to_element` method on it.
+
+Add the following snippet of code somewhere where class methods are defined:
+
+```ruby
+def send_keys_to_element(name, keys)
+  find_element(name).send_keys(keys)
+end
+```
+
+This method receives an element (`name`) and the `keys` that it will send to that element, and the keys are an array that can receive strings, or "special" keys, like `:enter`.
+
+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!*