End-to-end testing Best Practices

This is a tailored extension of the Best Practices found in the testing guide.

Link a test to its test-case issue

Every test should have a corresponding issue in the Quality Test Cases project. It's recommended that you reuse the issue created to plan the test. If one does not already exist you can create the issue yourself. Alternatively, you can run the test in a pipeline that has reporting enabled and the test-case issue reporter will automatically create a new issue.

Whether you create a new test-case issue or one is created automatically, you will need to manually add a testcase RSpec metadata tag. In most cases, a single test will be associated with a single test-case issue (see below for exceptions).

For example:

RSpec.describe 'Stage' do
  describe 'General description of the feature under test' do
    it 'test name', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/:issue_id' do
      ...
    end

    it 'another test', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/:another_issue_id' do
      ...
    end
  end
end

Exceptions

Most tests are defined by a single line of a spec file, which is why those tests can be linked to a single test-case issue via the testcase tag.

However, some tests don't have a one-to-one relationship between a line of a spec file and a test-case issue. This is because some tests are defined in a way that means a single line is associated with multiple tests, including:

  • Parallelized tests.
  • Templated tests.
  • Tests in shared examples that include more than one example.

In those and similar cases we can't assign a single testcase tag and so we rely on the test-case reporter to programmatically determine the correct test-case issue based on the name and description of the test. In such cases, the test-case reporter will automatically create a test-case issue the first time the test runs, if no issue exists already.

In such a case, if you create the issue yourself or want to reuse an existing issue, you must use this end-to-end test issue template to format the issue description.

To illustrate, there are two tests in the shared examples in qa/specs/features/ee/browser_ui/3_create/repository/restrict_push_protected_branch_spec.rb:

shared_examples 'only user with access pushes and merges' do
  it 'unselected maintainer user fails to push' do
    ...
  end

  it 'selected developer user pushes and merges' do
    ...
  end
end

Consider the following test that includes the shared examples:

RSpec.describe 'Create' do
  describe 'Restricted protected branch push and merge' do
    context 'when only one user is allowed to merge and push to a protected branch' do
      ...
      it_behaves_like 'only user with access pushes and merges'
    end
  end
end

There would be two associated test-case issues, one for each shared example, with the following content:

Test 1:

```markdown
Title: browser_ui/3_create/repository/restrict_push_protected_branch_spec.rb | Create Restricted
protected branch push and merge when only one user is allowed to merge and push to a protected
branch behaves like only user with access pushes and merges selecte...

Description:
### Full description

Create Restricted protected branch push and merge when only one user is allowed to merge and push
to a protected branch behaves like only user with access pushes and merges selected developer user
pushes and merges

### File path

./qa/specs/features/ee/browser_ui/3_create/repository/restrict_push_protected_branch_spec.rb

```

Test 2:

```markdown
Title: browser_ui/3_create/repository/restrict_push_protected_branch_spec.rb | Create Restricted
protected branch push and merge when only one user is allowed to merge and push to a protected
branch behaves like only user with access pushes and merges unselec...

Description:
### Full description

Create Restricted protected branch push and merge when only one user is allowed to merge and push
to a protected branch behaves like only user with access pushes and merges unselected maintainer
user fails to push

### File path

./qa/specs/features/ee/browser_ui/3_create/repository/restrict_push_protected_branch_spec.rb
```

Prefer API over UI

The end-to-end testing framework has the ability to fabricate its resources on a case-by-case basis. Resources should be fabricated via the API wherever possible.

We can save both time and money by fabricating resources that our test will need via the API.

Learn more about resources.

Avoid superfluous expectations

To keep tests lean, it is important that we only test what we need to test.

Ensure that you do not add any expect() statements that are unrelated to what needs to be tested.

For example:

#=> Good
Flow::Login.sign_in
Page::Main::Menu.perform do |menu|
  expect(menu).to be_signed_in
end

#=> Bad
Flow::Login.sign_in(as: user)
Page::Main::Menu.perform do |menu|
  expect(menu).to be_signed_in
  expect(page).to have_content(user.name) #=>  we already validated being signed in. redundant.
  expect(menu).to have_element(:nav_bar) #=> likely unnecessary. already validated in lower-level. test doesn't call for validating this.
end

#=> Good
issue = Resource::Issue.fabricate_via_api! do |issue|
  issue.name = 'issue-name'
end

Project::Issues::Index.perform do |index|
  expect(index).to have_issue(issue)
end

#=> Bad
issue = Resource::Issue.fabricate_via_api! do |issue|
  issue.name = 'issue-name'
end

Project::Issues::Index.perform do |index|
  expect(index).to have_issue(issue)
  expect(page).to have_content(issue.name) #=> page content check is redundant as the issue was already validated in the line above.
end

Prefer aggregate_failures when there are back-to-back expectations

In cases where there must be multiple (back-to-back) expectations within a test case, it is preferable to use aggregate_failures.

This allows you to group a set of expectations and see all the failures altogether, rather than having the test being aborted on the first failure.

For example:

#=> Good
Page::Search::Results.perform do |search|
  search.switch_to_code

  aggregate_failures 'testing search results' do
    expect(search).to have_file_in_project(template[:file_name], project.name)
    expect(search).to have_file_with_content(template[:file_name], content[0..33])
  end
end

#=> Bad
Page::Search::Results.perform do |search|
  search.switch_to_code
  expect(search).to have_file_in_project(template[:file_name], project.name)
  expect(search).to have_file_with_content(template[:file_name], content[0..33])
end

Prefer to split tests across multiple files

Our framework includes a couple of parallelization mechanisms that work by executing spec files in parallel.

However, because tests are parallelized by spec file and not by test/example, we can't achieve greater parallelization if a new test is added to an existing file.

Nonetheless, there could be other reasons to add a new test to an existing file.

For example, if tests share state that is expensive to set up it might be more efficient to perform that setup once even if it means the tests that use the setup can't be parallelized.

In summary:

  • Do: Split tests across separate files, unless the tests share expensive setup.
  • Don't: Put new tests in an existing file without considering the impact on parallelization.

let variables vs instance variables

By default, follow the testing best practices when using let or instance variables. However, in end-to-end tests, set-ups such as creating resources are expensive. If you use let to store a resource, it will be created for each example separately. If the resource can be shared among multiple examples, use an instance variable in the before(:all) block instead of let to save run time. When the variable cannot be shared by multiple examples, use let.

Limit the use of the UI in before(:context) and after hooks

Limit the use of before(:context) hooks to perform setup tasks with only API calls, non-UI operations, or basic UI operations such as login.

We use capybara-screenshot library to automatically save a screenshot on failure.

capybara-screenshot saves the screenshot in the RSpec's after hook. If there is a failure in before(:context), the after hook is not called and so the screenshot is not saved.

Given this fact, we should limit the use of before(:context) to only those operations where a screenshot is not needed.

Similarly, the after hook should only be used for non-UI operations. Any UI operations in after hook in a test file would execute before the after hook that takes the screenshot. This would result in moving the UI status away from the point of failure and so the screenshot would not be captured at the right moment.

Ensure tests do not leave the browser logged in

All tests expect to be able to log in at the start of the test.

For an example see issue #34736.

Ideally, actions performed in an after(:context) (or before(:context)) block are performed using the API. If it's necessary to do so with the user interface (for example, if API functionality doesn't exist), be sure to sign out at the end of the block.

after(:all) do
  login unless Page::Main::Menu.perform(&:signed_in?)

  # Do something while logged in

  Page::Main::Menu.perform(&:sign_out)
end

Tag tests that require Administrator access

We don't run tests that require Administrator access against our Production environments.

When you add a new test that requires Administrator access, apply the RSpec metadata :requires_admin so that the test will not be included in the test suites executed against Production and other environments on which we don't want to run those tests.

When running tests locally or configuring a pipeline, the environment variable QA_CAN_TEST_ADMIN_FEATURES can be set to false to skip tests that have the :requires_admin tag.

Prefer Commit resource over ProjectPush

In line with using the API, use a Commit resource whenever possible.

ProjectPush uses raw shell commands via the Git Command Line Interface (CLI) whereas the Commit resource makes an HTTP request.

# Using a commit resource
Resource::Repository::Commit.fabricate_via_api! do |commit|
  commit.commit_message = 'Initial commit'
  commit.add_files([
    { file_path: 'README.md', content: 'Hello, GitLab' }
  ])
end

# Using a ProjectPush
Resource::Repository::ProjectPush.fabricate! do |push|
  push.commit_message = 'Initial commit'
  push.file_name = 'README.md'
  push.file_content = 'Hello, GitLab'
end

A few exceptions for using a ProjectPush would be when your test calls for testing SSH integration or using the Git CLI.

Preferred method to blur elements

To blur an element, the preferred method is to click another element that does not alter the test state. If there's a mask that blocks the page elements, such as may occur with some dropdowns, use WebDriver's native mouse events to simulate a click event on the coordinates of an element. Use the following method: click_element_coordinates.

Avoid clicking the body for blurring elements such as inputs and dropdowns because it clicks the center of the viewport. This action can also unintentionally click other elements, altering the test state and causing it to fail.

# Clicking another element to blur an input
def add_issue_to_epic(issue_url)
  find_element(:issue_actions_split_button).find('button', text: 'Add an issue').click
  fill_element(:add_issue_input, issue_url)
  # Clicking the title blurs the input
  click_element(:title)
  click_element(:add_issue_button)
end

# Using native mouse click events in the case of a mask/overlay
click_element_coordinates(:title)

Ensure expect statements wait efficiently

In general, we use an expect statement to check that something is as we expect it. For example:

Page::Project::Pipeline::Show.perform do |pipeline|
  expect(pipeline).to have_job('a_job')
end

Create negatable matchers to speed expect checks

However, sometimes we want to check that something is not as we don't want it to be. In other words, we want to make sure something is absent. For unit tests and feature specs, we commonly use not_to because RSpec's built-in matchers are negatable, as are Capybara's, which means the following two statements are equivalent.

except(page).not_to have_text('hidden')
except(page).to have_no_text('hidden')

Unfortunately, that's not automatically the case for the predicate methods that we add to our page objects. We need to create our own negatable matchers.

The initial example uses the have_job matcher which is derived from the has_job? predicate method of the Page::Project::Pipeline::Show page object. To create a negatable matcher, we use has_no_job? for the negative case:

RSpec::Matchers.define :have_job do |job_name|
  match do |page_object|
    page_object.has_job?(job_name)
  end

  match_when_negated do |page_object|
    page_object.has_no_job?(job_name)
  end
end

And then the two expect statements in the following example are equivalent:

Page::Project::Pipeline::Show.perform do |pipeline|
  expect(pipeline).not_to have_job('a_job')
  expect(pipeline).to have_no_job('a_job')
end

See this merge request for a real example of adding a custom matcher.

We are creating custom negatable matchers in qa/spec/support/matchers.

NOTE: We need to create custom negatable matchers only for the predicate methods we've added to the test framework, and only if we're using not_to. If we use to have_no_* a negatable matcher is not necessary but it increases code readability.

Why we need negatable matchers

Consider the following code, but assume that we don't have a custom negatable matcher for have_job.

# Bad
Page::Project::Pipeline::Show.perform do |pipeline|
  expect(pipeline).not_to have_job('a_job')
end

For this statement to pass, have_job('a_job') has to return false so that not_to can negate it. The problem is that have_job('a_job') waits up to ten seconds for 'a job' to appear before returning false. Under the expected condition this test will take ten seconds longer than it needs to.

Instead, we could force no wait:

# Not as bad but potentially flaky
Page::Project::Pipeline::Show.perform do |pipeline|
  expect(pipeline).not_to have_job('a_job', wait: 0)
end

The problem is that if 'a_job' is present and we're waiting for it to disappear, this statement will fail.

Neither problem is present if we create a custom negatable matcher because the has_no_job? predicate method would be used, which would wait only as long as necessary for the job to disappear.

Lastly, negatable matchers are preferred over using matchers of the form have_no_* because it's a common and familiar practice to negate matchers using not_to. If we facilitate that practice by adding negatable matchers, we make it easier for subsequent test authors to write efficient tests.