diff options
Diffstat (limited to 'doc/development/testing_guide/end_to_end/best_practices.md')
-rw-r--r-- | doc/development/testing_guide/end_to_end/best_practices.md | 76 |
1 files changed, 57 insertions, 19 deletions
diff --git a/doc/development/testing_guide/end_to_end/best_practices.md b/doc/development/testing_guide/end_to_end/best_practices.md index 58bae749dc5..4d12a0f79cb 100644 --- a/doc/development/testing_guide/end_to_end/best_practices.md +++ b/doc/development/testing_guide/end_to_end/best_practices.md @@ -1,6 +1,11 @@ +--- +stage: none +group: Development +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +--- + # End-to-end testing Best Practices -NOTE: **Note:** This is a tailored extension of the Best Practices [found in the testing guide](../best_practices.md). ## Link a test to its test-case issue @@ -263,7 +268,7 @@ We don't run tests that require Administrator access against our Production envi 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. -Note: 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. +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` @@ -288,7 +293,6 @@ Resource::Repository::ProjectPush.fabricate! do |push| end ``` -NOTE: **Note:** A few exceptions for using a `ProjectPush` would be when your test calls for testing SSH integration or using the Git CLI. @@ -321,39 +325,69 @@ In general, we use an `expect` statement to check that something _is_ as we expe ```ruby Page::Project::Pipeline::Show.perform do |pipeline| - expect(pipeline).to have_job("a_job") + expect(pipeline).to have_job('a_job') end ``` -### Ensure `expect` checks for negation efficiently +### 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. In such a case we should use an appropriate -predicate method that returns quickly, rather than waiting for a state that won't appear. +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. + +```ruby +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](page_objects.md). We need to [create our own negatable matchers](https://relishapp.com/rspec/rspec-expectations/v/3-9/docs/custom-matchers/define-a-custom-matcher#matcher-with-separate-logic-for-expect().to-and-expect().not-to). + +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](https://gitlab.com/gitlab-org/gitlab/-/blob/87864b3047c23b4308f59c27a3757045944af447/qa/qa/page/project/pipeline/show.rb#L53). +To create a negatable matcher, we use `has_no_job?` for the negative case: + +```ruby +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 +``` -It's most efficient to use a predicate method that returns immediately when there is no job, or waits -until it disappears: +And then the two `expect` statements in the following example are equivalent: ```ruby -# Good Page::Project::Pipeline::Show.perform do |pipeline| - expect(pipeline).to have_no_job("a_job") + expect(pipeline).not_to have_job('a_job') + expect(pipeline).to have_no_job('a_job') end ``` -### Problematic alternatives +[See this merge request for a real example of adding a custom matcher](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/46302). -Alternatively, if we want to check that a job doesn't exist it might be tempting to use `not_to`: +NOTE: **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. + +### Why we need negatable matchers + +Consider the following code, but assume that we _don't_ have a custom negatable matcher for `have_job`. ```ruby # Bad Page::Project::Pipeline::Show.perform do |pipeline| - expect(pipeline).not_to have_job("a_job") + 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 +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: @@ -361,9 +395,13 @@ Instead, we could force no wait: ```ruby # Not as bad but potentially flaky Page::Project::Pipeline::Show.perform do |pipeline| - expect(pipeline).not_to have_job("a_job", wait: 0) + 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. +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. |